ntdb: fix recovery data write.

We were missing the last few bytes.  Found by 100 runs of ntdbtorture
-t -k.

The transaction test code didn't catch this, because usually those
last few bytes are irrelevant to the actual contents of the database.
We fix the test.

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

diff --git a/lib/ntdb/test/run-57-die-during-transaction.c b/lib/ntdb/test/run-57-die-during-transaction.c
index eb81c996cfe71d8683929390678b63e2242b5ee9..c508854f01a93ce1af3d811b4d83f77779d6b28f 100644 (file)
--- a/lib/ntdb/test/run-57-die-during-transaction.c
+++ b/lib/ntdb/test/run-57-die-during-transaction.c
@@ -103,6 +103,7 @@ static int target, current;
 static jmp_buf jmpbuf;
 #define TEST_DBNAME "run-57-die-during-transaction.ntdb"
 #define KEY_STRING "helloworld"
+#define DATA_STRING "Helloworld"
 
 static void maybe_die(int fd)
 {
@@ -152,14 +153,18 @@ static int ftruncate_check(int fd, off_t length)
        return ret;
 }
 
-static bool test_death(enum operation op, struct agent *agent)
+static bool test_death(enum operation op, struct agent *agent,
+                      bool pre_create_recovery)
 {
        struct ntdb_context *ntdb = NULL;
-       NTDB_DATA key;
+       NTDB_DATA key, data;
        enum agent_return ret;
        int needed_recovery = 0;
 
        current = target = 0;
+       /* Big long data to force a change. */
+       data = ntdb_mkdata(DATA_STRING, strlen(DATA_STRING));
+
 reset:
        unlink(TEST_DBNAME);
        ntdb = ntdb_open(TEST_DBNAME, NTDB_NOMMAP,
@@ -184,8 +189,14 @@ reset:
                        return false;
                }
 
+               /* Could be key, or data. */
                ret = external_agent_operation(agent, op,
                                               KEY_STRING "=" KEY_STRING);
+               if (ret != SUCCESS) {
+                       ret = external_agent_operation(agent, op,
+                                                      KEY_STRING
+                                                      "=" DATA_STRING);
+               }
                if (ret != SUCCESS) {
                        diag("Step %u op %s failed = %s", current,
                             operation_name(op),
@@ -228,8 +239,20 @@ reset:
 
        /* Put key for agent to fetch. */
        key = ntdb_mkdata(KEY_STRING, strlen(KEY_STRING));
+
+       if (pre_create_recovery) {
+               /* Using a transaction now means we allocate the recovery
+                * area immediately.  That makes the later transaction smaller
+                * and thus tickles a bug we had. */
+               if (ntdb_transaction_start(ntdb) != 0)
+                       return false;
+       }
        if (ntdb_store(ntdb, key, key, NTDB_INSERT) != 0)
                return false;
+       if (pre_create_recovery) {
+               if (ntdb_transaction_commit(ntdb) != 0)
+                       return false;
+       }
 
        /* This is the key we insert in transaction. */
        key.dsize--;
@@ -246,7 +269,7 @@ reset:
        if (ntdb_transaction_start(ntdb) != 0)
                return false;
 
-       if (ntdb_store(ntdb, key, key, NTDB_INSERT) != 0)
+       if (ntdb_store(ntdb, key, data, NTDB_INSERT) != 0)
                return false;
 
        if (ntdb_transaction_commit(ntdb) != 0)
@@ -275,9 +298,9 @@ int main(int argc, char *argv[])
 {
        enum operation ops[] = { FETCH, STORE, TRANSACTION_START };
        struct agent *agent;
-       int i;
+       int i, j;
 
-       plan_tests(12);
+       plan_tests(24);
        unlock_callback = maybe_die;
 
        external_agent_free = free_noleak;
@@ -285,9 +308,12 @@ int main(int argc, char *argv[])
        if (!agent)
                err(1, "preparing agent");
 
-       for (i = 0; i < sizeof(ops)/sizeof(ops[0]); i++) {
-               diag("Testing %s after death", operation_name(ops[i]));
-               ok1(test_death(ops[i], agent));
+       for (j = 0; j < 2; j++) {
+               for (i = 0; i < sizeof(ops)/sizeof(ops[0]); i++) {
+                       diag("Testing %s after death (%s recovery area)",
+                            operation_name(ops[i]), j ? "with" : "without");
+                       ok1(test_death(ops[i], agent, j));
+               }
        }
 
        free_external_agent(agent);

diff --git a/lib/ntdb/transaction.c b/lib/ntdb/transaction.c
index 9f953a50e34d558b2fe820b9083ad2cb3a2777ab..05f571e51a84a2a7aa52fa6383c30bab808702f4 100644 (file)
--- a/lib/ntdb/transaction.c
+++ b/lib/ntdb/transaction.c
@@ -927,7 +927,8 @@ static enum NTDB_ERROR transaction_setup_recovery(struct ntdb_context *ntdb)
        ntdb_convert(ntdb, recovery, sizeof(*recovery));
 
        /* write the recovery data to the recovery area */
-       ecode = methods->twrite(ntdb, recovery_off, recovery, recovery_size);
+       ecode = methods->twrite(ntdb, recovery_off, recovery,
+                               sizeof(*recovery) + recovery_size);
        if (ecode != NTDB_SUCCESS) {
                free(recovery);
                return ntdb_logerr(ntdb, ecode, NTDB_LOG_ERROR,