tdb2: catch errors in tdb1_needs_recovery()
authorRusty Russell <rusty@rustcorp.com.au>
Wed, 31 Aug 2011 06:01:07 +0000 (15:31 +0930)
committerRusty Russell <rusty@rustcorp.com.au>
Wed, 31 Aug 2011 06:01:07 +0000 (15:31 +0930)
The tdb1 backend simply returns "true" if there's an error determining
if a tdb needs recovery.  But this leads failtest down a rabbit hole;
it's better to return the error at this case (and makes for better for
diagnostics, since they will come from the first fault, not later in
tdb1_transaction_recover().

ccan/tdb2/tdb1_lock.c
ccan/tdb2/tdb1_private.h
ccan/tdb2/tdb1_transaction.c

index f21a22cacd10457a0191fcf7bd9c8b9065ca8ae2..495e1d1bf6f2e6deec85dcac84458a9c6f189ba9 100644 (file)
@@ -162,13 +162,21 @@ static int tdb1_lock_list(struct tdb_context *tdb, int list, int ltype,
                check = !have_data_locks(tdb);
                ret = tdb1_nest_lock(tdb, lock_offset(list), ltype, waitflag);
 
-               if (ret == 0 && check && tdb1_needs_recovery(tdb)) {
-                       tdb1_nest_unlock(tdb, lock_offset(list), ltype);
+               if (ret == 0 && check) {
+                       tdb_bool_err berr = tdb1_needs_recovery(tdb);
 
-                       if (tdb1_lock_and_recover(tdb) == -1) {
+                       if (berr < 0) {
                                return -1;
                        }
-                       return tdb1_lock_list(tdb, list, ltype, waitflag);
+                       if (berr == true) {
+                               tdb1_nest_unlock(tdb, lock_offset(list), ltype);
+
+                               if (tdb1_lock_and_recover(tdb) == -1) {
+                                       return -1;
+                               }
+                               return tdb1_lock_list(tdb, list, ltype,
+                                                     waitflag);
+                       }
                }
        }
        return ret;
@@ -249,6 +257,7 @@ int tdb1_allrecord_lock(struct tdb_context *tdb, int ltype,
                       enum tdb_lock_flags flags, bool upgradable)
 {
        enum TDB_ERROR ecode;
+       tdb_bool_err berr;
 
        /* tdb_lock_gradual() doesn't know about tdb->tdb1.traverse_read. */
        if (tdb->tdb1.traverse_read && !(tdb->flags & TDB_NOLOCK)) {
@@ -313,7 +322,12 @@ int tdb1_allrecord_lock(struct tdb_context *tdb, int ltype,
        tdb->file->allrecord_lock.ltype = upgradable ? F_WRLCK : ltype;
        tdb->file->allrecord_lock.off = upgradable;
 
-       if (tdb1_needs_recovery(tdb)) {
+       berr = tdb1_needs_recovery(tdb);
+       if (berr < 0) {
+               return -1;
+       }
+
+       if (berr == true) {
                tdb1_allrecord_unlock(tdb, ltype);
                if (tdb1_lock_and_recover(tdb) == -1) {
                        return -1;
index ea147a89676974d69ba955f146e7a3b84cbf0e5b..68dc39f6f8e02d1fcce98feb9a2a55b4e45c4763 100644 (file)
@@ -153,7 +153,7 @@ int tdb1_ofs_read(struct tdb_context *tdb, tdb1_off_t offset, tdb1_off_t *d);
 int tdb1_ofs_write(struct tdb_context *tdb, tdb1_off_t offset, tdb1_off_t *d);
 int tdb1_lock_record(struct tdb_context *tdb, tdb1_off_t off);
 int tdb1_unlock_record(struct tdb_context *tdb, tdb1_off_t off);
-bool tdb1_needs_recovery(struct tdb_context *tdb);
+tdb_bool_err tdb1_needs_recovery(struct tdb_context *tdb);
 int tdb1_rec_read(struct tdb_context *tdb, tdb1_off_t offset, struct tdb1_record *rec);
 int tdb1_rec_write(struct tdb_context *tdb, tdb1_off_t offset, struct tdb1_record *rec);
 int tdb1_do_delete(struct tdb_context *tdb, tdb1_off_t rec_ptr, struct tdb1_record *rec);
index 08eac1df3406b603570ea626bbdf580ac8f635d8..ecd7d2628e4981e42634da4617c6581f831a0d87 100644 (file)
@@ -1289,14 +1289,14 @@ int tdb1_transaction_recover(struct tdb_context *tdb)
 }
 
 /* Any I/O failures we say "needs recovery". */
-bool tdb1_needs_recovery(struct tdb_context *tdb)
+tdb_bool_err tdb1_needs_recovery(struct tdb_context *tdb)
 {
        tdb1_off_t recovery_head;
        struct tdb1_record rec;
 
        /* find the recovery area */
        if (tdb1_ofs_read(tdb, TDB1_RECOVERY_HEAD, &recovery_head) == -1) {
-               return true;
+               return tdb->last_error;
        }
 
        if (recovery_head == 0) {
@@ -1307,7 +1307,7 @@ bool tdb1_needs_recovery(struct tdb_context *tdb)
        /* read the recovery record */
        if (tdb->tdb1.io->tdb1_read(tdb, recovery_head, &rec,
                                   sizeof(rec), TDB1_DOCONV()) == -1) {
-               return true;
+               return tdb->last_error;
        }
 
        return (rec.magic == TDB1_RECOVERY_MAGIC);