From 8676198e76f0933fc82177629d2c67901a93119a Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 4 Mar 2016 10:43:58 +0000 Subject: [PATCH 1/3] [file_utils] rename get_nr_blocks() -> get_nr_metadata_blocks() and introduce get_nr_blocks() --- persistent-data/file_utils.cc | 18 +++++++++++++----- persistent-data/file_utils.h | 4 +++- thin-provisioning/thin_check.cc | 2 +- thin-provisioning/thin_rmap.cc | 2 +- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/persistent-data/file_utils.cc b/persistent-data/file_utils.cc index 2467079..4ed190e 100644 --- a/persistent-data/file_utils.cc +++ b/persistent-data/file_utils.cc @@ -8,11 +8,13 @@ #include using namespace base; +using namespace bcache; +using namespace persistent_data; //---------------------------------------------------------------- -persistent_data::block_address -persistent_data::get_nr_blocks(string const &path) +block_address +persistent_data::get_nr_blocks(string const &path, block_address block_size) { using namespace persistent_data; @@ -24,7 +26,7 @@ persistent_data::get_nr_blocks(string const &path) throw runtime_error("Couldn't stat dev path"); if (S_ISREG(info.st_mode) && info.st_size) - nr_blocks = div_up(info.st_size, MD_BLOCK_SIZE); + nr_blocks = div_down(info.st_size, block_size); else if (S_ISBLK(info.st_mode)) { // To get the size of a block device we need to @@ -39,7 +41,7 @@ persistent_data::get_nr_blocks(string const &path) throw runtime_error("ioctl BLKGETSIZE64 failed"); } ::close(fd); - nr_blocks = div_down(nr_blocks, MD_BLOCK_SIZE); + nr_blocks = div_down(nr_blocks, block_size); } else // FIXME: needs a better message throw runtime_error("bad path"); @@ -47,10 +49,16 @@ persistent_data::get_nr_blocks(string const &path) return nr_blocks; } +block_address +persistent_data::get_nr_metadata_blocks(string const &path) +{ + return get_nr_blocks(path, MD_BLOCK_SIZE); +} + persistent_data::block_manager<>::ptr persistent_data::open_bm(std::string const &dev_path, block_manager<>::mode m, bool excl) { - block_address nr_blocks = get_nr_blocks(dev_path); + block_address nr_blocks = get_nr_metadata_blocks(dev_path); return block_manager<>::ptr(new block_manager<>(dev_path, nr_blocks, 1, m, excl)); } diff --git a/persistent-data/file_utils.h b/persistent-data/file_utils.h index fcf203d..cb80bd4 100644 --- a/persistent-data/file_utils.h +++ b/persistent-data/file_utils.h @@ -9,7 +9,9 @@ // FIXME: move to a different unit namespace persistent_data { - persistent_data::block_address get_nr_blocks(string const &path); + block_address get_nr_blocks(string const &path, block_address block_size); + block_address get_nr_metadata_blocks(string const &path); + block_manager<>::ptr open_bm(std::string const &dev_path, block_manager<>::mode m, bool excl = true); diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 2451db2..4a3215c 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -43,7 +43,7 @@ using namespace thin_provisioning; namespace { block_manager<>::ptr open_bm(string const &path) { - block_address nr_blocks = get_nr_blocks(path); + block_address nr_blocks = get_nr_metadata_blocks(path); block_manager<>::mode m = block_manager<>::READ_ONLY; return block_manager<>::ptr(new block_manager<>(path, nr_blocks, 1, m)); } diff --git a/thin-provisioning/thin_rmap.cc b/thin-provisioning/thin_rmap.cc index 956fdf4..9f01255 100644 --- a/thin-provisioning/thin_rmap.cc +++ b/thin-provisioning/thin_rmap.cc @@ -23,7 +23,7 @@ using namespace thin_provisioning; namespace { block_manager<>::ptr open_bm(string const &path) { - block_address nr_blocks = get_nr_blocks(path); + block_address nr_blocks = get_nr_metadata_blocks(path); block_manager<>::mode m = block_manager<>::READ_ONLY; return block_manager<>::ptr(new block_manager<>(path, nr_blocks, 1, m)); } From 866986b883b9b1e4e68c16f10edd485ce28ea318 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 5 Apr 2016 12:27:00 +0100 Subject: [PATCH 2/3] [thin_trim] FInally get thin_trim emitting the right discards --- thin-provisioning/thin_trim.cc | 60 +++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/thin-provisioning/thin_trim.cc b/thin-provisioning/thin_trim.cc index ab6a897..0a8a3b0 100644 --- a/thin-provisioning/thin_trim.cc +++ b/thin-provisioning/thin_trim.cc @@ -22,17 +22,6 @@ using namespace thin_provisioning; //---------------------------------------------------------------- namespace { - void confirm_pool_is_not_active() { - cout << "The pool must *not* be active when running this tool.\n" - << "Do you wish to continue? [Y/N]\n" - << endl; - - string input; - cin >> input; - if (input != "Y") - exit(0); - } - class discard_emitter { public: discard_emitter(string const &data_dev, unsigned block_size, uint64_t nr_blocks) @@ -50,6 +39,8 @@ namespace { range[0] = block_to_byte(b); range[1] = block_to_byte(e) - range[0]; + cerr << "emitting discard for blocks (" << b << ", " << e << "]\n"; + if (ioctl(fd_, BLKDISCARD, &range)) throw runtime_error("discard ioctl failed"); } @@ -98,43 +89,57 @@ namespace { unsigned block_size_; }; - class trim_visitor : public space_map_detail::visitor { + class trim_iterator : public space_map::iterator { public: - trim_visitor(discard_emitter &e) + trim_iterator(discard_emitter &e) : emitter_(e) { } - virtual void visit(space_map_detail::missing_counts const &mc) { - throw std::runtime_error("corrupt metadata, please use thin_check for details"); + virtual void operator() (block_address b, ref_t count) { + highest_ = b; + + if (count) { + if (last_referenced_ && (b > *last_referenced_ + 1)) + emitter_.emit(*last_referenced_ + 1, b); + + last_referenced_ = b; + } } - virtual void visit(block_address b, uint32_t count) { - if (last_visited_ && (b > *last_visited_ + 1)) - emitter_.emit(*last_visited_ + 1, b); + void complete() { + if (last_referenced_) { + if (*last_referenced_ != *highest_) + emitter_.emit(*last_referenced_, *highest_ + 1ull); - last_visited_ = b; + } else if (highest_) + emitter_.emit(0ull, *highest_ + 1); } private: discard_emitter &emitter_; - boost::optional last_visited_; + boost::optional last_referenced_; + boost::optional highest_; }; int trim(string const &metadata_dev, string const &data_dev) { + cerr << "in trim\n"; + // We can trim any block that has zero count in the data // space map. block_manager<>::ptr bm = open_bm(metadata_dev, block_manager<>::READ_ONLY); metadata md(bm); - if (!md.data_sm_->get_nr_free()) + if (!md.data_sm_->get_nr_free()) { + cerr << "All data blocks allocated, nothing to discard\n"; return 0; + } discard_emitter de(data_dev, md.sb_.data_block_size_, md.data_sm_->get_nr_blocks()); - trim_visitor tv(de); + trim_iterator ti(de); - confirm_pool_is_not_active(); - md.data_sm_->visit(tv); + md.data_sm_->iterate(ti); + ti.complete(); return 0; } @@ -155,9 +160,8 @@ thin_trim_cmd::thin_trim_cmd() void thin_trim_cmd::usage(std::ostream &out) const { - out << "Usage: " << get_name() << " [options] {device|file}\n" + out << "Usage: " << get_name() << " [options] --metadata-dev {device|file} --data-dev {device|file}\n" << "Options:\n" - << " {--pool-inactive}\n" << " {-h|--help}\n" << " {-V|--version}" << endl; } @@ -188,6 +192,10 @@ thin_trim_cmd::run(int argc, char **argv) fs.data_dev = optarg; break; + case 2: + cerr << "--pool-inactive no longer required since we ensure the metadata device is opened exclusively.\n"; + break; + case 'h': usage(cout); return 0; From 7b1a11302103d5864180e98f65eeaf0d3dafbd6c Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 5 Apr 2016 13:06:26 +0100 Subject: [PATCH 3/3] update CHANGES --- CHANGES | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES b/CHANGES index 9a99f62..0703a2c 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,7 @@ v0.6.2 - Fix bug in thin_delta - Fix recent regression in thin_repair. - Force g++-98 dialect +- Fix bug in thin_trim v0.6.1 ======