lib/tdb2: fix OpenBSD incoherent mmap (tdb1 version)

This is a direct port of the previous patch, to the TDB2 codebase.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/lib/tdb2/tdb1_io.c b/lib/tdb2/tdb1_io.c
index e7d20b8cfcfe847595b9ae36f8f1588819c4dc00..4371c236cc66f720b7ae414fe2af32cce2a77a69 100644 (file)
--- a/lib/tdb2/tdb1_io.c
+++ b/lib/tdb2/tdb1_io.c
@@ -70,8 +70,7 @@ static int tdb1_oob(struct tdb_context *tdb, tdb1_off_t len, int probe)
                return -1;
        }
        tdb->file->map_size = st.st_size;
-       tdb1_mmap(tdb);
-       return 0;
+       return tdb1_mmap(tdb);
 }
 
 /* write a lump of data at a specified offset */
@@ -93,6 +92,10 @@ static int tdb1_write(struct tdb_context *tdb, tdb1_off_t off,
        if (tdb->file->map_ptr) {
                memcpy(off + (char *)tdb->file->map_ptr, buf, len);
        } else {
+#ifdef HAVE_INCOHERENT_MMAP
+               tdb->last_error = TDB_ERR_IO;
+               return -1;
+#else
                ssize_t written = pwrite(tdb->file->fd, buf, len, off);
                if ((written != (ssize_t)len) && (written != -1)) {
                        tdb_logerr(tdb, TDB_ERR_IO, TDB_LOG_WARNING,
@@ -118,6 +121,7 @@ static int tdb1_write(struct tdb_context *tdb, tdb1_off_t off,
                                                len, off);
                        return -1;
                }
+#endif
        }
        return 0;
 }
@@ -143,6 +147,10 @@ static int tdb1_read(struct tdb_context *tdb, tdb1_off_t off, void *buf,
        if (tdb->file->map_ptr) {
                memcpy(buf, off + (char *)tdb->file->map_ptr, len);
        } else {
+#ifdef HAVE_INCOHERENT_MMAP
+               tdb->last_error = TDB_ERR_IO;
+               return -1;
+#else
                ssize_t ret = pread(tdb->file->fd, buf, len, off);
                if (ret != (ssize_t)len) {
                        /* Ensure ecode is set for log fn. */
@@ -154,6 +162,7 @@ static int tdb1_read(struct tdb_context *tdb, tdb1_off_t off, void *buf,
                                                (int)tdb->file->map_size);
                        return -1;
                }
+#endif
        }
        if (cv) {
                tdb1_convert(buf, len);
@@ -206,13 +215,23 @@ int tdb1_munmap(struct tdb_context *tdb)
        return 0;
 }
 
-void tdb1_mmap(struct tdb_context *tdb)
+/* If mmap isn't coherent, *everyone* must always mmap. */
+static bool should_mmap(const struct tdb_context *tdb)
+{
+#ifdef HAVE_INCOHERENT_MMAP
+       return true;
+#else
+       return !(tdb->flags & TDB_NOMMAP);
+#endif
+}
+
+int tdb1_mmap(struct tdb_context *tdb)
 {
        if (tdb->flags & TDB_INTERNAL)
-               return;
+               return 0;
 
 #if HAVE_MMAP
-       if (!(tdb->flags & TDB_NOMMAP)) {
+       if (should_mmap(tdb)) {
                int mmap_flags;
                if ((tdb->open_flags & O_ACCMODE) == O_RDONLY)
                        mmap_flags = PROT_READ;
@@ -233,6 +252,10 @@ void tdb1_mmap(struct tdb_context *tdb)
                                   "tdb1_mmap failed for size %llu (%s)",
                                   (long long)tdb->file->map_size,
                                   strerror(errno));
+#ifdef HAVE_INCOHERENT_MMAP
+                       tdb->last_error = TDB_ERR_IO;
+                       return -1;
+#endif
                }
        } else {
                tdb->file->map_ptr = NULL;
@@ -240,6 +263,7 @@ void tdb1_mmap(struct tdb_context *tdb)
 #else
        tdb->file->map_ptr = NULL;
 #endif
+       return 0;
 }
 
 /* expand a file.  we prefer to use ftruncate, as that is what posix
@@ -353,12 +377,6 @@ int tdb1_expand(struct tdb_context *tdb, tdb1_off_t size)
        if (!(tdb->flags & TDB_INTERNAL))
                tdb1_munmap(tdb);
 
-       /*
-        * We must ensure the file is unmapped before doing this
-        * to ensure consistency with systems like OpenBSD where
-        * writes and mmaps are not consistent.
-        */
-
        /* expand the file itself */
        if (!(tdb->flags & TDB_INTERNAL)) {
                if (tdb->tdb1.io->tdb1_expand_file(tdb, tdb->file->map_size, size) != 0)
@@ -379,14 +397,9 @@ int tdb1_expand(struct tdb_context *tdb, tdb1_off_t size)
                }
                tdb->file->map_ptr = new_map_ptr;
        } else {
-               /*
-                * We must ensure the file is remapped before adding the space
-                * to ensure consistency with systems like OpenBSD where
-                * writes and mmaps are not consistent.
-                */
-
-               /* We're ok if the mmap fails as we'll fallback to read/write */
-               tdb1_mmap(tdb);
+               if (tdb1_mmap(tdb) != 0) {
+                       goto fail;
+               }
        }
 
        /* form a new freelist record */

diff --git a/lib/tdb2/tdb1_private.h b/lib/tdb2/tdb1_private.h
index 68dc39f6f8e02d1fcce98feb9a2a55b4e45c4763..833298b737b4f99a68d5c681263073c77f71c3e2 100644 (file)
--- a/lib/tdb2/tdb1_private.h
+++ b/lib/tdb2/tdb1_private.h
@@ -121,7 +121,7 @@ struct tdb1_methods {
   internal prototypes
 */
 int tdb1_munmap(struct tdb_context *tdb);
-void tdb1_mmap(struct tdb_context *tdb);
+int tdb1_mmap(struct tdb_context *tdb);
 int tdb1_lock(struct tdb_context *tdb, int list, int ltype);
 int tdb1_nest_lock(struct tdb_context *tdb, uint32_t offset, int ltype,
                  enum tdb_lock_flags flags);

diff --git a/lib/tdb2/test/run-tdb1-3G-file.c b/lib/tdb2/test/run-tdb1-3G-file.c
index e75122ada99cd928daa4bc7bb218187b6d5b0ef3..148611753f27913c7d4c4613ebb6391cd657e7b1 100644 (file)
--- a/lib/tdb2/test/run-tdb1-3G-file.c
+++ b/lib/tdb2/test/run-tdb1-3G-file.c
@@ -63,6 +63,7 @@ int main(int argc, char *argv[])
        tdb1_off_t rec_ptr;
        struct tdb1_record rec;
        union tdb_attribute hsize;
+       int ret;
 
        hsize.base.attr = TDB_ATTRIBUTE_TDB1_HASHSIZE;
        hsize.base.next = &tap_log_attr;
@@ -75,15 +76,33 @@ int main(int argc, char *argv[])
        ok1(tdb);
        tdb->tdb1.io = &large_io_methods;
 
-       /* Enlarge the file (internally multiplies by 2). */
-       ok1(tdb1_expand(tdb, 1500000000) == 0);
-
-       /* Put an entry in, and check it. */
        key.dsize = strlen("hi");
        key.dptr = (void *)"hi";
        orig_data.dsize = strlen("world");
        orig_data.dptr = (void *)"world";
 
+       /* Enlarge the file (internally multiplies by 2). */
+       ret = tdb1_expand(tdb, 1500000000);
+
+#ifdef HAVE_INCOHERENT_MMAP
+       /* This can fail due to mmap failure on 32 bit systems. */
+       if (ret == -1) {
+               /* These should now fail. */
+               ok1(tdb_store(tdb, key, orig_data, TDB_INSERT) == TDB_ERR_IO);
+               ok1(tdb_fetch(tdb, key, &data) == TDB_ERR_IO);
+               ok1(tdb_traverse(tdb, test_traverse, &orig_data) == TDB_ERR_IO);
+               ok1(tdb_delete(tdb, key) == TDB_ERR_IO);
+               ok1(tdb_traverse(tdb, test_traverse, NULL) == TDB_ERR_IO);
+               /* Skip the rest... */
+               for (ret = 0; ret < 26 - 6; ret++)
+                       ok1(1);
+               tdb_close(tdb);
+               return exit_status();
+       }
+#endif
+       ok1(ret == 0);
+
+       /* Put an entry in, and check it. */
        ok1(tdb_store(tdb, key, orig_data, TDB_INSERT) == TDB_SUCCESS);
 
        ok1(tdb_fetch(tdb, key, &data) == TDB_SUCCESS);