339 lines
12 KiB
Diff
339 lines
12 KiB
Diff
From 6ce838b83c28d9b64f34cf0c68cfebaf9affac11 Mon Sep 17 00:00:00 2001
|
|
From: Ming-Hung Tsai <mtsai@redhat.com>
|
|
Date: Fri, 21 Aug 2020 18:26:48 +0800
|
|
Subject: [PATCH] [thin_check] Allow using --clear-needs-check and
|
|
--skip-mappings together
|
|
|
|
Although it is not recommended to clear the flag without a full
|
|
examination, however, the usage has been documented as an approach
|
|
to reduce lvchange run time [1]. For the purpose of backward
|
|
compatibility and avoiding boot failure after upgrading thin_check [2],
|
|
the limitation is now removed.
|
|
|
|
[1] https://wiki.archlinux.org/index.php/LVM#Thinly-provisioned_root_volume_device_times_out
|
|
[2] Community feedback on previous commit:
|
|
https://github.com/jthornber/thin-provisioning-tools/commit/b278f4f
|
|
|
|
(cherry picked from commit f4675e3f32aad3d1518869e4f3824e05230c6a5d)
|
|
---
|
|
functional-tests/thin-functional-tests.scm | 65 +++++++++++++---
|
|
thin-provisioning/metadata_checker.cc | 88 +++++++++++-----------
|
|
thin-provisioning/metadata_checker.h | 5 +-
|
|
thin-provisioning/thin_check.cc | 2 +-
|
|
4 files changed, 106 insertions(+), 54 deletions(-)
|
|
|
|
diff --git a/functional-tests/thin-functional-tests.scm b/functional-tests/thin-functional-tests.scm
|
|
index 37d3df9..5b1423c 100644
|
|
--- a/functional-tests/thin-functional-tests.scm
|
|
+++ b/functional-tests/thin-functional-tests.scm
|
|
@@ -189,15 +189,6 @@
|
|
(run-fail (thin-check "--auto-repair" "--skip-mappings" md))
|
|
(run-fail (thin-check "--auto-repair" "--ignore-non-fatal-errors" md))))
|
|
|
|
- (define-scenario (thin-check incompatible-options clear-needs-check-flag)
|
|
- "Incompatible options should cause failure"
|
|
- (with-valid-metadata (md)
|
|
- (run-fail (thin-check "--clear-needs-check-flag" "-m" md))
|
|
- (run-fail (thin-check "--clear-needs-check-flag" "--override-mapping-root 123" md))
|
|
- (run-fail (thin-check "--clear-needs-check-flag" "--super-block-only" md))
|
|
- (run-fail (thin-check "--clear-needs-check-flag" "--skip-mappings" md))
|
|
- (run-fail (thin-check "--clear-needs-check-flag" "--ignore-non-fatal-errors" md))))
|
|
-
|
|
(define-scenario (thin-check superblock-only-valid)
|
|
"--super-block-only check passes on valid metadata"
|
|
(with-valid-metadata (md)
|
|
@@ -230,6 +221,62 @@
|
|
(with-valid-metadata (md)
|
|
(run-ok (thin-check "--clear-needs-check-flag" md))))
|
|
|
|
+ (define-scenario (thin-check mixing-clear-needs-check-flag super-block-only-should-pass)
|
|
+ "Accepts --clear-needs-check and --super-block-only"
|
|
+ (with-valid-metadata (md)
|
|
+ (run-ok (thin-check "--clear-needs-check-flag" "--super-block-only" md))))
|
|
+
|
|
+ (define-scenario (thin-check mixing-clear-needs-check-flag skip-mappings-should-pass)
|
|
+ "Accepts --clear-needs-check and --skip-mappings"
|
|
+ (with-valid-metadata (md)
|
|
+ (run-ok (thin-check "--clear-needs-check-flag" "--skip-mappings" md))))
|
|
+
|
|
+ (define-scenario (thin-check mixing-clear-needs-check-flag ignore-non-fatal-errors-should-pass)
|
|
+ "Accepts --clear-needs-check and --ignore-non-fatal-errors"
|
|
+ (with-valid-metadata (md)
|
|
+ (run-ok (thin-check "--clear-needs-check-flag" "--ignore-non-fatal-errors" md))))
|
|
+
|
|
+ (define-scenario (thin-check try-clear-needs-check-flag super-block-only-should-clear)
|
|
+ "--clear-needs-check is a noop while using --super-block-only"
|
|
+ (with-valid-metadata (md)
|
|
+ (set-needs-check-flag md)
|
|
+ (assert-metadata-needs-check md)
|
|
+ (run-ok (thin-check "--clear-needs-check-flag" "--super-block-only" md))
|
|
+ (assert-metadata-clean md)))
|
|
+
|
|
+ (define-scenario (thin-check try-clear-needs-check-flag skip-mappings-should-clear)
|
|
+ "--clear-needs-check is a noop while using --skip-mappings"
|
|
+ (with-valid-metadata (md)
|
|
+ (set-needs-check-flag md)
|
|
+ (assert-metadata-needs-check md)
|
|
+ (run-ok (thin-check "--clear-needs-check-flag" "--skip-mappings" md))
|
|
+ (assert-metadata-clean md)))
|
|
+
|
|
+ (define-scenario (thin-check try-clear-needs-check-flag ignore-non-fatal-errors-should-clear)
|
|
+ "--clear-needs-check works while using --ignore-non-fatal-errors"
|
|
+ (with-valid-metadata (md)
|
|
+ (set-needs-check-flag md)
|
|
+ (assert-metadata-needs-check md)
|
|
+ (run-ok (thin-check "--clear-needs-check-flag" "--ignore-non-fatal-errors" md))
|
|
+ (assert-metadata-clean md)))
|
|
+
|
|
+ (define-scenario (thin-check try-clear-needs-check-flag no-errors-should-clear)
|
|
+ "--clear-needs-check works if there's no errors"
|
|
+ (with-valid-metadata (md)
|
|
+ (set-needs-check-flag md)
|
|
+ (assert-metadata-needs-check md)
|
|
+ (run-ok (thin-check "--clear-needs-check-flag" md))
|
|
+ (assert-metadata-clean md)))
|
|
+
|
|
+ (define-scenario (thin-check try-clear-needs-check-flag fatal-errors-should-keep)
|
|
+ "--clear-needs-check is a noop if there's fatal errors"
|
|
+ (with-valid-metadata (md)
|
|
+ (set-needs-check-flag md)
|
|
+ (tamper-mapping-root md 10)
|
|
+ (assert-metadata-needs-check md)
|
|
+ (run-fail (thin-check "--clear-needs-check-flag" md))
|
|
+ (assert-metadata-needs-check md)))
|
|
+
|
|
(define-scenario (thin-check auto-repair)
|
|
"Accepts --auto-repair"
|
|
(with-valid-metadata (md)
|
|
diff --git a/thin-provisioning/metadata_checker.cc b/thin-provisioning/metadata_checker.cc
|
|
index e81e22c..1c9734e 100644
|
|
--- a/thin-provisioning/metadata_checker.cc
|
|
+++ b/thin-provisioning/metadata_checker.cc
|
|
@@ -371,7 +371,8 @@ namespace {
|
|
out_(cerr, 2),
|
|
info_out_(cout, 0),
|
|
expected_rc_(true), // set stop on the first error
|
|
- err_(NO_ERROR) {
|
|
+ err_(NO_ERROR),
|
|
+ metadata_checked_(false) {
|
|
|
|
if (output_opts == OUTPUT_QUIET) {
|
|
out_.disable();
|
|
@@ -381,7 +382,29 @@ namespace {
|
|
sb_location_ = get_superblock_location();
|
|
}
|
|
|
|
- void check() {
|
|
+ void check_and_repair() {
|
|
+ if (!check())
|
|
+ return;
|
|
+
|
|
+ if (!options_.use_metadata_snap_ &&
|
|
+ !options_.override_mapping_root_) {
|
|
+ if (options_.sm_opts_ == check_options::SPACE_MAP_FULL &&
|
|
+ options_.fix_metadata_leaks_)
|
|
+ fix_metadata_leaks(options_.open_transaction_);
|
|
+ if (options_.clear_needs_check_)
|
|
+ clear_needs_check_flag();
|
|
+ }
|
|
+ }
|
|
+
|
|
+ bool get_status() const {
|
|
+ if (options_.ignore_non_fatal_)
|
|
+ return (err_ == FATAL) ? false : true;
|
|
+
|
|
+ return (err_ == NO_ERROR) ? true : false;
|
|
+ }
|
|
+
|
|
+ private:
|
|
+ bool check() {
|
|
block_manager::ptr bm = open_bm(path_, block_manager::READ_ONLY,
|
|
!options_.use_metadata_snap_);
|
|
|
|
@@ -389,7 +412,7 @@ namespace {
|
|
if (err_ == FATAL) {
|
|
if (check_for_xml(bm))
|
|
out_ << "This looks like XML. thin_check only checks the binary metadata format." << end_message();
|
|
- return;
|
|
+ return false;
|
|
}
|
|
|
|
transaction_manager::ptr tm = open_tm(bm, sb_location_);
|
|
@@ -407,7 +430,7 @@ namespace {
|
|
err_ << examine_data_mappings(tm, sb, options_.data_mapping_opts_, out_, core_sm);
|
|
|
|
if (err_ == FATAL)
|
|
- return;
|
|
+ return false;
|
|
|
|
// if we're checking everything, and there were no errors,
|
|
// then we should check the space maps too.
|
|
@@ -419,10 +442,14 @@ namespace {
|
|
} else
|
|
err_ << examine_data_mappings(tm, sb, options_.data_mapping_opts_, out_,
|
|
optional<space_map::ptr>());
|
|
+
|
|
+ metadata_checked_ = true;
|
|
+
|
|
+ return true;
|
|
}
|
|
|
|
bool fix_metadata_leaks(bool open_transaction) {
|
|
- if (!verify_preconditions_before_fixing()) {
|
|
+ if (!metadata_checked_) {
|
|
out_ << "metadata has not been fully examined" << end_message();
|
|
return false;
|
|
}
|
|
@@ -458,12 +485,13 @@ namespace {
|
|
}
|
|
|
|
bool clear_needs_check_flag() {
|
|
- if (!verify_preconditions_before_fixing()) {
|
|
+ if (!metadata_checked_) {
|
|
out_ << "metadata has not been fully examined" << end_message();
|
|
return false;
|
|
}
|
|
|
|
- if (err_ != NO_ERROR)
|
|
+ if (err_ == FATAL ||
|
|
+ (err_ == NON_FATAL && !options_.ignore_non_fatal_))
|
|
return false;
|
|
|
|
block_manager::ptr bm = open_bm(path_, block_manager::READ_WRITE);
|
|
@@ -480,14 +508,6 @@ namespace {
|
|
return true;
|
|
}
|
|
|
|
- bool get_status() const {
|
|
- if (options_.ignore_non_fatal_)
|
|
- return (err_ == FATAL) ? false : true;
|
|
-
|
|
- return (err_ == NO_ERROR) ? true : false;
|
|
- }
|
|
-
|
|
- private:
|
|
block_address
|
|
get_superblock_location() {
|
|
block_address sb_location = superblock_detail::SUPERBLOCK_LOCATION;
|
|
@@ -545,19 +565,6 @@ namespace {
|
|
return err;
|
|
}
|
|
|
|
- bool verify_preconditions_before_fixing() const {
|
|
- if (options_.use_metadata_snap_ ||
|
|
- !!options_.override_mapping_root_ ||
|
|
- options_.sm_opts_ != check_options::SPACE_MAP_FULL ||
|
|
- options_.data_mapping_opts_ != check_options::DATA_MAPPING_LEVEL2)
|
|
- return false;
|
|
-
|
|
- if (!expected_rc_.get_counts().size())
|
|
- return false;
|
|
-
|
|
- return true;
|
|
- }
|
|
-
|
|
std::string const &path_;
|
|
check_options options_;
|
|
nested_output out_;
|
|
@@ -565,6 +572,7 @@ namespace {
|
|
block_address sb_location_;
|
|
block_counter expected_rc_;
|
|
base::error_state err_; // metadata state
|
|
+ bool metadata_checked_;
|
|
};
|
|
}
|
|
|
|
@@ -603,8 +611,9 @@ void check_options::set_ignore_non_fatal() {
|
|
ignore_non_fatal_ = true;
|
|
}
|
|
|
|
-void check_options::set_fix_metadata_leaks() {
|
|
+void check_options::set_auto_repair() {
|
|
fix_metadata_leaks_ = true;
|
|
+ clear_needs_check_ = true;
|
|
}
|
|
|
|
void check_options::set_clear_needs_check() {
|
|
@@ -612,7 +621,7 @@ void check_options::set_clear_needs_check() {
|
|
}
|
|
|
|
bool check_options::check_conformance() {
|
|
- if (fix_metadata_leaks_ || clear_needs_check_) {
|
|
+ if (fix_metadata_leaks_) {
|
|
if (ignore_non_fatal_) {
|
|
cerr << "cannot perform fix by ignoring non-fatal errors" << endl;
|
|
return false;
|
|
@@ -627,12 +636,12 @@ bool check_options::check_conformance() {
|
|
cerr << "cannot perform fix with an overridden mapping root" << endl;
|
|
return false;
|
|
}
|
|
+ }
|
|
|
|
- if (data_mapping_opts_ != DATA_MAPPING_LEVEL2 ||
|
|
- sm_opts_ != SPACE_MAP_FULL) {
|
|
- cerr << "cannot perform fix without a full examination" << endl;
|
|
- return false;
|
|
- }
|
|
+ if (fix_metadata_leaks_ &&
|
|
+ (data_mapping_opts_ != DATA_MAPPING_LEVEL2 || sm_opts_ != SPACE_MAP_FULL)) {
|
|
+ cerr << "cannot perform fix without a full examination" << endl;
|
|
+ return false;
|
|
}
|
|
|
|
return true;
|
|
@@ -646,14 +655,7 @@ thin_provisioning::check_metadata(std::string const &path,
|
|
output_options output_opts)
|
|
{
|
|
metadata_checker checker(path, check_opts, output_opts);
|
|
-
|
|
- checker.check();
|
|
- if (check_opts.fix_metadata_leaks_)
|
|
- checker.fix_metadata_leaks(check_opts.open_transaction_);
|
|
- if (check_opts.fix_metadata_leaks_ ||
|
|
- check_opts.clear_needs_check_)
|
|
- checker.clear_needs_check_flag();
|
|
-
|
|
+ checker.check_and_repair();
|
|
return checker.get_status();
|
|
}
|
|
|
|
diff --git a/thin-provisioning/metadata_checker.h b/thin-provisioning/metadata_checker.h
|
|
index 5569d27..ea66dc3 100644
|
|
--- a/thin-provisioning/metadata_checker.h
|
|
+++ b/thin-provisioning/metadata_checker.h
|
|
@@ -45,14 +45,17 @@ namespace thin_provisioning {
|
|
void set_override_mapping_root(bcache::block_address b);
|
|
void set_metadata_snap();
|
|
void set_ignore_non_fatal();
|
|
- void set_fix_metadata_leaks();
|
|
+ void set_auto_repair();
|
|
void set_clear_needs_check();
|
|
|
|
+ // flags for checking
|
|
bool use_metadata_snap_;
|
|
data_mapping_options data_mapping_opts_;
|
|
space_map_options sm_opts_;
|
|
boost::optional<bcache::block_address> override_mapping_root_;
|
|
bool ignore_non_fatal_;
|
|
+
|
|
+ // flags for repairing
|
|
bool fix_metadata_leaks_;
|
|
bool clear_needs_check_;
|
|
bool open_transaction_;
|
|
diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc
|
|
index 60f7838..e3c9db3 100644
|
|
--- a/thin-provisioning/thin_check.cc
|
|
+++ b/thin-provisioning/thin_check.cc
|
|
@@ -166,7 +166,7 @@ thin_check_cmd::run(int argc, char **argv)
|
|
|
|
case 6:
|
|
// auto-repair
|
|
- fs.check_opts.set_fix_metadata_leaks();
|
|
+ fs.check_opts.set_auto_repair();
|
|
break;
|
|
|
|
default:
|
|
--
|
|
2.31.1
|
|
|