From 637fc5ec0869145d1b5272ee0e1e51e21517be79 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Mon, 7 Jun 2021 14:54:04 +0800 Subject: [PATCH] [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 (cherry picked from commit 62536defc86f12363e4178ee5a2cefc40fa416d9) --- ft-lib/bcache.c | 20 ++++++++++--- functional-tests/thin-functional-tests.scm | 46 ++++++++++++++++++++++++++++++ functional-tests/thin/xml.scm | 4 +-- thin-provisioning/restore_emitter.cc | 2 +- 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/ft-lib/bcache.c b/ft-lib/bcache.c index ee5b6c5..1c63377 100644 --- a/ft-lib/bcache.c +++ b/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) { diff --git a/functional-tests/thin-functional-tests.scm b/functional-tests/thin-functional-tests.scm index 55b0e60..fcabddf 100644 --- a/functional-tests/thin-functional-tests.scm +++ b/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) diff --git a/functional-tests/thin/xml.scm b/functional-tests/thin/xml.scm index 551a536..7d9314b 100644 --- a/functional-tests/thin/xml.scm +++ b/functional-tests/thin/xml.scm @@ -22,7 +22,7 @@ (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))) @@ -30,7 +30,7 @@ (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))) diff --git a/thin-provisioning/restore_emitter.cc b/thin-provisioning/restore_emitter.cc index 6e95a53..114ba4f 100644 --- a/thin-provisioning/restore_emitter.cc +++ b/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; -- 1.8.3.1