From ce8945e82962076b4a111ae5a592794df01e82ba Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Fri, 26 Jun 2020 23:04:15 +0800 Subject: [PATCH 1/9] [io_generator] Fix address boundary for doing IO Limit the addresses to be within the specified range --- base/io_generator.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/base/io_generator.cc b/base/io_generator.cc index a808a6c..4d6ebb0 100644 --- a/base/io_generator.cc +++ b/base/io_generator.cc @@ -48,8 +48,10 @@ namespace { base::sector_t next_offset() { sector_t r = current_; current_ += block_size_; - if (current_ > end_) + if (current_ > end_) { current_ = begin_; + return begin_; + } return r; } From 4f8466c48952481fbe5ef38ea6975b90976e7742 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Tue, 30 Jun 2020 14:07:38 +0800 Subject: [PATCH 2/9] [btree] Fix parent key index for the new shadow --- persistent-data/data-structures/btree-remove.tcc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/persistent-data/data-structures/btree-remove.tcc b/persistent-data/data-structures/btree-remove.tcc index d222273..9d3317e 100644 --- a/persistent-data/data-structures/btree-remove.tcc +++ b/persistent-data/data-structures/btree-remove.tcc @@ -93,7 +93,7 @@ namespace persistent_data { { using namespace btree_detail; - unsigned i = 0; + unsigned i = *index; bool r = false; for (;;) { From cf127f3471242dea7ba60bfdaf7948a298bf5999 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Tue, 30 Jun 2020 16:42:30 +0800 Subject: [PATCH 3/9] [btree] Fix reference counts of children below a shadow --- persistent-data/data-structures/btree-remove.tcc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/persistent-data/data-structures/btree-remove.tcc b/persistent-data/data-structures/btree-remove.tcc index 9d3317e..852d267 100644 --- a/persistent-data/data-structures/btree-remove.tcc +++ b/persistent-data/data-structures/btree-remove.tcc @@ -94,10 +94,11 @@ namespace persistent_data { using namespace btree_detail; unsigned i = *index; - bool r = false; for (;;) { - r = spine.step(block); + bool inc = spine.step(block); + if (inc) + inc_children(spine, leaf_rc); // patch up the parent to point to the new shadow if (spine.has_parent()) { @@ -115,9 +116,9 @@ namespace persistent_data { return true; } - r = rebalance_children(spine, key); + bool r = rebalance_children(spine, key); if (!r) - break; + return false; n = spine.get_node(); if (n.get_type() == btree_detail::LEAF) { @@ -133,7 +134,7 @@ namespace persistent_data { block = n.value_at(i); } - return r; + return true; } template From a407c831df4d26d25e751dac44dce07536f91762 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 2 Jul 2020 10:51:10 +0100 Subject: [PATCH 4/9] [thin_repair] Fix segfault with metadata containing loops (bz1853241) 'seen' bit was being set too late. --- thin-provisioning/metadata_dumper.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/thin-provisioning/metadata_dumper.cc b/thin-provisioning/metadata_dumper.cc index 0dd43c6..2ceffcc 100644 --- a/thin-provisioning/metadata_dumper.cc +++ b/thin-provisioning/metadata_dumper.cc @@ -614,8 +614,8 @@ namespace { return it->second; } else { - node_info info = get_info_(b); examined_[b] = true; + node_info info = get_info_(b); if (!failed(info)) infos_.insert(make_pair(b, info)); From fec11289b09f57fe539d02b2d27fcecbbfaf887e Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 2 Jul 2020 16:03:23 +0100 Subject: [PATCH 5/9] [thin_check/dump] Under populated nodes are now 'non fatal errors' thin_dump always ignores non-fatal-errors. --- .../data-structures/btree_damage_visitor.h | 22 +++++++--- thin-provisioning/device_tree.cc | 11 +++-- thin-provisioning/device_tree.h | 6 ++- thin-provisioning/mapping_tree.cc | 30 ++++++++------ thin-provisioning/mapping_tree.h | 25 +++++++---- thin-provisioning/metadata_checker.cc | 41 +++++++++++-------- thin-provisioning/metadata_checker.h | 2 + thin-provisioning/metadata_dumper.cc | 16 ++++---- thin-provisioning/thin_check.cc | 1 + 9 files changed, 98 insertions(+), 56 deletions(-) diff --git a/persistent-data/data-structures/btree_damage_visitor.h b/persistent-data/data-structures/btree_damage_visitor.h index b57c553..7eaa8d2 100644 --- a/persistent-data/data-structures/btree_damage_visitor.h +++ b/persistent-data/data-structures/btree_damage_visitor.h @@ -106,10 +106,12 @@ namespace persistent_data { typedef boost::optional maybe_run64; btree_damage_visitor(ValueVisitor &value_visitor, - DamageVisitor &damage_visitor) + DamageVisitor &damage_visitor, + bool ignore_non_fatal) : avoid_repeated_visits_(true), value_visitor_(value_visitor), - damage_visitor_(damage_visitor) { + damage_visitor_(damage_visitor), + ignore_non_fatal_(ignore_non_fatal) { } bool visit_internal(node_location const &loc, @@ -178,7 +180,7 @@ namespace persistent_data { else if (!checker_.check_block_nr(n) || !checker_.check_value_size(n) || !checker_.check_max_entries(n) || - !checker_.check_nr_entries(n, loc.is_sub_root()) || + !check_nr_entries(loc, n) || !checker_.check_ordered_keys(n) || !checker_.check_parent_key(n, loc.is_sub_root() ? boost::optional() : loc.key)) { report_damage(checker_.get_last_error_string()); @@ -202,7 +204,7 @@ namespace persistent_data { else if (!checker_.check_block_nr(n) || !checker_.check_value_size(n) || !checker_.check_max_entries(n) || - !checker_.check_nr_entries(n, loc.is_sub_root()) || + !check_nr_entries(loc, n) || !checker_.check_ordered_keys(n) || !checker_.check_parent_key(n, loc.is_sub_root() ? boost::optional() : loc.key) || !checker_.check_leaf_key(n, last_leaf_key_[loc.level()])) { @@ -300,6 +302,12 @@ namespace persistent_data { maybe_issue_damage(*old_path); } + template + bool check_nr_entries(node_location const &loc, + btree_detail::node_ref const &n) { + return ignore_non_fatal_ || checker_.check_nr_entries(n, loc.is_sub_root()); + } + //-------------------------------- bool avoid_repeated_visits_; @@ -314,15 +322,17 @@ namespace persistent_data { path_tracker path_tracker_; damage_tracker dt_; std::list damage_reasons_; + bool ignore_non_fatal_; }; } template void btree_visit_values(btree const &tree, ValueVisitor &value_visitor, - DamageVisitor &damage_visitor) { + DamageVisitor &damage_visitor, + bool ignore_non_fatal = false) { btree_detail::btree_damage_visitor - v(value_visitor, damage_visitor); + v(value_visitor, damage_visitor, ignore_non_fatal); tree.visit_depth_first(v); } } diff --git a/thin-provisioning/device_tree.cc b/thin-provisioning/device_tree.cc index 4b4e0dd..f0d92a7 100644 --- a/thin-provisioning/device_tree.cc +++ b/thin-provisioning/device_tree.cc @@ -96,18 +96,21 @@ namespace thin_provisioning { void thin_provisioning::walk_device_tree(device_tree const &tree, device_tree_detail::device_visitor &vv, - device_tree_detail::damage_visitor &dv) + device_tree_detail::damage_visitor &dv, + bool ignore_non_fatal) { visitor_adapter av(vv); ll_damage_visitor ll_dv(dv); - btree_visit_values(tree, av, ll_dv); + btree_visit_values(tree, av, ll_dv, ignore_non_fatal); } void -thin_provisioning::check_device_tree(device_tree const &tree, damage_visitor &visitor) +thin_provisioning::check_device_tree(device_tree const &tree, + damage_visitor &visitor, + bool ignore_non_fatal) { noop_visitor vv; - walk_device_tree(tree, vv, visitor); + walk_device_tree(tree, vv, visitor, ignore_non_fatal); } //---------------------------------------------------------------- diff --git a/thin-provisioning/device_tree.h b/thin-provisioning/device_tree.h index d7178cd..5bfefda 100644 --- a/thin-provisioning/device_tree.h +++ b/thin-provisioning/device_tree.h @@ -75,9 +75,11 @@ namespace thin_provisioning { void walk_device_tree(device_tree const &tree, device_tree_detail::device_visitor &dev_v, - device_tree_detail::damage_visitor &dv); + device_tree_detail::damage_visitor &dv, + bool ignore_non_fatal = false); void check_device_tree(device_tree const &tree, - device_tree_detail::damage_visitor &visitor); + device_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal = false); } //---------------------------------------------------------------- diff --git a/thin-provisioning/mapping_tree.cc b/thin-provisioning/mapping_tree.cc index f23ec70..a73dc3a 100644 --- a/thin-provisioning/mapping_tree.cc +++ b/thin-provisioning/mapping_tree.cc @@ -228,54 +228,60 @@ namespace { void thin_provisioning::walk_mapping_tree(dev_tree const &tree, mapping_tree_detail::device_visitor &dev_v, - mapping_tree_detail::damage_visitor &dv) + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal) { dev_tree_damage_visitor ll_dv(dv); - btree_visit_values(tree, dev_v, ll_dv); + btree_visit_values(tree, dev_v, ll_dv, ignore_non_fatal); } void thin_provisioning::check_mapping_tree(dev_tree const &tree, - mapping_tree_detail::damage_visitor &visitor) + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal) { noop_block_visitor dev_v; - walk_mapping_tree(tree, dev_v, visitor); + walk_mapping_tree(tree, dev_v, visitor, ignore_non_fatal); } void thin_provisioning::walk_mapping_tree(mapping_tree const &tree, mapping_tree_detail::mapping_visitor &mv, - mapping_tree_detail::damage_visitor &dv) + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal) { mapping_tree_damage_visitor ll_dv(dv); - btree_visit_values(tree, mv, ll_dv); + btree_visit_values(tree, mv, ll_dv, ignore_non_fatal); } void thin_provisioning::check_mapping_tree(mapping_tree const &tree, - mapping_tree_detail::damage_visitor &visitor) + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal) { noop_block_time_visitor mv; - walk_mapping_tree(tree, mv, visitor); + walk_mapping_tree(tree, mv, visitor, ignore_non_fatal); } void thin_provisioning::walk_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, mapping_tree_detail::mapping_visitor &mv, - mapping_tree_detail::damage_visitor &dv) + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal) { single_mapping_tree_damage_visitor ll_dv(dv, dev_id); - btree_visit_values(tree, mv, ll_dv); + btree_visit_values(tree, mv, ll_dv, ignore_non_fatal); } void thin_provisioning::check_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, - mapping_tree_detail::damage_visitor &visitor) + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal) { noop_block_time_visitor mv; - walk_mapping_tree(tree, dev_id, mv, visitor); + walk_mapping_tree(tree, dev_id, mv, visitor, ignore_non_fatal); } //---------------------------------------------------------------- diff --git a/thin-provisioning/mapping_tree.h b/thin-provisioning/mapping_tree.h index 22ad514..7545b74 100644 --- a/thin-provisioning/mapping_tree.h +++ b/thin-provisioning/mapping_tree.h @@ -129,23 +129,32 @@ namespace thin_provisioning { void walk_mapping_tree(dev_tree const &tree, mapping_tree_detail::device_visitor &dev_v, - mapping_tree_detail::damage_visitor &dv); - void check_mapping_tree(dev_tree const &tree, - mapping_tree_detail::damage_visitor &visitor); + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal = false); void walk_mapping_tree(mapping_tree const &tree, mapping_tree_detail::mapping_visitor &mv, - mapping_tree_detail::damage_visitor &dv); - void check_mapping_tree(mapping_tree const &tree, - mapping_tree_detail::damage_visitor &visitor); + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal = false); void walk_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, mapping_tree_detail::mapping_visitor &mv, - mapping_tree_detail::damage_visitor &dv); + mapping_tree_detail::damage_visitor &dv, + bool ignore_non_fatal = false); + void check_mapping_tree(single_mapping_tree const &tree, uint64_t dev_id, - mapping_tree_detail::damage_visitor &visitor); + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal = false); + + void check_mapping_tree(dev_tree const &tree, + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal = false); + + void check_mapping_tree(mapping_tree const &tree, + mapping_tree_detail::damage_visitor &visitor, + bool ignore_non_fatal = false); } //---------------------------------------------------------------- diff --git a/thin-provisioning/metadata_checker.cc b/thin-provisioning/metadata_checker.cc index ed3d20d..7a619c9 100644 --- a/thin-provisioning/metadata_checker.cc +++ b/thin-provisioning/metadata_checker.cc @@ -150,28 +150,30 @@ namespace { error_state examine_devices_tree_(transaction_manager::ptr tm, superblock_detail::superblock const &sb, - nested_output &out) { + nested_output &out, + bool ignore_non_fatal) { out << "examining devices tree" << end_message(); nested_output::nest _ = out.push(); devices_reporter dev_rep(out); device_tree dtree(*tm, sb.device_details_root_, device_tree_detail::device_details_traits::ref_counter()); - check_device_tree(dtree, dev_rep); + check_device_tree(dtree, dev_rep, ignore_non_fatal); return dev_rep.get_error(); } error_state examine_top_level_mapping_tree_(transaction_manager::ptr tm, superblock_detail::superblock const &sb, - nested_output &out) { + nested_output &out, + bool ignore_non_fatal) { out << "examining top level of mapping tree" << end_message(); nested_output::nest _ = out.push(); mapping_reporter mapping_rep(out); dev_tree dtree(*tm, sb.data_mapping_root_, mapping_tree_detail::mtree_traits::ref_counter(*tm)); - check_mapping_tree(dtree, mapping_rep); + check_mapping_tree(dtree, mapping_rep, ignore_non_fatal); return mapping_rep.get_error(); } @@ -179,7 +181,8 @@ namespace { error_state examine_mapping_tree_(transaction_manager::ptr tm, superblock_detail::superblock const &sb, nested_output &out, - optional data_sm) { + optional data_sm, + bool ignore_non_fatal) { out << "examining mapping tree" << end_message(); nested_output::nest _ = out.push(); @@ -189,18 +192,19 @@ namespace { if (data_sm) { data_ref_counter dcounter(*data_sm); - walk_mapping_tree(mtree, dcounter, mapping_rep); + walk_mapping_tree(mtree, dcounter, mapping_rep, ignore_non_fatal); } else - check_mapping_tree(mtree, mapping_rep); + check_mapping_tree(mtree, mapping_rep, ignore_non_fatal); return mapping_rep.get_error(); } error_state examine_top_level_mapping_tree(transaction_manager::ptr tm, superblock_detail::superblock const &sb, - nested_output &out) { - error_state err = examine_devices_tree_(tm, sb, out); - err << examine_top_level_mapping_tree_(tm, sb, out); + nested_output &out, + bool ignore_non_fatal) { + error_state err = examine_devices_tree_(tm, sb, out, ignore_non_fatal); + err << examine_top_level_mapping_tree_(tm, sb, out, ignore_non_fatal); return err; } @@ -208,9 +212,10 @@ namespace { error_state examine_mapping_tree(transaction_manager::ptr tm, superblock_detail::superblock const &sb, nested_output &out, - optional data_sm) { - error_state err = examine_devices_tree_(tm, sb, out); - err << examine_mapping_tree_(tm, sb, out, data_sm); + optional data_sm, + bool ignore_non_fatal) { + error_state err = examine_devices_tree_(tm, sb, out, ignore_non_fatal); + err << examine_mapping_tree_(tm, sb, out, data_sm, ignore_non_fatal); return err; } @@ -351,7 +356,7 @@ namespace { } private: - static error_state + error_state examine_data_mappings(transaction_manager::ptr tm, superblock_detail::superblock const &sb, check_options::data_mapping_options option, @@ -361,10 +366,10 @@ namespace { switch (option) { case check_options::DATA_MAPPING_LEVEL1: - err << examine_top_level_mapping_tree(tm, sb, out); + err << examine_top_level_mapping_tree(tm, sb, out, options_.ignore_non_fatal_); break; case check_options::DATA_MAPPING_LEVEL2: - err << examine_mapping_tree(tm, sb, out, data_sm); + err << examine_mapping_tree(tm, sb, out, data_sm, options_.ignore_non_fatal_); break; default: break; // do nothing @@ -424,6 +429,10 @@ void check_options::set_metadata_snap() { use_metadata_snap_ = true; sm_opts_ = SPACE_MAP_NONE; } + +void check_options::set_ignore_non_fatal() { + ignore_non_fatal_ = true; +} base::error_state thin_provisioning::check_metadata(block_manager::ptr bm, diff --git a/thin-provisioning/metadata_checker.h b/thin-provisioning/metadata_checker.h index 1b94d7e..05f36cc 100644 --- a/thin-provisioning/metadata_checker.h +++ b/thin-provisioning/metadata_checker.h @@ -44,11 +44,13 @@ namespace thin_provisioning { void set_skip_mappings(); void set_override_mapping_root(bcache::block_address b); void set_metadata_snap(); + void set_ignore_non_fatal(); bool use_metadata_snap_; data_mapping_options check_data_mappings_; space_map_options sm_opts_; boost::optional override_mapping_root_; + bool ignore_non_fatal_; }; enum output_options { diff --git a/thin-provisioning/metadata_dumper.cc b/thin-provisioning/metadata_dumper.cc index 2ceffcc..4feb40f 100644 --- a/thin-provisioning/metadata_dumper.cc +++ b/thin-provisioning/metadata_dumper.cc @@ -132,7 +132,7 @@ namespace { auto tree = device_tree(tm, root, device_tree_detail::device_details_traits::ref_counter()); try { - walk_device_tree(tree, de, dv); + walk_device_tree(tree, de, dv, true); } catch (...) { return optional>(); } @@ -157,7 +157,7 @@ namespace { auto tree = dev_tree(tm, root, mapping_tree_detail::mtree_traits::ref_counter(tm)); try { - walk_mapping_tree(tree, me, mv); + walk_mapping_tree(tree, me, mv, true); } catch (...) { return optional>(); } @@ -754,7 +754,7 @@ namespace { // Since we're not mutating the btrees we don't need a real space map noop_map::ptr sm(new noop_map); single_mapping_tree tree(tm_, subtree_root, mapping_tree_detail::block_time_ref_counter(sm)); - walk_mapping_tree(tree, dev_id, static_cast(me), *damage_policy_); + walk_mapping_tree(tree, dev_id, static_cast(me), *damage_policy_, true); } dump_options const &opts_; @@ -785,7 +785,7 @@ namespace { dump_options opts; details_extractor de(opts); device_tree_detail::damage_visitor::ptr dd_policy(details_damage_policy(true)); - walk_device_tree(*md.details_, de, *dd_policy); + walk_device_tree(*md.details_, de, *dd_policy, true); e->begin_superblock("", sb.time_, sb.trans_id_, @@ -798,7 +798,7 @@ namespace { { mapping_tree_detail::damage_visitor::ptr md_policy(mapping_damage_policy(true)); mapping_tree_emit_visitor mte(opts, *md.tm_, e, de.get_details(), mapping_damage_policy(true)); - walk_mapping_tree(*md.mappings_top_level_, mte, *md_policy); + walk_mapping_tree(*md.mappings_top_level_, mte, *md_policy, true); } e->end_superblock(); @@ -882,7 +882,7 @@ thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e, dump_options { details_extractor de(opts); device_tree_detail::damage_visitor::ptr dd_policy(details_damage_policy(false)); - walk_device_tree(*md->details_, de, *dd_policy); + walk_device_tree(*md->details_, de, *dd_policy, true); e->begin_superblock("", md->sb_.time_, md->sb_.trans_id_, @@ -895,7 +895,7 @@ thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e, dump_options { mapping_tree_detail::damage_visitor::ptr md_policy(mapping_damage_policy(false)); mapping_tree_emit_visitor mte(opts, *md->tm_, e, de.get_details(), mapping_damage_policy(false)); - walk_mapping_tree(*md->mappings_top_level_, mte, *md_policy); + walk_mapping_tree(*md->mappings_top_level_, mte, *md_policy, true); } e->end_superblock(); @@ -924,7 +924,7 @@ thin_provisioning::metadata_dump_subtree(metadata::ptr md, emitter::ptr e, bool mapping_tree_detail::block_time_ref_counter(md->data_sm_)); // FIXME: pass the current device id instead of zero walk_mapping_tree(tree, 0, static_cast(me), - *mapping_damage_policy(repair)); + *mapping_damage_policy(repair), true); } //---------------------------------------------------------------- diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 05f5582..26599c6 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -174,6 +174,7 @@ thin_check_cmd::run(int argc, char **argv) case 3: // ignore-non-fatal-errors fs.ignore_non_fatal_errors = true; + fs.check_opts.set_ignore_non_fatal(); break; case 4: From 9c45d830d90f7ea833b7ec738880c3271f1379b5 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 3 Jul 2020 12:08:01 +0100 Subject: [PATCH 6/9] bump version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 262b51c..2752650 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.9.0-rc1 +0.9.0-rc2 From 73d26393ab660b804d09aae002d0283ceb1109b7 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 3 Jul 2020 13:13:59 +0100 Subject: [PATCH 7/9] [functional-tests] bump version in help strings --- functional-tests/scenario-string-constants.scm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/functional-tests/scenario-string-constants.scm b/functional-tests/scenario-string-constants.scm index 90b0507..a1779e8 100644 --- a/functional-tests/scenario-string-constants.scm +++ b/functional-tests/scenario-string-constants.scm @@ -76,7 +76,7 @@ Options: {-V|--version}") (define thin-metadata-pack-help - "thin_metadata_pack 0.9.0-rc1 + "thin_metadata_pack 0.9.0-rc2 Produces a compressed file of thin metadata. Only packs metadata blocks that are actually used. USAGE: @@ -91,7 +91,7 @@ OPTIONS: -o Specify packed output file") (define thin-metadata-unpack-help - "thin_metadata_unpack 0.9.0-rc1 + "thin_metadata_unpack 0.9.0-rc2 Unpack a compressed file of thin metadata. USAGE: From 23f3033f61c806422bbee9170c37bacaec887ace Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 3 Jul 2020 13:28:02 +0100 Subject: [PATCH 8/9] [functional-tests] bump version nr --- functional-tests/thin-functional-tests.scm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/functional-tests/thin-functional-tests.scm b/functional-tests/thin-functional-tests.scm index dbabb10..d5e5d00 100644 --- a/functional-tests/thin-functional-tests.scm +++ b/functional-tests/thin-functional-tests.scm @@ -511,7 +511,7 @@ (define-scenario (thin-metadata-pack version) "accepts --version" (run-ok-rcv (stdout _) (thin-metadata-pack "--version") - (assert-equal "thin_metadata_pack 0.9.0-rc1" stdout))) + (assert-equal "thin_metadata_pack 0.9.0-rc2" stdout))) (define-scenario (thin-metadata-pack h) "accepts -h" @@ -553,7 +553,7 @@ (define-scenario (thin-metadata-unpack version) "accepts --version" (run-ok-rcv (stdout _) (thin-metadata-unpack "--version") - (assert-equal "thin_metadata_unpack 0.9.0-rc1" stdout))) + (assert-equal "thin_metadata_unpack 0.9.0-rc2" stdout))) (define-scenario (thin-metadata-unpack h) "accepts -h" From b7e02d0ae4639fb7d83e05774d549dd29089e919 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 9 Jul 2020 13:52:53 +0100 Subject: [PATCH 9/9] [thin_metadata_pack] FIx bug where pack would try and read too much. --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + src/pack/toplevel.rs | 21 +++++++++------------ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0ca6d78..25d5f5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,6 +21,11 @@ dependencies = [ "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "anyhow" +version = "1.0.31" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "arrayvec" version = "0.4.12" @@ -380,6 +385,7 @@ dependencies = [ name = "thinp" version = "0.1.0" dependencies = [ + "anyhow 1.0.31 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)", "clap 2.33.1 (registry+https://github.com/rust-lang/crates.io-index)", "crc32c 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -456,6 +462,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum adler32 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "5d2e7343e7fc9de883d1b0341e0b13970f764c14101234857d2ddafa1cb1cac2" "checksum aho-corasick 0.7.10 (registry+https://github.com/rust-lang/crates.io-index)" = "8716408b8bc624ed7f65d223ddb9ac2d044c0547b6fa4b0d554f3a9540496ada" "checksum ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b" +"checksum anyhow 1.0.31 (registry+https://github.com/rust-lang/crates.io-index)" = "85bb70cc08ec97ca5450e6eba421deeea5f172c0fc61f78b5357b2a8e8be195f" "checksum arrayvec 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)" = "cd9fd44efafa8690358b7408d253adf110036b88f55672a933f01d616ad9b1b9" "checksum atty 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)" = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8" "checksum autocfg 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f8aac770f1885fd7e387acedd76065302551364496e46b3dd00860b2f8359b9d" diff --git a/Cargo.toml b/Cargo.toml index e1e1c7c..dadedba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ edition = "2018" license = "GPL3" [dependencies] +anyhow = "1.0" byteorder = "1.3" clap = "2.33" crc32c = "0.4" diff --git a/src/pack/toplevel.rs b/src/pack/toplevel.rs index bd1757d..76f9bf6 100644 --- a/src/pack/toplevel.rs +++ b/src/pack/toplevel.rs @@ -1,3 +1,4 @@ +use anyhow::Result; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use flate2::{read::ZlibDecoder, write::ZlibEncoder, Compression}; @@ -34,11 +35,6 @@ fn shuffle(v: &mut Vec) { v.shuffle(&mut rng); } -// FIXME: move to a utils module -fn div_up(n: u64, d: u64) -> u64 { - (n + d - 1) / d -} - // Each thread processes multiple contiguous runs of blocks, called // chunks. Chunks are shuffled so each thread gets chunks spread // across the dev in case there are large regions that don't contain @@ -47,12 +43,17 @@ fn mk_chunk_vecs(nr_blocks: u64, nr_jobs: u64) -> Vec> { use std::cmp::{max, min}; let chunk_size = min(4 * 1024u64, max(128u64, nr_blocks / (nr_jobs * 64))); - let nr_chunks = div_up(nr_blocks, chunk_size); + let nr_chunks = nr_blocks / chunk_size; let mut chunks = Vec::with_capacity(nr_chunks as usize); for i in 0..nr_chunks { chunks.push((i * chunk_size, (i + 1) * chunk_size)); } + // there may be a smaller chunk at the back of the file. + if nr_chunks * chunk_size < nr_blocks { + chunks.push((nr_chunks * chunk_size, nr_blocks)); + } + shuffle(&mut chunks); let mut vs = Vec::with_capacity(nr_jobs as usize); @@ -104,11 +105,7 @@ pub fn pack(input_file: &str, output_file: &str) -> Result<(), Box> { Ok(()) } -fn crunch( - input: Arc>, - output: Arc>, - ranges: Vec<(u64, u64)>, -) -> io::Result<()> +fn crunch(input: Arc>, output: Arc>, ranges: Vec<(u64, u64)>) -> Result<()> where R: Read + Seek, W: Write, @@ -256,7 +253,7 @@ fn pack_block(w: &mut W, kind: BT, buf: &[u8]) { BT::NODE => check(&pack_btree_node(w, buf)), BT::INDEX => check(&pack_index(w, buf)), BT::BITMAP => check(&pack_bitmap(w, buf)), - BT::UNKNOWN => {panic!("asked to pack an unknown block type")} + BT::UNKNOWN => panic!("asked to pack an unknown block type"), } }