rpms
/
device-mapper-persistent-data
7
0
Fork 0
Code Issues Pull Requests Releases Activity
device-mapper-persistent-data/SOURCES/0019-thin_check-Allow-using...

339 lines
12 KiB
Diff
Raw Permalink Blame History

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
Reference in New Issue View Git Blame Copy Permalink