[thin] Clear superblock flags in restored metadata

The needs_check flag is unnecessary for a restored metadata since
it is assumed clean and has no errors

ft-lib/bcache.c

@@ -201,8 +201,10 @@ static int engine_issue(struct io_engine *e, int fd, enum dir d,
 
 	cb_array[0] = &cb->cb;
 	r = io_submit(e->aio_context, 1, cb_array);
-	if (r < 0)
+	if (r < 0) {
+		warn("io_submit failed, ret=%d\n", r);
 		cb_free(e->cbs, cb);
+	}
 
 	return r;
 }
@@ -219,7 +221,7 @@ static int engine_wait(struct io_engine *e, struct timespec *ts, complete_fn fn)
 	memset(&event, 0, sizeof(event));
 	r = io_getevents(e->aio_context, 1, MAX_IO, event, ts);
 	if (r < 0) {
-		warn("io_getevents failed");
+		warn("io_getevents failed, ret=%d\n", r);
 		return r;
 	}
 
@@ -514,12 +516,22 @@ static void relink(struct block *b)
  */
 static int issue_low_level(struct block *b, enum dir d)
 {
+	int r;
 	struct bcache *cache = b->cache;
 	sector_t sb = b->index * cache->block_sectors;
 	sector_t se = sb + cache->block_sectors;
 	set_flags(b, BF_IO_PENDING);
+	cache->nr_io_pending++;
+	list_add_tail(&b->list, &cache->io_pending);
 
-	return engine_issue(cache->engine, cache->fd, d, sb, se, b->data, b);
+	r = engine_issue(cache->engine, cache->fd, d, sb, se, b->data, b);
+	if (r < 0) {
+		list_del(&b->list);
+		cache->nr_io_pending--;
+		clear_flags(b, BF_IO_PENDING);
+		return r;
+	}
+	return 0;
 }
 
 static void issue_read(struct block *b)
@@ -709,7 +721,7 @@ struct bcache *bcache_simple(const char *path, unsigned nr_cache_blocks)
 	int r;
 	struct stat info;
 	struct bcache *cache;
-	int fd = open(path, O_DIRECT | O_EXCL | O_RDONLY);
+	int fd = open(path, O_DIRECT | O_EXCL | O_RDWR);
 	uint64_t s;
 
 	if (fd < 0) {

functional-tests/thin-functional-tests.scm

@@ -12,6 +12,7 @@
     (process)
     (scenario-string-constants)
     (temp-file)
+    (thin metadata)
     (thin xml)
     (srfi s8 receive))
 
@@ -30,6 +31,12 @@
        (with-temp-file-containing ((v "thin.xml" (fmt #f (generate-xml 10 1000))))
                                   b1 b2 ...))))
 
+  (define-syntax with-needs-check-thin-xml
+    (syntax-rules ()
+      ((_ (v) b1 b2 ...)
+       (with-temp-file-containing ((v "thin.xml" (fmt #f (generate-xml 10 1000 1))))
+                                  b1 b2 ...))))
+
   (define-syntax with-valid-metadata
     (syntax-rules ()
       ((_ (md) b1 b2 ...)
@@ -63,6 +70,28 @@
          (damage-superblock md)
          b1 b2 ...))))
 
+  (define superblock-salt 160774)
+  (define (set-needs-check-flag md)
+    (with-bcache (cache md 1)
+      (with-block (b cache 0 (get-flags dirty))
+        (let ((sb (block->superblock b)))
+          (ftype-set! ThinSuperblock (flags) sb 1)
+          ;;;;;; Update the csum manually since the block validator for ft-lib is not ready
+          (let ((csum (checksum-block b (ftype-sizeof unsigned-32) superblock-salt)))
+            (ftype-set! ThinSuperblock (csum) sb csum))))))
+
+  (define (get-superblock-flags md)
+    (with-bcache (cache md 1)
+      (with-block (b cache 0 (get-flags))
+        (let ((sb (block->superblock b)))
+          (ftype-ref ThinSuperblock (flags) sb)))))
+
+  (define (assert-metadata-needs-check md)
+    (assert-equal (get-superblock-flags md) 1))
+
+  (define (assert-metadata-clean md)
+    (assert-equal (get-superblock-flags md) 0))
+
   ;; We have to export something that forces all the initialisation expressions
   ;; to run.
   (define (register-thin-tests) #t)
@@ -173,6 +202,13 @@
   ;;;-----------------------------------------------------------
   ;;; thin_restore scenarios
   ;;;-----------------------------------------------------------
+  (define-scenario (thin-restore clear-needs-check-flag)
+    "thin_restore should clear the needs-check flag"
+    (with-empty-metadata (md)
+      (with-needs-check-thin-xml (xml)
+        (run-ok-rcv (stdout _) (thin-restore "-i" xml "-o" md "-q")
+          (assert-eof stdout)))
+      (assert-metadata-clean md)))
 
   (define-scenario (thin-restore print-version-v)
     "print help (-V)"
@@ -439,6 +475,16 @@
   ;;;-----------------------------------------------------------
   ;;; thin_repair scenarios
   ;;;-----------------------------------------------------------
+  (define-scenario (thin-repair clear-needs-check-flag)
+    "thin_repair should clear the needs-check flag"
+    (with-valid-metadata (md1)
+      (set-needs-check-flag md1)
+      (assert-metadata-needs-check md1)
+      (with-empty-metadata (md2)
+        (run-ok-rcv (stdout stderr) (thin-repair "-i" md1 "-o" md2)
+          (assert-eof stderr))
+        (assert-metadata-clean md2))))
+
   (define-scenario (thin-repair dont-repair-xml)
     "Fails gracefully if run on XML rather than metadata"
     (with-thin-xml (xml)

functional-tests/thin/xml.scm

@@ -22,15 +22,15 @@
                                (length . ,nr-mappings)
                                (time . 1)))))
 
-  (define (generate-xml max-thins max-mappings)
+  (define (generate-xml max-thins max-mappings . needs-check)
     (let ((nr-thins ((make-uniform-generator 1 max-thins)))
           (nr-mappings-g (make-uniform-generator (div-down max-mappings 2)
                                                  max-mappings)))
       (let ((nr-mappings (iterate nr-mappings-g nr-thins)))
        (tag 'superblock `((uuid . "")
                          (time . 1)
                          (transaction . 1)
-                         (flags . 0)
+                         (flags . ,(if (null? needs-check) 0 (car needs-check)))
                          (version . 2)
                          (data-block-size . 128)
                          (nr-data-blocks . ,(apply + nr-mappings)))

thin-provisioning/restore_emitter.cc

@@ -57,7 +57,7 @@ namespace {
 			memcpy(&sb.uuid_, uuid.c_str(), std::min(sizeof(sb.uuid_), uuid.length()));
 			sb.time_ = time;
 			sb.trans_id_ = trans_id;
-			sb.flags_ = flags ? *flags : 0;
+			sb.flags_ = 0;
 			sb.version_ = version ? *version : 1;
 			sb.data_block_size_ = data_block_size;
 			sb.metadata_snap_ = metadata_snap ? *metadata_snap : 0;