From 52f03b7542b5bfe153d18944a8afd31363ad3171 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 25 Aug 2011 15:05:23 +0100 Subject: [PATCH] check metadata reference counts --- error_set.cc | 7 +++ error_set.h | 2 + metadata.cc | 52 ++++++++++------- metadata.h | 3 +- space_map_disk.h | 110 ++++++++++++++++++++++++++++++++++-- space_map_disk_structures.h | 2 +- 6 files changed, 146 insertions(+), 30 deletions(-) diff --git a/error_set.cc b/error_set.cc index 11bd012..54d0682 100644 --- a/error_set.cc +++ b/error_set.cc @@ -29,6 +29,13 @@ error_set::add_child(error_set::ptr err) children_.push_back(err); } +void +error_set::add_child(boost::optional maybe_errs) +{ + if (maybe_errs) + children_.push_back(*maybe_errs); +} + void error_set::add_child(string const &err) { diff --git a/error_set.h b/error_set.h index ad4893c..23b8cdc 100644 --- a/error_set.h +++ b/error_set.h @@ -1,6 +1,7 @@ #ifndef ERROR_SET_H #define ERROR_SET_H +#include #include #include #include @@ -22,6 +23,7 @@ namespace persistent_data { std::string const &get_description() const; std::list const &get_children() const; void add_child(error_set::ptr err); + void add_child(boost::optional maybe_errs); void add_child(std::string const &err); private: diff --git a/metadata.cc b/metadata.cc index 8bdbcbc..ec7fb43 100644 --- a/metadata.cc +++ b/metadata.cc @@ -182,6 +182,7 @@ thin::set_mapped_blocks(block_address count) metadata::metadata(std::string const &dev_path) : tm_(open_tm(dev_path)), sb_(read_superblock(tm_->get_bm())), + metadata_sm_(open_metadata_sm(tm_, static_cast(&sb_.metadata_space_map_root_))), data_sm_(open_disk_sm(tm_, static_cast(&sb_.data_space_map_root_))), details_(tm_, sb_.device_details_root_, device_details_traits::ref_counter()), mappings_top_level_(tm_, sb_.data_mapping_root_, mtree_ref_counter(tm_)), @@ -326,6 +327,32 @@ metadata::device_exists(thin_dev_t dev) const return details_.lookup(key); } +namespace { + // FIXME: this doesn't check for non-zero counts in the sm that are + // actually zero. + optional + check_ref_counts(string const &desc, block_counter const &actual, + space_map::ptr sm) { + error_set::ptr errors(new error_set(desc)); + + bool bad = false; + block_counter::count_map const &counts = actual.get_counts(); + block_counter::count_map::const_iterator it, end = counts.end(); + for (it = counts.begin(); it != end; ++it) { + uint32_t ref_count = sm->get_count(it->first); + if (ref_count != it->second) { + ostringstream out; + out << it->first << ": was " << ref_count + << ", expected " << it->second; + errors->add_child(out.str()); + bad = true; + } + } + + return bad ? optional(errors) : optional(); + } +} + boost::optional metadata::check() { @@ -351,27 +378,10 @@ metadata::check() } data_sm_->check(metadata_counter); - - { - error_set::ptr data_errors(new error_set("Errors in data reference counts")); - - bool bad = false; - block_counter::count_map const &data_counts = data_counter.get_counts(); - for (block_counter::count_map::const_iterator it = data_counts.begin(); - it != data_counts.end(); ++it) { - uint32_t ref_count = data_sm_->get_count(it->first); - if (ref_count != it->second) { - ostringstream out; - out << it->first << ": was " << ref_count - << ", expected " << it->second; - data_errors->add_child(out.str()); - bad = true; - } - } - - if (bad) - errors->add_child(data_errors); - } + errors->add_child(check_ref_counts("Errors in metadata block reference counts", + metadata_counter, metadata_sm_)); + errors->add_child(check_ref_counts("Errors in data block reference counts", + data_counter, data_sm_)); return (errors->get_children().size() > 0) ? optional(errors) : diff --git a/metadata.h b/metadata.h index dc5a280..51c5b9d 100644 --- a/metadata.h +++ b/metadata.h @@ -192,8 +192,7 @@ namespace thin_provisioning { tm_ptr tm_; superblock sb_; - // Ignoring the metadata sm for now, since we don't need it for the basic 'dump' tool - // space_map::ptr metadata_sm_; + space_map::ptr metadata_sm_; sm_disk_detail::sm_disk::ptr data_sm_; detail_tree details_; dev_tree mappings_top_level_; diff --git a/space_map_disk.h b/space_map_disk.h index 0523f85..a5f3648 100644 --- a/space_map_disk.h +++ b/space_map_disk.h @@ -175,6 +175,7 @@ namespace persistent_data { } void commit() { + commit_ies(); } void inc(block_address b) { @@ -252,9 +253,10 @@ namespace persistent_data { private: virtual index_entry find_ie(block_address b) const = 0; virtual void save_ie(block_address b, struct index_entry ie) = 0; + virtual void commit_ies() = 0; ref_t lookup_bitmap(block_address b) const { - index_entry ie = find_ie(b); + index_entry ie = find_ie(b / entries_per_block_); bitmap bm(tm_, ie); return bm.lookup(b % entries_per_block_); } @@ -263,7 +265,7 @@ namespace persistent_data { if (n > 3) throw runtime_error("bitmap can only hold 2 bit values"); - index_entry ie = find_ie(b); + index_entry ie = find_ie(b / entries_per_block_); bitmap bm(tm_, ie); bm.insert(b % entries_per_block_, n); save_ie(b, bm.get_ie()); @@ -346,8 +348,8 @@ namespace persistent_data { } private: - index_entry find_ie(block_address b) const { - uint64_t key[1] = {b / sm_disk_base::get_entries_per_block()}; + index_entry find_ie(block_address ie_index) const { + uint64_t key[1] = {ie_index}; optional mindex = bitmaps_.lookup(key); if (!mindex) throw runtime_error("Couldn't lookup bitmap"); @@ -355,13 +357,93 @@ namespace persistent_data { return *mindex; } - void save_ie(block_address b, struct index_entry ie) { - uint64_t key[1] = {b / sm_disk_base::get_entries_per_block()}; + void save_ie(block_address ie_index, struct index_entry ie) { + uint64_t key[1] = {ie_index}; bitmaps_.insert(key, ie); } + void commit_ies() { + } + btree<1, index_entry_traits, BlockSize> bitmaps_; }; + + template + class sm_metadata : public sm_disk_base { + public: + typedef boost::shared_ptr > ptr; + + sm_metadata(typename transaction_manager::ptr tm) + : sm_disk_base(tm), + entries_(MAX_METADATA_BITMAPS) { + // FIXME: allocate a new bitmap root + } + + sm_metadata(typename transaction_manager::ptr tm, + sm_root const &root) + : sm_disk_base(tm, root), + bitmap_root_(root.bitmap_root_), + entries_(MAX_METADATA_BITMAPS) { + load_ies(); + } + + size_t root_size() { + return sizeof(sm_root_disk); + } + + // FIXME: common code + void copy_root(void *dest, size_t len) { + sm_root_disk d; + sm_root v; + + if (len < sizeof(d)) + throw runtime_error("root too small"); + + v.nr_blocks_ = sm_disk_base::get_nr_blocks(); + v.nr_allocated_ = sm_disk_base::get_nr_allocated(); + v.bitmap_root_ = bitmap_root_; + v.ref_count_root_ = sm_disk_base::get_ref_count_root(); + sm_root_traits::pack(v, d); + ::memcpy(dest, &d, sizeof(d)); + } + + private: + index_entry find_ie(block_address ie_index) const { + return entries_[ie_index]; + } + + void save_ie(block_address ie_index, struct index_entry ie) { + entries_[ie_index] = ie; + } + + void load_ies() { + typename block_manager::read_ref rr = + sm_disk_base::get_tm()->read_lock(bitmap_root_); + + metadata_index const *mdi = reinterpret_cast(&rr.data()); + + for (unsigned i = 0; i < MAX_METADATA_BITMAPS; i++) + index_entry_traits::unpack(*(mdi->index + i), entries_[i]); + } + + void commit_ies() { + std::pair::write_ref, bool> p = + sm_disk_base::get_tm()->shadow(bitmap_root_); + + bitmap_root_ = p.first.get_location(); + metadata_index *mdi = reinterpret_cast(&p.first.data()); + + mdi->csum_ = to_disk<__le32, uint32_t>(0); + mdi->padding_ = to_disk<__le32, uint32_t>(0); + mdi->blocknr_ = to_disk<__le64>(bitmap_root_); + + for (unsigned i = 0; i < entries_.size(); i++) + index_entry_traits::pack(entries_[i], mdi->index[i]); + } + + block_address bitmap_root_; + std::vector entries_; + }; } template @@ -391,6 +473,22 @@ namespace persistent_data { return typename sm_disk::ptr( new sm_disk(tm, v)); } + + template + typename sm_disk_detail::sm_metadata::ptr + open_metadata_sm(typename transaction_manager::ptr tm, + void * root) + { + using namespace sm_disk_detail; + + sm_root_disk d; + sm_root v; + + ::memcpy(&d, root, sizeof(d)); + sm_root_traits::unpack(d, v); + return typename sm_metadata::ptr( + new sm_metadata(tm, v)); + } } //---------------------------------------------------------------- diff --git a/space_map_disk_structures.h b/space_map_disk_structures.h index f36a1b4..9896783 100644 --- a/space_map_disk_structures.h +++ b/space_map_disk_structures.h @@ -48,7 +48,7 @@ namespace persistent_data { __le32 padding_; __le64 blocknr_; - struct index_entry index[MAX_METADATA_BITMAPS]; + struct index_entry_disk index[MAX_METADATA_BITMAPS]; } __attribute__ ((packed)); struct sm_root_disk {