diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 64ed669..4e0af8e 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -14,6 +14,7 @@ #include "base/error_state.h" #include "base/error_string.h" +#include "base/file_utils.h" #include "base/nested_output.h" #include "caching/commands.h" #include "caching/metadata.h" @@ -212,12 +213,18 @@ namespace { error_state metadata_check(string const &path, flags const &fs, bool &needs_check_set) { - block_manager<>::ptr bm = open_bm(path, block_manager<>::READ_ONLY); - nested_output out(cerr, 2); if (fs.quiet_) out.disable(); + if (file_utils::get_file_length(path) < persistent_data::MD_BLOCK_SIZE) { + out << "Metadata device/file too small. Is this binary metadata?" + << end_message(); + return FATAL; + } + + block_manager<>::ptr bm = open_bm(path, block_manager<>::READ_ONLY); + superblock_reporter sb_rep(out); mapping_reporter mapping_rep(out); hint_reporter hint_rep(out); @@ -229,8 +236,11 @@ namespace { check_superblock(bm, bm->get_nr_blocks(), sb_rep); } - if (sb_rep.get_error() == FATAL) + if (sb_rep.get_error() == FATAL) { + if (check_for_xml(bm)) + out << "This looks like XML. cache_check only checks the binary metadata format." << end_message(); return FATAL; + } superblock sb = read_superblock(bm); transaction_manager::ptr tm = open_tm(bm, SUPERBLOCK_LOCATION); diff --git a/caching/superblock.cc b/caching/superblock.cc index a76d8b0..500b5ff 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -365,7 +365,7 @@ caching::check_superblock(superblock const &sb, if (sb.magic != SUPERBLOCK_MAGIC) { ostringstream msg; - msg << "magic in incorrect: " << sb.magic; + msg << "magic is incorrect: " << sb.magic; visitor.visit(superblock_invalid(msg.str())); } diff --git a/era/era_check.cc b/era/era_check.cc index b640c3f..af51c26 100644 --- a/era/era_check.cc +++ b/era/era_check.cc @@ -14,6 +14,7 @@ #include "base/error_state.h" #include "base/error_string.h" +#include "base/file_utils.h" #include "base/nested_output.h" #include "era/commands.h" #include "era/writeset_tree.h" @@ -185,11 +186,19 @@ namespace { return info; } - error_state metadata_check(block_manager<>::ptr bm, flags const &fs) { + error_state metadata_check(string const &path, flags const &fs) { nested_output out(cerr, 2); if (fs.quiet_) out.disable(); + if (file_utils::get_file_length(path) < persistent_data::MD_BLOCK_SIZE) { + out << "Metadata device/file too small. Is this binary metadata?" + << end_message(); + return FATAL; + } + + block_manager<>::ptr bm = open_bm(path, block_manager<>::READ_ONLY); + superblock_reporter sb_rep(out); out << "examining superblock" << end_message(); @@ -198,8 +207,11 @@ namespace { check_superblock(bm, bm->get_nr_blocks(), sb_rep); } - if (sb_rep.get_error() == FATAL) + if (sb_rep.get_error() == FATAL) { + if (check_for_xml(bm)) + out << "This looks like XML. era_check only checks the binary metadata format." << end_message(); return FATAL; + } superblock sb = read_superblock(bm); transaction_manager::ptr tm = open_tm(bm, SUPERBLOCK_LOCATION); @@ -233,8 +245,7 @@ namespace { throw runtime_error(msg.str()); } - block_manager<>::ptr bm = open_bm(path, block_manager<>::READ_ONLY); - err = metadata_check(bm, fs); + err = metadata_check(path, fs); return err == NO_ERROR ? 0 : 1; } diff --git a/era/superblock.cc b/era/superblock.cc index 8ab0bec..7b10d83 100644 --- a/era/superblock.cc +++ b/era/superblock.cc @@ -283,7 +283,7 @@ era::check_superblock(superblock const &sb, if (sb.magic != SUPERBLOCK_MAGIC) { ostringstream msg; - msg << "magic in incorrect: " << sb.magic; + msg << "magic is incorrect: " << sb.magic; visitor.visit(superblock_invalid(msg.str())); } diff --git a/functional-tests/cache-functional-tests.scm b/functional-tests/cache-functional-tests.scm index f69b814..62d39d5 100644 --- a/functional-tests/cache-functional-tests.scm +++ b/functional-tests/cache-functional-tests.scm @@ -133,12 +133,18 @@ (cache-restore "-i" xml "-o" md "--debug-override-metadata-version" "12345") (run-fail "cache_check" md)))) - (define-scenario (cache-check dont-repair-xml) - "Fails gracefully if run on XML rather than metadata" + (define-scenario (cache-check tiny-metadata) + "Prints helpful message in case XML metadata given" (with-cache-xml (xml) - (with-empty-metadata (md) - (receive (_ stderr) (run-fail "cache_check " xml) - #t)))) + (receive (_ stderr) (run-fail "cache_check" xml) + (assert-starts-with "Metadata device/file too small. Is this binary metadata?" stderr)))) + + (define-scenario (cache-check spot-accidental-xml-data) + "Prints helpful message if XML metadata given" + (with-cache-xml (xml) + (system (fmt #f "man bash >> " xml)) + (receive (_ stderr) (run-fail "cache_check" xml) + (assert-matches ".*This looks like XML. cache_check only checks the binary metadata format." stderr)))) ;;;----------------------------------------------------------- ;;; cache_restore scenarios diff --git a/functional-tests/era-functional-tests.scm b/functional-tests/era-functional-tests.scm index efb874a..82409a0 100644 --- a/functional-tests/era-functional-tests.scm +++ b/functional-tests/era-functional-tests.scm @@ -107,6 +107,19 @@ (assert-eof stdout) (assert-eof stderr)))) + (define-scenario (era-check tiny-metadata) + "Prints helpful message in case XML metadata given" + (with-era-xml (xml) + (receive (_ stderr) (run-fail "era_check" xml) + (assert-starts-with "Metadata device/file too small. Is this binary metadata?" stderr)))) + + (define-scenario (era-check spot-accidental-xml-data) + "Prints helpful message if XML metadata given" + (with-era-xml (xml) + (system (fmt #f "man bash >> " xml)) + (receive (_ stderr) (run-fail "era_check" xml) + (assert-matches ".*This looks like XML. era_check only checks the binary metadata format." stderr)))) + ;;;----------------------------------------------------------- ;;; era_restore scenarios ;;;----------------------------------------------------------- diff --git a/persistent-data/file_utils.cc b/persistent-data/file_utils.cc index 38986fc..1743aff 100644 --- a/persistent-data/file_utils.cc +++ b/persistent-data/file_utils.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -17,6 +18,14 @@ using namespace std; //---------------------------------------------------------------- +bool +persistent_data::check_for_xml(block_manager<>::ptr bm) { + block_manager<>::read_ref b = bm->read_lock(0); + const char *data = reinterpret_cast(b.data()); + return (!strncmp(data, "::ptr bm); persistent_data::block_address get_nr_blocks(std::string const &path, sector_t block_size = MD_BLOCK_SIZE); block_address get_nr_metadata_blocks(std::string const &path); diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 5645f52..f3d30fd 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -190,12 +190,6 @@ namespace { return err; } - void check_for_xml(block_manager<>::ptr bm, nested_output &out) { - block_manager<>::read_ref b = bm->read_lock(superblock_detail::SUPERBLOCK_LOCATION); - if (!strncmp(reinterpret_cast(b.data()), "