From 77286e0bc754a7ab2e6cbc6210b7d38b82923f40 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 15 Aug 2013 10:35:07 +0100 Subject: [PATCH 01/80] Take out some 'typename's to support older versions of gcc (Heinz). --- persistent-data/block.tcc | 2 +- persistent-data/data-structures/btree.h | 8 ++++---- .../space-maps/subtracting_span_iterator.h | 2 +- thin-provisioning/metadata.cc | 2 +- thin-provisioning/metadata_dumper.cc | 2 +- thin-provisioning/thin_check.cc | 15 +++++++-------- thin-provisioning/thin_dump.cc | 3 ++- thin-provisioning/thin_rmap.cc | 2 +- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/persistent-data/block.tcc b/persistent-data/block.tcc index f392b5a..36e872d 100644 --- a/persistent-data/block.tcc +++ b/persistent-data/block.tcc @@ -61,7 +61,7 @@ namespace { } bool file_exists(string const &path) { - typename ::stat info; + struct ::stat info; int r = ::stat(path.c_str(), &info); if (r) { diff --git a/persistent-data/data-structures/btree.h b/persistent-data/data-structures/btree.h index 0dd9f37..30a3577 100644 --- a/persistent-data/data-structures/btree.h +++ b/persistent-data/data-structures/btree.h @@ -213,7 +213,7 @@ namespace persistent_data { class ro_spine : private boost::noncopyable { public: ro_spine(transaction_manager::ptr tm, - typename block_manager<>::validator::ptr v) + block_manager<>::validator::ptr v) : tm_(tm), validator_(v) { } @@ -227,7 +227,7 @@ namespace persistent_data { private: transaction_manager::ptr tm_; - typename block_manager<>::validator::ptr validator_; + block_manager<>::validator::ptr validator_; std::list::read_ref> spine_; }; @@ -237,7 +237,7 @@ namespace persistent_data { typedef transaction_manager::write_ref write_ref; shadow_spine(transaction_manager::ptr tm, - typename block_manager<>::validator::ptr v) + block_manager<>::validator::ptr v) : tm_(tm), validator_(v) { @@ -287,7 +287,7 @@ namespace persistent_data { private: transaction_manager::ptr tm_; - typename block_manager<>::validator::ptr validator_; + block_manager<>::validator::ptr validator_; std::list::write_ref> spine_; block_address root_; }; diff --git a/persistent-data/space-maps/subtracting_span_iterator.h b/persistent-data/space-maps/subtracting_span_iterator.h index 92b8fdf..8d83188 100644 --- a/persistent-data/space-maps/subtracting_span_iterator.h +++ b/persistent-data/space-maps/subtracting_span_iterator.h @@ -11,7 +11,7 @@ namespace persistent_data { class subtracting_span_iterator : public space_map::span_iterator { public: - typedef typename base::run_set block_set; + typedef base::run_set block_set; typedef space_map::span span; subtracting_span_iterator(block_address max, diff --git a/thin-provisioning/metadata.cc b/thin-provisioning/metadata.cc index 23317db..00832bb 100644 --- a/thin-provisioning/metadata.cc +++ b/thin-provisioning/metadata.cc @@ -42,7 +42,7 @@ namespace { block_manager<>::ptr open_bm(string const &dev_path, bool writeable) { block_address nr_blocks = get_nr_blocks(dev_path); - typename block_io<>::mode m = writeable ? + block_io<>::mode m = writeable ? block_io<>::READ_WRITE : block_io<>::READ_ONLY; diff --git a/thin-provisioning/metadata_dumper.cc b/thin-provisioning/metadata_dumper.cc index aa12cfc..a611354 100644 --- a/thin-provisioning/metadata_dumper.cc +++ b/thin-provisioning/metadata_dumper.cc @@ -139,7 +139,7 @@ namespace { class details_extractor : public btree<1, device_tree_detail::device_details_traits>::visitor { public: - typedef typename btree<1, device_tree_detail::device_details_traits>::visitor::node_location node_location; + typedef btree<1, device_tree_detail::device_details_traits>::visitor::node_location node_location; typedef boost::shared_ptr ptr; typedef btree_checker<1, device_tree_detail::device_details_traits> checker; diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index 517d85d..e0ea2c1 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -144,7 +144,7 @@ namespace { block_manager<>::ptr open_bm(string const &path) { block_address nr_blocks = get_nr_blocks(path); - typename block_io<>::mode m = block_io<>::READ_ONLY; + block_io<>::mode m = block_io<>::READ_ONLY; return block_manager<>::ptr(new block_manager<>(path, nr_blocks, 1, m)); } @@ -344,13 +344,12 @@ namespace { int main(int argc, char **argv) { int c; - flags fs = { - .check_device_tree = true, - .check_mapping_tree_level1 = true, - .check_mapping_tree_level2 = true, - .ignore_non_fatal_errors = false, - .quiet = false - }; + flags fs; + fs.check_device_tree = true; + fs.check_mapping_tree_level1 = true, + fs.check_mapping_tree_level2 = true, + fs.ignore_non_fatal_errors = false, + fs.quiet = false; char const shortopts[] = "qhV"; option const longopts[] = { diff --git a/thin-provisioning/thin_dump.cc b/thin-provisioning/thin_dump.cc index 6b3e910..d2c8950 100644 --- a/thin-provisioning/thin_dump.cc +++ b/thin-provisioning/thin_dump.cc @@ -94,7 +94,8 @@ int main(int argc, char **argv) char *end_ptr; string format = "xml"; block_address metadata_snap = 0; - struct flags flags = { .find_metadata_snap = false, .repair = false }; + struct flags flags; + flags.find_metadata_snap = flags.repair = false; const struct option longopts[] = { { "help", no_argument, NULL, 'h'}, diff --git a/thin-provisioning/thin_rmap.cc b/thin-provisioning/thin_rmap.cc index 96f2a63..f58682d 100644 --- a/thin-provisioning/thin_rmap.cc +++ b/thin-provisioning/thin_rmap.cc @@ -23,7 +23,7 @@ namespace { block_manager<>::ptr open_bm(string const &path) { block_address nr_blocks = get_nr_blocks(path); - typename block_io<>::mode m = block_io<>::READ_ONLY; + block_io<>::mode m = block_io<>::READ_ONLY; return block_manager<>::ptr(new block_manager<>(path, nr_blocks, 1, m)); } From 500e508c6db2d8c138078661c4c6504269dd18d1 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 15 Aug 2013 16:26:17 +0100 Subject: [PATCH 02/80] fix up some coverity issues --- persistent-data/block.h | 1 - persistent-data/cache.h | 24 +++++++++++++++++++----- persistent-data/data-structures/btree.h | 8 ++++++-- persistent-data/hex_dump.cc | 5 ++++- thin-provisioning/metadata_dumper.cc | 1 + thin-provisioning/restore_emitter.cc | 1 + thin-provisioning/superblock.cc | 2 ++ thin-provisioning/thin_check.cc | 3 ++- thin-provisioning/thin_rmap.cc | 9 ++++++++- 9 files changed, 43 insertions(+), 11 deletions(-) diff --git a/persistent-data/block.h b/persistent-data/block.h index a39da3d..f7020d2 100644 --- a/persistent-data/block.h +++ b/persistent-data/block.h @@ -65,7 +65,6 @@ namespace persistent_data { int fd_; block_address nr_blocks_; mode mode_; - bool writeable_; }; template diff --git a/persistent-data/cache.h b/persistent-data/cache.h index b13e864..6c4e660 100644 --- a/persistent-data/cache.h +++ b/persistent-data/cache.h @@ -68,15 +68,29 @@ namespace base { v_(v) { } - struct { - value_entry *next_, *prev_; - } lru_; + struct lru { + lru() + : next_(0), + prev_(0) { + } + + value_entry *next_, *prev_; + }; + + struct lookup { + lookup() + : parent_(0), + left_(0), + right_(0), + color_() { + } - struct { value_entry *parent_, *left_, *right_; int color_; - } lookup_; + }; + lru lru_; + lookup lookup_; unsigned ref_count_; value_type v_; }; diff --git a/persistent-data/data-structures/btree.h b/persistent-data/data-structures/btree.h index 30a3577..26e687a 100644 --- a/persistent-data/data-structures/btree.h +++ b/persistent-data/data-structures/btree.h @@ -235,6 +235,7 @@ namespace persistent_data { public: typedef transaction_manager::read_ref read_ref; typedef transaction_manager::write_ref write_ref; + typedef boost::optional maybe_block; shadow_spine(transaction_manager::ptr tm, block_manager<>::validator::ptr v) @@ -282,14 +283,17 @@ namespace persistent_data { } block_address get_root() const { - return root_; + if (root_) + return *root_; + + throw std::runtime_error("shadow spine has no root"); } private: transaction_manager::ptr tm_; block_manager<>::validator::ptr validator_; std::list::write_ref> spine_; - block_address root_; + maybe_block root_; }; // Used to keep a record of a nested btree's position. diff --git a/persistent-data/hex_dump.cc b/persistent-data/hex_dump.cc index 353ed33..ec3380e 100644 --- a/persistent-data/hex_dump.cc +++ b/persistent-data/hex_dump.cc @@ -29,6 +29,8 @@ void base::hex_dump(ostream &out, void const *data_, size_t len) { unsigned char const *data = reinterpret_cast(data_), *end = data + len; + + ios_base::fmtflags old_flags = out.flags(); out << hex; while (data < end) { @@ -36,7 +38,8 @@ void base::hex_dump(ostream &out, void const *data_, size_t len) out << setw(2) << setfill('0') << (unsigned) *data << " "; out << endl; } - out << dec; + + out.setf(old_flags); } //---------------------------------------------------------------- diff --git a/thin-provisioning/metadata_dumper.cc b/thin-provisioning/metadata_dumper.cc index a611354..8ade304 100644 --- a/thin-provisioning/metadata_dumper.cc +++ b/thin-provisioning/metadata_dumper.cc @@ -40,6 +40,7 @@ namespace { md_sm_(md_sm), data_sm_(data_sm), in_range_(false), + time_(), found_errors_(false) { } diff --git a/thin-provisioning/restore_emitter.cc b/thin-provisioning/restore_emitter.cc index b1fe870..fd1d4ab 100644 --- a/thin-provisioning/restore_emitter.cc +++ b/thin-provisioning/restore_emitter.cc @@ -32,6 +32,7 @@ namespace { restorer(metadata::ptr md) : md_(md), in_superblock_(false), + nr_data_blocks_(), empty_mapping_(new_mapping_tree()) { } diff --git a/thin-provisioning/superblock.cc b/thin-provisioning/superblock.cc index 130e70d..b89ac64 100644 --- a/thin-provisioning/superblock.cc +++ b/thin-provisioning/superblock.cc @@ -37,6 +37,7 @@ superblock_traits::unpack(superblock_disk const &disk, superblock &value) value.metadata_nr_blocks_ = to_cpu(disk.metadata_nr_blocks_); value.compat_flags_ = to_cpu(disk.compat_flags_); + value.compat_ro_flags_ = to_cpu(disk.compat_ro_flags_); value.incompat_flags_ = to_cpu(disk.incompat_flags_); } @@ -70,6 +71,7 @@ superblock_traits::pack(superblock const &value, superblock_disk &disk) disk.metadata_nr_blocks_ = to_disk(value.metadata_nr_blocks_); disk.compat_flags_ = to_disk(value.compat_flags_); + disk.compat_ro_flags_ = to_disk(value.compat_ro_flags_); disk.incompat_flags_ = to_disk(value.incompat_flags_); } diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index e0ea2c1..b5826df 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -216,7 +216,8 @@ namespace { class mapping_reporter : public mapping_tree_detail::damage_visitor { public: mapping_reporter(nested_output &out) - : out_(out) { + : out_(out), + err_(NO_ERROR) { } virtual void visit(mapping_tree_detail::missing_devices const &d) { diff --git a/thin-provisioning/thin_rmap.cc b/thin-provisioning/thin_rmap.cc index f58682d..3b90c52 100644 --- a/thin-provisioning/thin_rmap.cc +++ b/thin-provisioning/thin_rmap.cc @@ -150,7 +150,14 @@ int main(int argc, char **argv) case 1: // region - regions.push_back(parse_region(optarg)); + try { + regions.push_back(parse_region(optarg)); + + } catch (std::exception const &e) { + cerr << e.what(); + return 1; + } + break; default: From 1e925d4cf1cbddd4632ca902b03b475a5681d194 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 13:39:12 +0100 Subject: [PATCH 03/80] caching/superblock.{h,cc} --- Makefile.in | 8 +- cache/metadata_disk_structures.h | 120 --------------- {cache => caching}/cache_metadata.h | 0 {cache => caching}/check.cc | 0 {cache => caching}/dump.cc | 0 caching/metadata_disk_structures.cc | 28 ++++ caching/metadata_disk_structures.h | 35 +++++ .../superblock.cc | 108 +++++++++++--- caching/superblock.h | 139 ++++++++++++++++++ 9 files changed, 292 insertions(+), 146 deletions(-) delete mode 100644 cache/metadata_disk_structures.h rename {cache => caching}/cache_metadata.h (100%) rename {cache => caching}/check.cc (100%) rename {cache => caching}/dump.cc (100%) create mode 100644 caching/metadata_disk_structures.cc create mode 100644 caching/metadata_disk_structures.h rename cache/metadata_disk_structures.cc => caching/superblock.cc (59%) create mode 100644 caching/superblock.h diff --git a/Makefile.in b/Makefile.in index 505166d..0d19c7f 100644 --- a/Makefile.in +++ b/Makefile.in @@ -48,7 +48,7 @@ PDATA_SOURCE=\ SOURCE=\ $(PDATA_SOURCE) \ \ - cache/metadata_disk_structures.cc \ + caching/superblock.cc \ \ thin-provisioning/device_tree.cc \ thin-provisioning/file_utils.cc \ @@ -66,7 +66,7 @@ SOURCE=\ PDATA_OBJECTS=$(subst .cc,.o,$(SOURCE)) CXX_PROGRAM_SOURCE=\ - cache/check.cc \ + caching/check.cc \ \ thin-provisioning/thin_check.cc \ thin-provisioning/thin_dump.cc \ @@ -228,11 +228,11 @@ CACHE_CHECK_SOURCE=\ persistent-data/space-maps/recursive.cc \ persistent-data/space-maps/careful_alloc.cc \ persistent-data/transaction_manager.cc \ - cache/metadata_disk_structures.cc + caching/superblock.cc CACHE_CHECK_OBJECTS=$(subst .cc,.o,$(CACHE_CHECK_SOURCE)) -cache_check: $(CACHE_CHECK_OBJECTS) cache/check.o +cache_check: $(CACHE_CHECK_OBJECTS) caching/check.o @echo " [LD] $@" $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) diff --git a/cache/metadata_disk_structures.h b/cache/metadata_disk_structures.h deleted file mode 100644 index fe6d261..0000000 --- a/cache/metadata_disk_structures.h +++ /dev/null @@ -1,120 +0,0 @@ -// Copyright (C) 2012 Red Hat, Inc. All rights reserved. -// -// This file is part of the thin-provisioning-tools source. -// -// thin-provisioning-tools is free software: you can redistribute it -// and/or modify it under the terms of the GNU General Public License -// as published by the Free Software Foundation, either version 3 of -// the License, or (at your option) any later version. -// -// thin-provisioning-tools is distributed in the hope that it will be -// useful, but WITHOUT ANY WARRANTY; without even the implied warranty -// of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License along -// with thin-provisioning-tools. If not, see -// . - -#ifndef CACHE_METADATA_DISK_STRUCTURES_H -#define CACHE_METADATA_DISK_STRUCTURES_H - -#include "persistent-data/endian_utils.h" -#include "persistent-data/data-structures/btree.h" - -//---------------------------------------------------------------- - -// FIXME: rename to just METADATA_DISK_STRUCTURES -namespace cache_tools { - using namespace base; // FIXME: don't use namespaces in headers. - - unsigned const SPACE_MAP_ROOT_SIZE = 128; - unsigned const CACHE_POLICY_NAME_SIZE = 16; - unsigned const CACHE_POLICY_VERSION_SIZE = 3; - - typedef unsigned char __u8; - - struct superblock_disk { - le32 csum; - le32 flags; - le64 blocknr; - - __u8 uuid[16]; - le64 magic; - le32 version; - - __u8 policy_name[CACHE_POLICY_NAME_SIZE]; - le32 policy_version[CACHE_POLICY_VERSION_SIZE]; - le32 policy_hint_size; - - __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; - - le64 mapping_root; - le64 hint_root; - - le64 discard_root; - le64 discard_block_size; - le64 discard_nr_blocks; - - le32 data_block_size; /* in 512-byte sectors */ - le32 metadata_block_size; /* in 512-byte sectors */ - le32 cache_blocks; - - le32 compat_flags; - le32 compat_ro_flags; - le32 incompat_flags; - - le32 read_hits; - le32 read_misses; - le32 write_hits; - le32 write_misses; - } __attribute__ ((packed)); - - struct superblock { - uint32_t csum; - uint32_t flags; - uint64_t blocknr; - - __u8 uuid[16]; - uint64_t magic; - uint32_t version; - - __u8 policy_name[CACHE_POLICY_NAME_SIZE]; - uint32_t policy_version[CACHE_POLICY_VERSION_SIZE]; - uint32_t policy_hint_size; - - __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; - - uint64_t mapping_root; - uint64_t hint_root; - - uint64_t discard_root; - uint64_t discard_block_size; - uint64_t discard_nr_blocks; - - uint32_t data_block_size; /* in 512-byte sectors */ - uint32_t metadata_block_size; /* in 512-byte sectors */ - uint32_t cache_blocks; - - uint32_t compat_flags; - uint32_t compat_ro_flags; - uint32_t incompat_flags; - - uint32_t read_hits; - uint32_t read_misses; - uint32_t write_hits; - uint32_t write_misses; - }; - - struct superblock_traits { - typedef superblock_disk disk_type; - typedef superblock value_type; - - static void unpack(superblock_disk const &disk, superblock &value); - static void pack(superblock const &value, superblock_disk &disk); - }; -} - -//---------------------------------------------------------------- - -#endif diff --git a/cache/cache_metadata.h b/caching/cache_metadata.h similarity index 100% rename from cache/cache_metadata.h rename to caching/cache_metadata.h diff --git a/cache/check.cc b/caching/check.cc similarity index 100% rename from cache/check.cc rename to caching/check.cc diff --git a/cache/dump.cc b/caching/dump.cc similarity index 100% rename from cache/dump.cc rename to caching/dump.cc diff --git a/caching/metadata_disk_structures.cc b/caching/metadata_disk_structures.cc new file mode 100644 index 0000000..8face77 --- /dev/null +++ b/caching/metadata_disk_structures.cc @@ -0,0 +1,28 @@ +// Copyright (C) 2012 Red Hat, Inc. All rights reserved. +// +// This file is part of the thin-provisioning-tools source. +// +// thin-provisioning-tools is free software: you can redistribute it +// and/or modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation, either version 3 of +// the License, or (at your option) any later version. +// +// thin-provisioning-tools is distributed in the hope that it will be +// useful, but WITHOUT ANY WARRANTY; without even the implied warranty +// of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with thin-provisioning-tools. If not, see +// . + +#include "metadata_disk_structures.h" + +#include + +using namespace cache_tools; + +//---------------------------------------------------------------- + + +//---------------------------------------------------------------- diff --git a/caching/metadata_disk_structures.h b/caching/metadata_disk_structures.h new file mode 100644 index 0000000..ef2363e --- /dev/null +++ b/caching/metadata_disk_structures.h @@ -0,0 +1,35 @@ +// Copyright (C) 2012 Red Hat, Inc. All rights reserved. +// +// This file is part of the thin-provisioning-tools source. +// +// thin-provisioning-tools is free software: you can redistribute it +// and/or modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation, either version 3 of +// the License, or (at your option) any later version. +// +// thin-provisioning-tools is distributed in the hope that it will be +// useful, but WITHOUT ANY WARRANTY; without even the implied warranty +// of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with thin-provisioning-tools. If not, see +// . + +#ifndef CACHE_METADATA_DISK_STRUCTURES_H +#define CACHE_METADATA_DISK_STRUCTURES_H + +#include "persistent-data/endian_utils.h" +#include "persistent-data/data-structures/btree.h" + +//---------------------------------------------------------------- + +// FIXME: rename to just METADATA_DISK_STRUCTURES +namespace cache_tools { + using namespace base; // FIXME: don't use namespaces in headers. + +} + +//---------------------------------------------------------------- + +#endif diff --git a/cache/metadata_disk_structures.cc b/caching/superblock.cc similarity index 59% rename from cache/metadata_disk_structures.cc rename to caching/superblock.cc index d822fbd..4a8b153 100644 --- a/cache/metadata_disk_structures.cc +++ b/caching/superblock.cc @@ -1,26 +1,7 @@ -// Copyright (C) 2012 Red Hat, Inc. All rights reserved. -// -// This file is part of the thin-provisioning-tools source. -// -// thin-provisioning-tools is free software: you can redistribute it -// and/or modify it under the terms of the GNU General Public License -// as published by the Free Software Foundation, either version 3 of -// the License, or (at your option) any later version. -// -// thin-provisioning-tools is distributed in the hope that it will be -// useful, but WITHOUT ANY WARRANTY; without even the implied warranty -// of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License along -// with thin-provisioning-tools. If not, see -// . +#include "caching/superblock.h" -#include "metadata_disk_structures.h" - -#include - -using namespace cache_tools; +using namespace caching; +using namespace superblock_detail; //---------------------------------------------------------------- @@ -110,4 +91,87 @@ superblock_traits::pack(superblock const &core, superblock_disk &disk) disk.write_misses = to_disk(core.write_misses); } +//-------------------------------- + +superblock_corruption::superblock_corruption(std::string const &desc) + : desc_(desc) +{ +} + +void +superblock_corruption::visit(damage_visitor &v) const +{ + v.visit(*this); +} + +//-------------------------------- + +namespace { + using namespace persistent_data; + using namespace superblock_detail; + + uint32_t const VERSION = 1; + unsigned const SECTOR_TO_BLOCK_SHIFT = 3; + uint32_t const SUPERBLOCK_CSUM_SEED = 9031977; + + struct sb_validator : public block_manager<>::validator { + virtual void check(buffer<> const &b, block_address location) const { + superblock_disk const *sbd = reinterpret_cast(&b); + crc32c sum(SUPERBLOCK_CSUM_SEED); + sum.append(&sbd->flags, MD_BLOCK_SIZE - sizeof(uint32_t)); + if (sum.get_sum() != to_cpu(sbd->csum)) + throw checksum_error("bad checksum in superblock"); + } + + virtual void prepare(buffer<> &b, block_address location) const { + superblock_disk *sbd = reinterpret_cast(&b); + crc32c sum(SUPERBLOCK_CSUM_SEED); + sum.append(&sbd->flags, MD_BLOCK_SIZE - sizeof(uint32_t)); + sbd->csum = to_disk(sum.get_sum()); + } + }; +} + +persistent_data::block_manager<>::validator::ptr +caching::superblock_validator() +{ + return block_manager<>::validator::ptr(new sb_validator); +} + +//-------------------------------- + +superblock_detail::superblock +caching::read_superblock(persistent_data::block_manager<>::ptr bm, + persistent_data::block_address location) +{ + using namespace superblock_detail; + + superblock sb; + block_manager<>::read_ref r = bm->read_lock(location, superblock_validator()); + superblock_disk const *sbd = reinterpret_cast(&r.data()); + superblock_traits::unpack(*sbd, sb); + + return sb; +} + +superblock_detail::superblock +caching::read_superblock(persistent_data::block_manager<>::ptr bm) +{ + return read_superblock(bm, SUPERBLOCK_LOCATION); +} + +void +caching::check_superblock(persistent_data::block_manager<>::ptr bm, + superblock_detail::damage_visitor &visitor) +{ + using namespace superblock_detail; + + try { + bm->read_lock(SUPERBLOCK_LOCATION, superblock_validator()); + + } catch (std::exception const &e) { + visitor.visit(superblock_corruption(e.what())); + } +} + //---------------------------------------------------------------- diff --git a/caching/superblock.h b/caching/superblock.h new file mode 100644 index 0000000..f6b9a0c --- /dev/null +++ b/caching/superblock.h @@ -0,0 +1,139 @@ +#ifndef CACHE_SUPERBLOCK_H +#define CACHE_SUPERBLOCK_H + +#include "persistent-data/endian_utils.h" +#include "persistent-data/data-structures/btree.h" + +//---------------------------------------------------------------- + +namespace caching { + namespace superblock_detail { + using namespace base; + + unsigned const SPACE_MAP_ROOT_SIZE = 128; + unsigned const CACHE_POLICY_NAME_SIZE = 16; + unsigned const CACHE_POLICY_VERSION_SIZE = 3; + + typedef unsigned char __u8; + + struct superblock_disk { + le32 csum; + le32 flags; + le64 blocknr; + + __u8 uuid[16]; + le64 magic; + le32 version; + + __u8 policy_name[CACHE_POLICY_NAME_SIZE]; + le32 policy_version[CACHE_POLICY_VERSION_SIZE]; + le32 policy_hint_size; + + __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; + + le64 mapping_root; + le64 hint_root; + + le64 discard_root; + le64 discard_block_size; + le64 discard_nr_blocks; + + le32 data_block_size; /* in 512-byte sectors */ + le32 metadata_block_size; /* in 512-byte sectors */ + le32 cache_blocks; + + le32 compat_flags; + le32 compat_ro_flags; + le32 incompat_flags; + + le32 read_hits; + le32 read_misses; + le32 write_hits; + le32 write_misses; + } __attribute__ ((packed)); + + struct superblock { + uint32_t csum; + uint32_t flags; + uint64_t blocknr; + + __u8 uuid[16]; + uint64_t magic; + uint32_t version; + + __u8 policy_name[CACHE_POLICY_NAME_SIZE]; + uint32_t policy_version[CACHE_POLICY_VERSION_SIZE]; + uint32_t policy_hint_size; + + __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; + + uint64_t mapping_root; + uint64_t hint_root; + + uint64_t discard_root; + uint64_t discard_block_size; + uint64_t discard_nr_blocks; + + uint32_t data_block_size; /* in 512-byte sectors */ + uint32_t metadata_block_size; /* in 512-byte sectors */ + uint32_t cache_blocks; + + uint32_t compat_flags; + uint32_t compat_ro_flags; + uint32_t incompat_flags; + + uint32_t read_hits; + uint32_t read_misses; + uint32_t write_hits; + uint32_t write_misses; + }; + + struct superblock_traits { + typedef superblock_disk disk_type; + typedef superblock value_type; + + static void unpack(superblock_disk const &disk, superblock &value); + static void pack(superblock const &value, superblock_disk &disk); + }; + + block_address const SUPERBLOCK_LOCATION = 0; + uint32_t const SUPERBLOCK_MAGIC = 06142003; + + //-------------------------------- + + class damage_visitor; + + struct damage { + virtual ~damage() {} + virtual void visit(damage_visitor &v) const = 0; + }; + + struct superblock_corruption : public damage { + superblock_corruption(std::string const &desc); + void visit(damage_visitor &v) const; + + std::string desc_; + }; + + class damage_visitor { + public: + virtual ~damage_visitor() {} + + void visit(damage const &d); + + virtual void visit(superblock_corruption const &d) = 0; + }; + } + + persistent_data::block_manager<>::validator::ptr superblock_validator(); + + superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm); + superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm, + persistent_data::block_address location); + void check_superblock(persistent_data::block_manager<>::ptr bm, + superblock_detail::damage_visitor &visitor); +} + +//---------------------------------------------------------------- + +#endif From 94bd3aef3b48de7ed7a77bf21d90576630bad1a7 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 13:48:25 +0100 Subject: [PATCH 04/80] Put nested_output in it's own file --- base/nested_output.h | 94 +++++++++++++++++++++++++++++++++ thin-provisioning/thin_check.cc | 85 +---------------------------- 2 files changed, 95 insertions(+), 84 deletions(-) create mode 100644 base/nested_output.h diff --git a/base/nested_output.h b/base/nested_output.h new file mode 100644 index 0000000..7bb6947 --- /dev/null +++ b/base/nested_output.h @@ -0,0 +1,94 @@ +#ifndef BASE_NESTED_OUTPUT_H +#define BASE_NESTED_OUTPUT_H + +#include + +//---------------------------------------------------------------- + +namespace base { + class end_message {}; + + class nested_output { + public: + nested_output(std::ostream &out, unsigned step) + : out_(out), + step_(step), + beginning_of_line_(true), + enabled_(true), + indent_(0) { + } + + template + nested_output &operator <<(T const &t) { + if (beginning_of_line_) { + beginning_of_line_ = false; + indent(); + } + + if (enabled_) + out_ << t; + + return *this; + } + + nested_output &operator <<(end_message const &m) { + beginning_of_line_ = true; + + if (enabled_) + out_ << std::endl; + + return *this; + } + + void inc_indent() { + indent_ += step_; + } + + void dec_indent() { + indent_ -= step_; + } + + struct nest { + nest(nested_output &out) + : out_(out) { + out_.inc_indent(); + } + + ~nest() { + out_.dec_indent(); + } + + nested_output &out_; + }; + + nest push() { + return nest(*this); + } + + void enable() { + enabled_ = true; + } + + void disable() { + enabled_ = false; + } + + private: + void indent() { + if (enabled_) + for (unsigned i = 0; i < indent_; i++) + out_ << ' '; + } + + std::ostream &out_; + unsigned step_; + + bool beginning_of_line_; + bool enabled_; + unsigned indent_; + }; +} + +//---------------------------------------------------------------- + +#endif diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index b5826df..bf27e70 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -22,6 +22,7 @@ #include "version.h" +#include "base/nested_output.h" #include "persistent-data/space-maps/core.h" #include "thin-provisioning/device_tree.h" #include "thin-provisioning/file_utils.h" @@ -35,90 +36,6 @@ using namespace thin_provisioning; namespace { - class end_message {}; - - class nested_output { - public: - nested_output(ostream &out, unsigned step) - : out_(out), - step_(step), - beginning_of_line_(true), - enabled_(true), - indent_(0) { - } - - template - nested_output &operator <<(T const &t) { - if (beginning_of_line_) { - beginning_of_line_ = false; - indent(); - } - - if (enabled_) - out_ << t; - - return *this; - } - - nested_output &operator <<(end_message const &m) { - beginning_of_line_ = true; - - if (enabled_) - out_ << endl; - - return *this; - } - - void inc_indent() { - indent_ += step_; - } - - void dec_indent() { - indent_ -= step_; - } - - struct nest { - nest(nested_output &out) - : out_(out) { - out_.inc_indent(); - } - - ~nest() { - out_.dec_indent(); - } - - nested_output &out_; - }; - - nest push() { - return nest(*this); - } - - void enable() { - enabled_ = true; - } - - void disable() { - enabled_ = false; - } - - private: - void indent() { - if (enabled_) - for (unsigned i = 0; i < indent_; i++) - out_ << ' '; - } - - ostream &out_; - unsigned step_; - - bool beginning_of_line_; - bool enabled_; - unsigned indent_; - }; - - //-------------------------------- - enum error_state { NO_ERROR, NON_FATAL, // eg, lost blocks From 67551d81f12c175562d49236d7abd391530581bf Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 14:07:04 +0100 Subject: [PATCH 05/80] Put error_state into into it's own file. --- Makefile.in | 17 ++++++++--------- base/error_state.cc | 19 +++++++++++++++++++ base/error_state.h | 18 ++++++++++++++++++ caching/{check.cc => cache_check.cc} | 0 thin-provisioning/thin_check.cc | 25 +++---------------------- 5 files changed, 48 insertions(+), 31 deletions(-) create mode 100644 base/error_state.cc create mode 100644 base/error_state.h rename caching/{check.cc => cache_check.cc} (100%) diff --git a/Makefile.in b/Makefile.in index 0d19c7f..4dc2555 100644 --- a/Makefile.in +++ b/Makefile.in @@ -29,7 +29,11 @@ PROGRAMS=\ all: $(PROGRAMS) -PDATA_SOURCE=\ +SOURCE=\ + base/error_state.cc \ + \ + caching/superblock.cc \ + \ persistent-data/checksum.cc \ persistent-data/endian_utils.cc \ persistent-data/error_set.cc \ @@ -42,13 +46,7 @@ PDATA_SOURCE=\ persistent-data/space_map.cc \ persistent-data/space-maps/disk.cc \ persistent-data/space-maps/recursive.cc \ - persistent-data/space-maps/careful_alloc.cc -#PDATA_OBJECTS=$(subst .cc,.o,$(PDATA_SOURCE)) - -SOURCE=\ - $(PDATA_SOURCE) \ - \ - caching/superblock.cc \ + persistent-data/space-maps/careful_alloc.cc \ \ thin-provisioning/device_tree.cc \ thin-provisioning/file_utils.cc \ @@ -66,7 +64,7 @@ SOURCE=\ PDATA_OBJECTS=$(subst .cc,.o,$(SOURCE)) CXX_PROGRAM_SOURCE=\ - caching/check.cc \ + caching/cache_check.cc \ \ thin-provisioning/thin_check.cc \ thin-provisioning/thin_dump.cc \ @@ -140,6 +138,7 @@ THIN_DUMP_SOURCE=$(SOURCE) THIN_REPAIR_SOURCE=$(SOURCE) THIN_RESTORE_SOURCE=$(SOURCE) THIN_CHECK_SOURCE=\ + base/error_state.cc \ persistent-data/checksum.cc \ persistent-data/endian_utils.cc \ persistent-data/error_set.cc \ diff --git a/base/error_state.cc b/base/error_state.cc new file mode 100644 index 0000000..fdb899e --- /dev/null +++ b/base/error_state.cc @@ -0,0 +1,19 @@ +#include "base/error_state.h" + +//---------------------------------------------------------------- + +base::error_state +base::combine_errors(error_state lhs, error_state rhs) { + switch (lhs) { + case NO_ERROR: + return rhs; + + case NON_FATAL: + return (rhs == FATAL) ? FATAL : lhs; + + default: + return lhs; + } +} + +//---------------------------------------------------------------- diff --git a/base/error_state.h b/base/error_state.h new file mode 100644 index 0000000..9864f6a --- /dev/null +++ b/base/error_state.h @@ -0,0 +1,18 @@ +#ifndef BASE_ERROR_STATE_H +#define BASE_ERROR_STATE_H + +//---------------------------------------------------------------- + +namespace base { + enum error_state { + NO_ERROR, + NON_FATAL, // eg, lost blocks + FATAL // needs fixing before pool can be activated + }; + + error_state combine_errors(error_state lhs, error_state rhs); +} + +//---------------------------------------------------------------- + +#endif diff --git a/caching/check.cc b/caching/cache_check.cc similarity index 100% rename from caching/check.cc rename to caching/cache_check.cc diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index bf27e70..acca39b 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -22,6 +22,7 @@ #include "version.h" +#include "base/error_state.h" #include "base/nested_output.h" #include "persistent-data/space-maps/core.h" #include "thin-provisioning/device_tree.h" @@ -29,33 +30,13 @@ #include "thin-provisioning/mapping_tree.h" #include "thin-provisioning/superblock.h" +using namespace base; using namespace std; using namespace thin_provisioning; //---------------------------------------------------------------- namespace { - - enum error_state { - NO_ERROR, - NON_FATAL, // eg, lost blocks - FATAL // needs fixing before pool can be activated - }; - - error_state - combine_errors(error_state lhs, error_state rhs) { - switch (lhs) { - case NO_ERROR: - return rhs; - - case NON_FATAL: - return (rhs == FATAL) ? FATAL : lhs; - - default: - return lhs; - } - } - //-------------------------------- block_manager<>::ptr @@ -91,7 +72,7 @@ namespace { err_ = combine_errors(err_, FATAL); } - error_state get_error() const { + base::error_state get_error() const { return err_; } From 487f48145f49dd4506e59da18228996af22617d8 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 14:28:46 +0100 Subject: [PATCH 06/80] Move cache_check_steps.rb -> cache_steps.rb --- .../step_definitions/{cache_check_steps.rb => cache_steps.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename features/step_definitions/{cache_check_steps.rb => cache_steps.rb} (100%) diff --git a/features/step_definitions/cache_check_steps.rb b/features/step_definitions/cache_steps.rb similarity index 100% rename from features/step_definitions/cache_check_steps.rb rename to features/step_definitions/cache_steps.rb From be6f90f16c87854babd47b095ae5bfc666ea2cbb Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 14:33:19 +0100 Subject: [PATCH 07/80] [thin_restore] use full paths for includes. --- thin-provisioning/thin_restore.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/thin-provisioning/thin_restore.cc b/thin-provisioning/thin_restore.cc index 950a3a8..ef16be1 100644 --- a/thin-provisioning/thin_restore.cc +++ b/thin-provisioning/thin_restore.cc @@ -16,11 +16,11 @@ // with thin-provisioning-tools. If not, see // . -#include "emitter.h" -#include "human_readable_format.h" -#include "metadata.h" -#include "restore_emitter.h" -#include "xml_format.h" +#include "thin-provisioning/emitter.h" +#include "thin-provisioning/human_readable_format.h" +#include "thin-provisioning/metadata.h" +#include "thin-provisioning/restore_emitter.h" +#include "thin-provisioning/xml_format.h" #include "version.h" #include From 7ada06aa775519e14c682c6c64d7d06d6a7ead34 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 14:34:51 +0100 Subject: [PATCH 08/80] fluff --- thin-provisioning/thin_restore.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/thin-provisioning/thin_restore.cc b/thin-provisioning/thin_restore.cc index ef16be1..ec26ad3 100644 --- a/thin-provisioning/thin_restore.cc +++ b/thin-provisioning/thin_restore.cc @@ -37,6 +37,8 @@ using namespace persistent_data; using namespace std; using namespace thin_provisioning; +//---------------------------------------------------------------- + namespace { int restore(string const &backup_file, string const &dev) { try { @@ -121,3 +123,5 @@ int main(int argc, char **argv) return restore(input, output); } + +//---------------------------------------------------------------- From 5fa1c0a2128d02315f7ebdb896f58a9f320c6abc Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 16:28:25 +0100 Subject: [PATCH 09/80] Add a feature that tests failure when the output file is missed off the thin_restore command line. --- features/thin_restore.feature | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/features/thin_restore.feature b/features/thin_restore.feature index 373b1b6..a8e75fc 100644 --- a/features/thin_restore.feature +++ b/features/thin_restore.feature @@ -40,6 +40,13 @@ Feature: thin_restore No input file provided. """ + Scenario: missing output file + When I run thin_restore with -i metadata.xml + Then it should fail with: + """ + No output file provided. + """ + Scenario: dump/restore is a noop Given valid metadata When I dump From 4360c8cbffd1012131ae4924acd08165b1108dc8 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 16:29:41 +0100 Subject: [PATCH 10/80] Some work on cache_check and cache_restore --- Makefile.in | 16 ++++- caching/cache_check.cc | 21 +----- caching/cache_restore.cc | 82 ++++++++++++++++++++++++ features/cache_restore.feature | 48 ++++++++++++++ features/step_definitions/cache_steps.rb | 9 +++ 5 files changed, 155 insertions(+), 21 deletions(-) create mode 100644 caching/cache_restore.cc create mode 100644 features/cache_restore.feature diff --git a/Makefile.in b/Makefile.in index 4dc2555..4110365 100644 --- a/Makefile.in +++ b/Makefile.in @@ -20,6 +20,9 @@ V=@ PROGRAMS=\ + cache_check \ + cache_restore \ + \ thin_check \ thin_dump \ thin_restore \ @@ -65,6 +68,7 @@ PDATA_OBJECTS=$(subst .cc,.o,$(SOURCE)) CXX_PROGRAM_SOURCE=\ caching/cache_check.cc \ + caching/cache_restore.cc \ \ thin-provisioning/thin_check.cc \ thin-provisioning/thin_dump.cc \ @@ -228,13 +232,19 @@ CACHE_CHECK_SOURCE=\ persistent-data/space-maps/careful_alloc.cc \ persistent-data/transaction_manager.cc \ caching/superblock.cc - CACHE_CHECK_OBJECTS=$(subst .cc,.o,$(CACHE_CHECK_SOURCE)) -cache_check: $(CACHE_CHECK_OBJECTS) caching/check.o +CACHE_RESTORE_SOURCE=$(SOURCE) +CACHE_RESTORE_OBJECTS=$(subst .cc,.o,$(CACHE_RESTORE_SOURCE)) + +cache_check: $(CACHE_CHECK_OBJECTS) caching/cache_check.o @echo " [LD] $@" $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) +cache_restore: $(CACHE_RESTORE_OBJECTS) caching/cache_restore.o + @echo " [LD] $@" + $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) + DEPEND_FILES=\ $(subst .cc,.d,$(SOURCE)) \ $(subst .cc,.d,$(TEST_SOURCE)) \ @@ -275,7 +285,7 @@ include unit-tests/Makefile .PHONEY: features -features: thin_check cache_check +features: $(PROGRAMS) cucumber --no-color --format progress test: features unit-test diff --git a/caching/cache_check.cc b/caching/cache_check.cc index ad7d41c..3de52dc 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -1,21 +1,3 @@ -// Copyright (C) 2011 Red Hat, Inc. All rights reserved. -// -// This file is part of the thin-provisioning-tools source. -// -// thin-provisioning-tools is free software: you can redistribute it -// and/or modify it under the terms of the GNU General Public License -// as published by the Free Software Foundation, either version 3 of -// the License, or (at your option) any later version. -// -// thin-provisioning-tools is distributed in the hope that it will be -// useful, but WITHOUT ANY WARRANTY; without even the implied warranty -// of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License along -// with thin-provisioning-tools. If not, see -// . - #include #include #include @@ -30,6 +12,9 @@ #include #include +#include "base/error_state.h" +#include "base/nested_output.h" + using namespace std; //---------------------------------------------------------------- diff --git a/caching/cache_restore.cc b/caching/cache_restore.cc new file mode 100644 index 0000000..b02aa2a --- /dev/null +++ b/caching/cache_restore.cc @@ -0,0 +1,82 @@ +#include "version.h" + +#include +#include +#include +#include + +using namespace std; + +//---------------------------------------------------------------- + +namespace { + void usage(ostream &out, string const &cmd) { + out << "Usage: " << cmd << " [options]" << endl + << "Options:" << endl + << " {-h|--help}" << endl + << " {-i|--input} " << endl + << " {-o|--output} " << endl + << " {-V|--version}" << endl; + } +} + +int main(int argc, char **argv) +{ + int c; + string input, output; + char const *prog_name = basename(argv[0]); + char const *short_opts = "hi:o:V"; + option const long_opts[] = { + { "help", no_argument, NULL, 'h'}, + { "input", required_argument, NULL, 'i' }, + { "output", required_argument, NULL, 'o'}, + { "version", no_argument, NULL, 'V'}, + { NULL, no_argument, NULL, 0 } + }; + + while ((c = getopt_long(argc, argv, short_opts, long_opts, NULL)) != -1) { + switch(c) { + case 'h': + usage(cout, prog_name); + return 0; + + case 'i': + input = optarg; + break; + + case 'o': + output = optarg; + break; + + case 'V': + cout << THIN_PROVISIONING_TOOLS_VERSION << endl; + return 0; + + default: + usage(cerr, prog_name); + return 1; + } + } + + if (argc != optind) { + usage(cerr, prog_name); + return 1; + } + + if (input.empty()) { + cerr << "No input file provided." << endl << endl; + usage(cerr, prog_name); + return 1; + } + + if (output.empty()) { + cerr << "No output file provided." << endl << endl; + usage(cerr, prog_name); + return 1; + } + + return 0; +} + +//---------------------------------------------------------------- + diff --git a/features/cache_restore.feature b/features/cache_restore.feature new file mode 100644 index 0000000..4ab9594 --- /dev/null +++ b/features/cache_restore.feature @@ -0,0 +1,48 @@ +Feature: thin_restore + Scenario: print version (-V flag) + When I run cache_restore with -V + Then it should pass with version + + Scenario: print version (--version flag) + When I run cache_restore with --version + Then it should pass with version + + Scenario: print help (-h) + When I run cache_restore with -h + Then it should pass with: + + """ + Usage: cache_restore [options] + Options: + {-h|--help} + {-i|--input} + {-o|--output} + {-V|--version} + """ + + Scenario: print help (--help) + When I run cache_restore with -h + Then it should pass with: + + """ + Usage: cache_restore [options] + Options: + {-h|--help} + {-i|--input} + {-o|--output} + {-V|--version} + """ + + Scenario: missing input file + When I run cache_restore with -o metadata.bin + Then it should fail with: + """ + No input file provided. + """ + + Scenario: missing output file + When I run cache_restore with -i metadata.xml + Then it should fail with: + """ + No output file provided. + """ diff --git a/features/step_definitions/cache_steps.rb b/features/step_definitions/cache_steps.rb index edba139..d82b923 100644 --- a/features/step_definitions/cache_steps.rb +++ b/features/step_definitions/cache_steps.rb @@ -55,3 +55,12 @@ end Then /^usage to stderr$/ do assert_partial_output(USAGE, all_stderr) end + +When(/^I run cache_check with (.*?)$/) do |opts| + run_simple("cache_check #{opts} #{dev_file}", false) +end + +When(/^I run cache_restore with (.*?)$/) do |opts| + run_simple("cache_restore #{opts}", false) +end + From a933749cbfcaccc30fc55b1370d177658b4d9b72 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 16 Aug 2013 16:44:44 +0100 Subject: [PATCH 11/80] update ignore file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index e5aa523..8ffca90 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ thin_repair thin_rmap cache_check +cache_restore *.metadata bad-metadata From d3ce6b811b298072785a0b53d4ee9a2fbf65572a Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 19 Aug 2013 12:40:03 +0100 Subject: [PATCH 12/80] Start stubbing out cache_dump --- .gitignore | 1 + Makefile.in | 8 ++++ caching/cache_dump.cc | 60 ++++++++++++++++++++++++ features/cache_dump.feature | 30 ++++++++++++ features/cache_restore.feature | 7 +++ features/step_definitions/cache_steps.rb | 19 ++++++++ 6 files changed, 125 insertions(+) create mode 100644 caching/cache_dump.cc create mode 100644 features/cache_dump.feature diff --git a/.gitignore b/.gitignore index 8ffca90..e06e498 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ thin_repair thin_rmap cache_check +cache_dump cache_restore *.metadata diff --git a/Makefile.in b/Makefile.in index 4110365..0dbb0ba 100644 --- a/Makefile.in +++ b/Makefile.in @@ -21,6 +21,7 @@ V=@ PROGRAMS=\ cache_check \ + cache_dump \ cache_restore \ \ thin_check \ @@ -234,6 +235,9 @@ CACHE_CHECK_SOURCE=\ caching/superblock.cc CACHE_CHECK_OBJECTS=$(subst .cc,.o,$(CACHE_CHECK_SOURCE)) +CACHE_DUMP_SOURCE=$(SOURCE) +CACHE_DUMP_OBJECTS=$(subst .cc,.o,$(CACHE_DUMP_SOURCE)) + CACHE_RESTORE_SOURCE=$(SOURCE) CACHE_RESTORE_OBJECTS=$(subst .cc,.o,$(CACHE_RESTORE_SOURCE)) @@ -241,6 +245,10 @@ cache_check: $(CACHE_CHECK_OBJECTS) caching/cache_check.o @echo " [LD] $@" $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) +cache_dump: $(CACHE_DUMP_OBJECTS) caching/cache_dump.o + @echo " [LD] $@" + $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) + cache_restore: $(CACHE_RESTORE_OBJECTS) caching/cache_restore.o @echo " [LD] $@" $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc new file mode 100644 index 0000000..7d15a39 --- /dev/null +++ b/caching/cache_dump.cc @@ -0,0 +1,60 @@ +#include +#include +#include + +#include "version.h" + +using namespace std; + +//---------------------------------------------------------------- + +namespace { + void usage(ostream &out, string const &cmd) { + out << "Usage: " << cmd << " [options] {device|file}" << endl + << "Options:" << endl + << " {-h|--help}" << endl + << " {-V|--version}" << endl; + } +} + +//---------------------------------------------------------------- + +int main(int argc, char **argv) +{ + int c; + char const shortopts[] = "hV"; + + option const longopts[] = { + { "help", no_argument, NULL, 'h'}, + { "version", no_argument, NULL, 'V'}, + { NULL, no_argument, NULL, 0 } + }; + + while ((c = getopt_long(argc, argv, shortopts, longopts, NULL)) != -1) { + switch(c) { + case 'h': + usage(cout, basename(argv[0])); + return 0; + + case 'V': + cout << THIN_PROVISIONING_TOOLS_VERSION << endl; + return 0; + + default: + usage(cerr, basename(argv[0])); + return 1; + } + } + +#if 0 + if (argc == optind) { + cerr << "No input file provided." << endl; + usage(cerr, basename(argv[0])); + return 1; + } +#endif + + return 0; +} + +//---------------------------------------------------------------- diff --git a/features/cache_dump.feature b/features/cache_dump.feature new file mode 100644 index 0000000..6885bba --- /dev/null +++ b/features/cache_dump.feature @@ -0,0 +1,30 @@ +Feature: cache_dump + Scenario: print version (-V flag) + When I run cache_dump with -V + Then it should pass with version + + Scenario: print version (--version flag) + When I run cache_dump with --version + Then it should pass with version + + Scenario: print help (-h) + When I run cache_dump with -h + Then it should pass with: + + """ + Usage: cache_dump [options] {device|file} + Options: + {-h|--help} + {-V|--version} + """ + + Scenario: print help (--help) + When I run cache_dump with -h + Then it should pass with: + + """ + Usage: cache_dump [options] {device|file} + Options: + {-h|--help} + {-V|--version} + """ diff --git a/features/cache_restore.feature b/features/cache_restore.feature index 4ab9594..3fdea64 100644 --- a/features/cache_restore.feature +++ b/features/cache_restore.feature @@ -46,3 +46,10 @@ Feature: thin_restore """ No output file provided. """ + + Scenario: dump/restore is a noop + Given valid cache metadata + When I dump cache + And I restore cache + And I dump cache + Then cache dumps 1 and 2 should be identical diff --git a/features/step_definitions/cache_steps.rb b/features/step_definitions/cache_steps.rb index d82b923..d61c418 100644 --- a/features/step_definitions/cache_steps.rb +++ b/features/step_definitions/cache_steps.rb @@ -64,3 +64,22 @@ When(/^I run cache_restore with (.*?)$/) do |opts| run_simple("cache_restore #{opts}", false) end +When(/^I run cache_dump with (.*?)$/) do |opts| + run_simple("cache_dump #{opts}", false) +end + +Given(/^valid cache metadata$/) do + pending # express the regexp above with the code you wish you had +end + +When(/^I dump cache$/) do + pending # express the regexp above with the code you wish you had +end + +When(/^I restore cache$/) do + pending # express the regexp above with the code you wish you had +end + +Then(/^cache dumps (\d+) and (\d+) should be identical$/) do |arg1, arg2| + pending # express the regexp above with the code you wish you had +end From 6615b25e4b2aad9809a7c7c5b6d6ffc27acc3baf Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 11 Sep 2013 11:40:46 +0100 Subject: [PATCH 13/80] WIP on cache tools --- Gemfile.lock | 12 +- Makefile.in | 13 +- caching/cache_dump.cc | 35 ++- caching/cache_restore.cc | 37 ++- caching/emitter.h | 44 +++ caching/metadata.cc | 17 ++ caching/metadata.h | 33 +++ caching/xml_format.cc | 279 ++++++++++++++++++ caching/xml_format.h | 17 ++ features/cache_dump.feature | 19 ++ features/cache_restore.feature | 15 +- features/step_definitions/cache_steps.rb | 37 ++- .../file_utils.cc | 11 +- .../file_utils.h | 3 +- thin-provisioning/metadata.cc | 17 +- thin-provisioning/metadata_checker.cc | 2 +- thin-provisioning/thin_check.cc | 2 +- thin-provisioning/thin_dump.cc | 19 +- thin-provisioning/thin_rmap.cc | 2 +- 19 files changed, 560 insertions(+), 54 deletions(-) create mode 100644 caching/emitter.h create mode 100644 caching/metadata.cc create mode 100644 caching/metadata.h create mode 100644 caching/xml_format.cc create mode 100644 caching/xml_format.h rename {thin-provisioning => persistent-data}/file_utils.cc (78%) rename {thin-provisioning => persistent-data}/file_utils.h (73%) diff --git a/Gemfile.lock b/Gemfile.lock index 16b0c75..7684f60 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -8,22 +8,22 @@ GEM builder (3.2.2) childprocess (0.3.9) ffi (~> 1.0, >= 1.0.11) - cucumber (1.3.5) + cucumber (1.3.7) builder (>= 2.1.2) diff-lcs (>= 1.1.3) - gherkin (~> 2.12.0) + gherkin (~> 2.12.1) multi_json (~> 1.7.5) multi_test (>= 0.0.2) diff-lcs (1.2.4) ejt_command_line (0.0.2) ffi (1.9.0) - gherkin (2.12.0) + gherkin (2.12.1) multi_json (~> 1.3) - multi_json (1.7.7) + multi_json (1.7.9) multi_test (0.0.2) - rspec-expectations (2.14.0) + rspec-expectations (2.14.2) diff-lcs (>= 1.1.3, < 2.0) - thinp_xml (0.0.6) + thinp_xml (0.0.8) ejt_command_line (= 0.0.2) PLATFORMS diff --git a/Makefile.in b/Makefile.in index 0dbb0ba..699e00d 100644 --- a/Makefile.in +++ b/Makefile.in @@ -37,10 +37,13 @@ SOURCE=\ base/error_state.cc \ \ caching/superblock.cc \ + caching/metadata.cc \ + caching/xml_format.cc \ \ persistent-data/checksum.cc \ persistent-data/endian_utils.cc \ persistent-data/error_set.cc \ + persistent-data/file_utils.cc \ persistent-data/hex_dump.cc \ persistent-data/lock_tracker.cc \ persistent-data/transaction_manager.cc \ @@ -53,7 +56,6 @@ SOURCE=\ persistent-data/space-maps/careful_alloc.cc \ \ thin-provisioning/device_tree.cc \ - thin-provisioning/file_utils.cc \ thin-provisioning/human_readable_format.cc \ thin-provisioning/mapping_tree.cc \ thin-provisioning/metadata.cc \ @@ -147,6 +149,7 @@ THIN_CHECK_SOURCE=\ persistent-data/checksum.cc \ persistent-data/endian_utils.cc \ persistent-data/error_set.cc \ + persistent-data/file_utils.cc \ persistent-data/hex_dump.cc \ persistent-data/lock_tracker.cc \ persistent-data/data-structures/btree.cc \ @@ -155,7 +158,6 @@ THIN_CHECK_SOURCE=\ persistent-data/space-maps/recursive.cc \ persistent-data/space-maps/careful_alloc.cc \ persistent-data/transaction_manager.cc \ - thin-provisioning/file_utils.cc \ thin-provisioning/device_tree.cc \ thin-provisioning/mapping_tree.cc \ thin-provisioning/metadata.cc \ @@ -166,6 +168,7 @@ THIN_RMAP_SOURCE=\ persistent-data/checksum.cc \ persistent-data/endian_utils.cc \ persistent-data/error_set.cc \ + persistent-data/file_utils.cc \ persistent-data/hex_dump.cc \ persistent-data/lock_tracker.cc \ persistent-data/data-structures/btree.cc \ @@ -174,7 +177,6 @@ THIN_RMAP_SOURCE=\ persistent-data/space-maps/recursive.cc \ persistent-data/space-maps/careful_alloc.cc \ persistent-data/transaction_manager.cc \ - thin-provisioning/file_utils.cc \ thin-provisioning/device_tree.cc \ thin-provisioning/mapping_tree.cc \ thin-provisioning/metadata.cc \ @@ -232,7 +234,10 @@ CACHE_CHECK_SOURCE=\ persistent-data/space-maps/recursive.cc \ persistent-data/space-maps/careful_alloc.cc \ persistent-data/transaction_manager.cc \ - caching/superblock.cc + caching/superblock.cc \ + caching/metadata.cc \ + caching/xml_format.cc + CACHE_CHECK_OBJECTS=$(subst .cc,.o,$(CACHE_CHECK_SOURCE)) CACHE_DUMP_SOURCE=$(SOURCE) diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index 7d15a39..73467cd 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -1,18 +1,41 @@ +#include #include #include #include #include "version.h" +#include "caching/xml_format.h" using namespace std; +using namespace caching; //---------------------------------------------------------------- namespace { + string const STDOUT_PATH("-"); + + bool want_stdout(string const &output) { + return output == STDOUT_PATH; + } + + int dump_(string const &dev, ostream &out) { + emitter::ptr e = create_xml_emitter(out); + return 0; + } + + int dump(string const &dev, string const &output) { + if (want_stdout(output)) { + ofstream out(output.c_str()); + return dump_(dev, out); + } else + return dump_(dev, cout); + } + void usage(ostream &out, string const &cmd) { out << "Usage: " << cmd << " [options] {device|file}" << endl << "Options:" << endl << " {-h|--help}" << endl + << " {-o }" << endl << " {-V|--version}" << endl; } } @@ -22,10 +45,12 @@ namespace { int main(int argc, char **argv) { int c; - char const shortopts[] = "hV"; + string output("-"); + char const shortopts[] = "ho:V"; option const longopts[] = { { "help", no_argument, NULL, 'h'}, + { "output", required_argument, NULL, 'o'}, { "version", no_argument, NULL, 'V'}, { NULL, no_argument, NULL, 0 } }; @@ -36,6 +61,10 @@ int main(int argc, char **argv) usage(cout, basename(argv[0])); return 0; + case 'o': + output = optarg; + break; + case 'V': cout << THIN_PROVISIONING_TOOLS_VERSION << endl; return 0; @@ -46,15 +75,13 @@ int main(int argc, char **argv) } } -#if 0 if (argc == optind) { cerr << "No input file provided." << endl; usage(cerr, basename(argv[0])); return 1; } -#endif - return 0; + return dump(argv[optind], output); } //---------------------------------------------------------------- diff --git a/caching/cache_restore.cc b/caching/cache_restore.cc index b02aa2a..8211f7e 100644 --- a/caching/cache_restore.cc +++ b/caching/cache_restore.cc @@ -1,15 +1,50 @@ #include "version.h" +#include "caching/metadata.h" +#include "caching/xml_format.h" +#include "persistent-data/file_utils.h" + +#include #include #include #include #include +using namespace caching; +using namespace persistent_data; using namespace std; //---------------------------------------------------------------- namespace { + void check_file_exists(string const &file) { + struct stat info; + int r = ::stat(file.c_str(), &info); + if (r) + throw runtime_error("Couldn't stat file"); + + if (!S_ISREG(info.st_mode)) + throw runtime_error("Not a regular file"); + } + + int restore(string const &xml_file, string const &dev) { + try { + block_manager<>::ptr bm = open_bm(dev, block_io<>::READ_ONLY); + metadata::ptr md(new metadata(bm, metadata::CREATE)); + emitter::ptr restorer = create_restore_emitter(md); + + check_file_exists(xml_file); + ifstream in(xml_file.c_str(), ifstream::in); + parse_xml(in, restorer); + + } catch (std::exception &e) { + cerr << e.what() << endl; + return 1; + } + + return 0; + } + void usage(ostream &out, string const &cmd) { out << "Usage: " << cmd << " [options]" << endl << "Options:" << endl @@ -75,7 +110,7 @@ int main(int argc, char **argv) return 1; } - return 0; + return restore(input, output); } //---------------------------------------------------------------- diff --git a/caching/emitter.h b/caching/emitter.h new file mode 100644 index 0000000..a2a84dc --- /dev/null +++ b/caching/emitter.h @@ -0,0 +1,44 @@ +#ifndef CACHE_EMITTER_H +#define CACHE_EMITTER_H + +#include +#include + +#include "persistent-data/block.h" + +//---------------------------------------------------------------- + +namespace caching { + namespace pd = persistent_data; + + class emitter { + public: + typedef boost::shared_ptr ptr; + + virtual ~emitter() {} + + virtual void begin_superblock(std::string const &uuid, + pd::block_address block_size, + pd::block_address nr_cache_blocks, + std::string const &policy) = 0; + + virtual void end_superblock() = 0; + + virtual void begin_mappings() = 0; + virtual void end_mappings() = 0; + + virtual void mapping(pd::block_address cblock, + pd::block_address oblock, + bool dirty) = 0; + + virtual void begin_hints() = 0; + virtual void end_hints() = 0; + + virtual void hint(pd::block_address cblock, + std::string const &data) = 0; + }; +} + +//---------------------------------------------------------------- + +#endif diff --git a/caching/metadata.cc b/caching/metadata.cc new file mode 100644 index 0000000..5985080 --- /dev/null +++ b/caching/metadata.cc @@ -0,0 +1,17 @@ +#include "caching/metadata.h" + +using namespace caching; + +//---------------------------------------------------------------- + +metadata::metadata(block_manager<>::ptr bm, open_type ot) +{ + +} + +metadata::metadata(block_manager<>::ptr bm, block_address metadata_snap) +{ + +} + +//---------------------------------------------------------------- diff --git a/caching/metadata.h b/caching/metadata.h new file mode 100644 index 0000000..99f6c17 --- /dev/null +++ b/caching/metadata.h @@ -0,0 +1,33 @@ +#ifndef CACHE_METADATA_H +#define CACHE_METADATA_H + +#include "persistent-data/block.h" +#include "persistent-data/data-structures/array.h" +#include "persistent-data/endian_utils.h" +#include "persistent-data/space-maps/disk.h" +#include "persistent-data/transaction_manager.h" + +#include "caching/superblock.h" + +//---------------------------------------------------------------- + +namespace caching { + class metadata { + public: + enum open_type { + CREATE, + OPEN + }; + + typedef block_manager<>::read_ref read_ref; + typedef block_manager<>::write_ref write_ref; + typedef boost::shared_ptr ptr; + + metadata(block_manager<>::ptr bm, open_type ot); + metadata(block_manager<>::ptr bm, block_address metadata_snap); + }; +}; + +//---------------------------------------------------------------- + +#endif diff --git a/caching/xml_format.cc b/caching/xml_format.cc new file mode 100644 index 0000000..9d0f439 --- /dev/null +++ b/caching/xml_format.cc @@ -0,0 +1,279 @@ +#include "caching/xml_format.h" + +#include +#include + +using namespace boost; +using namespace caching; +using namespace persistent_data; +using namespace std; + +//---------------------------------------------------------------- + +namespace { + // base64 encoding? + string encode(string const &data) { + return data; + } + + string decode(string const &data) { + return data; + } + + //-------------------------------- + // Emitter + //-------------------------------- + class xml_emitter : public emitter { + public: + xml_emitter(ostream &out) + : out_(out), + indent_(0) { + } + + void begin_superblock(std::string const &uuid, + block_address block_size, + block_address nr_cache_blocks, + std::string const &policy) { + indent(); + out_ << "" << endl; + inc(); + } + + virtual void end_superblock() { + dec(); + indent(); + out_ << "" << endl; + } + + virtual void begin_mappings() { + indent(); + out_ << "" << endl; + inc(); + } + + virtual void end_mappings() { + dec(); + indent(); + out_ << "" << endl; + } + + virtual void mapping(block_address cblock, + block_address oblock, + bool dirty) { + indent(); + out_ << "" << endl; + } + + virtual void begin_hints() { + indent(); + out_ << "" << endl; + inc(); + } + + virtual void end_hints() { + dec(); + indent(); + out_ << "" << endl; + } + + virtual void hint(block_address cblock, + std::string const &data) { + out_ << "" << endl; + } + + private: + string as_truth(bool v) const { + return v ? "true" : "false"; + } + + // FIXME: factor out a common class with the thin_provisioning emitter + void indent() { + for (unsigned i = 0; i < indent_ * 2; i++) + out_ << ' '; + } + + void inc() { + indent_++; + } + + void dec() { + indent_--; + } + + ostream &out_; + unsigned indent_; + }; + + //-------------------------------- + // Parser + //-------------------------------- + + // FIXME: factor out common code with thinp one + typedef std::map attributes; + + void build_attributes(attributes &a, char const **attr) { + while (*attr) { + char const *key = *attr; + + attr++; + if (!*attr) { + ostringstream out; + out << "No value given for xml attribute: " << key; + throw runtime_error(out.str()); + } + + char const *value = *attr; + a.insert(make_pair(string(key), string(value))); + attr++; + } + } + + template + T get_attr(attributes const &attr, string const &key) { + attributes::const_iterator it = attr.find(key); + if (it == attr.end()) { + ostringstream out; + out << "could not find attribute: " << key; + throw runtime_error(out.str()); + } + + return boost::lexical_cast(it->second); + } + + template + boost::optional get_opt_attr(attributes const &attr, string const &key) { + typedef boost::optional rtype; + attributes::const_iterator it = attr.find(key); + if (it == attr.end()) + return rtype(); + + return rtype(boost::lexical_cast(it->second)); + } + + void parse_superblock(emitter *e, attributes const &attr) { + e->begin_superblock(get_attr(attr, "uuid"), + get_attr(attr, "block_size"), + get_attr(attr, "nr_cache_blocks"), + get_attr(attr, "policy")); + } + + bool to_bool(string const &str) { + if (str == "true") + return true; + + else if (str == "false") + return false; + + throw runtime_error("bad boolean value"); + } + + void parse_mapping(emitter *e, attributes const &attr) { + e->mapping(get_attr(attr, "cache_block"), + get_attr(attr, "origin_block"), + to_bool(get_attr(attr, "dirty"))); + } + + void parse_hint(emitter *e, attributes const &attr) { + e->hint(get_attr(attr, "cache_block"), + decode(get_attr(attr, "data"))); + } + + void start_tag(void *data, char const *el, char const **attr) { + emitter *e = static_cast(data); + attributes a; + + build_attributes(a, attr); + + if (!strcmp(el, "superblock")) + parse_superblock(e, a); + + else if (!strcmp(el, "mappings")) + e->begin_mappings(); + + else if (!strcmp(el, "mapping")) + parse_mapping(e, a); + + else if (!strcmp(el, "hints")) + e->begin_hints(); + + else if (!strcmp(el, "hint")) + parse_hint(e, a); + + else + throw runtime_error("unknown tag type"); + } + + void end_tag(void *data, const char *el) { + emitter *e = static_cast(data); + + if (!strcmp(el, "superblock")) + e->end_superblock(); + + else if (!strcmp(el, "mappings")) + e->end_mappings(); + + else if (!strcmp(el, "mapping")) + // do nothing + ; + + else if (!strcmp(el, "hints")) + e->end_hints(); + + else if (!strcmp(el, "hint")) + // do nothing + ; + + else + throw runtime_error("unknown tag close"); + } + +} + +//---------------------------------------------------------------- + +caching::emitter::ptr +caching::create_xml_emitter(ostream &out) +{ + return emitter::ptr(new xml_emitter(out)); +} + +void +caching::parse_xml(istream &in, emitter::ptr e) +{ + XML_Parser parser = XML_ParserCreate(NULL); + if (!parser) + throw runtime_error("couldn't create xml parser"); + + XML_SetUserData(parser, e.get()); + XML_SetElementHandler(parser, start_tag, end_tag); + + while (!in.eof()) { + char buffer[4096]; + in.read(buffer, sizeof(buffer)); + size_t len = in.gcount(); + int done = in.eof(); + + if (!XML_Parse(parser, buffer, len, done)) { + ostringstream out; + out << "Parse error at line " + << XML_GetCurrentLineNumber(parser) + << ":\n" + << XML_ErrorString(XML_GetErrorCode(parser)) + << endl; + throw runtime_error(out.str()); + } + } + +} + +//---------------------------------------------------------------- diff --git a/caching/xml_format.h b/caching/xml_format.h new file mode 100644 index 0000000..6855fb5 --- /dev/null +++ b/caching/xml_format.h @@ -0,0 +1,17 @@ +#ifndef CACHE_XML_FORMAT_H +#define CACHE_XML_FORMAT_H + +#include "emitter.h" + +#include + +//---------------------------------------------------------------- + +namespace caching { + emitter::ptr create_xml_emitter(std::ostream &out); + void parse_xml(std::istream &in, emitter::ptr e); +} + +//---------------------------------------------------------------- + +#endif diff --git a/features/cache_dump.feature b/features/cache_dump.feature index 6885bba..15bcecd 100644 --- a/features/cache_dump.feature +++ b/features/cache_dump.feature @@ -28,3 +28,22 @@ Feature: cache_dump {-h|--help} {-V|--version} """ + + Scenario: accepts an output file + Given valid cache metadata + When I run cache_dump with -o metadata.xml metadata.bin + Then it should pass + + Scenario: missing input file + When I run cache_dump + Then it should fail with: + """ + No input file provided. + """ + + Scenario: dump/restore is a noop + Given valid cache metadata + When I cache_dump + And I cache_restore + And I cache_dump + Then cache dumps 1 and 2 should be identical diff --git a/features/cache_restore.feature b/features/cache_restore.feature index 3fdea64..a38d4b4 100644 --- a/features/cache_restore.feature +++ b/features/cache_restore.feature @@ -40,6 +40,10 @@ Feature: thin_restore No input file provided. """ + Scenario: input file not found + When I run cache_restore with -i foo.xml -o metadata.bin + Then it should fail + Scenario: missing output file When I run cache_restore with -i metadata.xml Then it should fail with: @@ -47,9 +51,8 @@ Feature: thin_restore No output file provided. """ - Scenario: dump/restore is a noop - Given valid cache metadata - When I dump cache - And I restore cache - And I dump cache - Then cache dumps 1 and 2 should be identical + Scenario: successfully restores a valid xml file + Given a small xml file + And an empty dev file + When I run cache_restore with -i metadata.xml -o metadata.bin + Then it should pass diff --git a/features/step_definitions/cache_steps.rb b/features/step_definitions/cache_steps.rb index d61c418..342fd74 100644 --- a/features/step_definitions/cache_steps.rb +++ b/features/step_definitions/cache_steps.rb @@ -64,22 +64,41 @@ When(/^I run cache_restore with (.*?)$/) do |opts| run_simple("cache_restore #{opts}", false) end +When(/^I run cache_dump$/) do + run_simple("cache_dump", false) +end + When(/^I run cache_dump with (.*?)$/) do |opts| run_simple("cache_dump #{opts}", false) end Given(/^valid cache metadata$/) do - pending # express the regexp above with the code you wish you had -end + in_current_dir do + system("cache_xml create --nr-cache-blocks uniform[1000-5000] --nr-mappings uniform[500-1000] > #{xml_file}") + end -When(/^I dump cache$/) do - pending # express the regexp above with the code you wish you had -end - -When(/^I restore cache$/) do - pending # express the regexp above with the code you wish you had + run_simple("dd if=/dev/zero of=#{dev_file} bs=4k count=1024") + run_simple("cache_restore -i #{xml_file} -o #{dev_file}") end Then(/^cache dumps (\d+) and (\d+) should be identical$/) do |arg1, arg2| - pending # express the regexp above with the code you wish you had + run_simple("diff -ub #{dump_files[d1.to_i]} #{dump_files[d2.to_i]}", true) +end + +Given(/^a small xml file$/) do + in_current_dir do + system("cache_xml create --nr-cache-blocks 3 --nr-mappings 3 --layout linear --dirty-percent 100 > #{xml_file}") + end +end + +Given(/^an empty dev file$/) do + run_simple("dd if=/dev/zero of=#{dev_file} bs=4k count=1024") +end + +When(/^I cache_dump$/) do + run_simple("cache_dump #{dev_file} -o #{new_dump_file}", true) +end + +When(/^I cache_restore$/) do + run_simple("cache_restore -i #{dump_files[-1]} -o #{dev_file}", true) end diff --git a/thin-provisioning/file_utils.cc b/persistent-data/file_utils.cc similarity index 78% rename from thin-provisioning/file_utils.cc rename to persistent-data/file_utils.cc index 4110ff2..e5fef3f 100644 --- a/thin-provisioning/file_utils.cc +++ b/persistent-data/file_utils.cc @@ -1,5 +1,5 @@ #include "persistent-data/math_utils.h" -#include "thin-provisioning/file_utils.h" +#include "persistent-data/file_utils.h" #include #include @@ -12,7 +12,7 @@ using namespace base; //---------------------------------------------------------------- persistent_data::block_address -thin_provisioning::get_nr_blocks(string const &path) +persistent_data::get_nr_blocks(string const &path) { using namespace persistent_data; @@ -47,4 +47,11 @@ thin_provisioning::get_nr_blocks(string const &path) return nr_blocks; } +persistent_data::block_manager<>::ptr +persistent_data::open_bm(std::string const &dev_path, block_io<>::mode m) +{ + block_address nr_blocks = get_nr_blocks(dev_path); + return block_manager<>::ptr(new block_manager<>(dev_path, nr_blocks, 1, m)); +} + //---------------------------------------------------------------- diff --git a/thin-provisioning/file_utils.h b/persistent-data/file_utils.h similarity index 73% rename from thin-provisioning/file_utils.h rename to persistent-data/file_utils.h index 9990032..c2d2d81 100644 --- a/thin-provisioning/file_utils.h +++ b/persistent-data/file_utils.h @@ -5,8 +5,9 @@ //---------------------------------------------------------------- -namespace thin_provisioning { +namespace persistent_data { persistent_data::block_address get_nr_blocks(string const &path); + block_manager<>::ptr open_bm(std::string const &dev_path, block_io<>::mode m); } //---------------------------------------------------------------- diff --git a/thin-provisioning/metadata.cc b/thin-provisioning/metadata.cc index 00832bb..fdc96e3 100644 --- a/thin-provisioning/metadata.cc +++ b/thin-provisioning/metadata.cc @@ -17,9 +17,9 @@ // . #include "thin-provisioning/device_tree.h" -#include "thin-provisioning/file_utils.h" #include "thin-provisioning/metadata.h" +#include "persistent-data/file_utils.h" #include "persistent-data/math_utils.h" #include "persistent-data/space-maps/core.h" #include "persistent-data/space-maps/disk.h" @@ -40,15 +40,6 @@ namespace { unsigned const METADATA_CACHE_SIZE = 1024; - block_manager<>::ptr open_bm(string const &dev_path, bool writeable) { - block_address nr_blocks = get_nr_blocks(dev_path); - block_io<>::mode m = writeable ? - block_io<>::READ_WRITE : - block_io<>::READ_ONLY; - - return block_manager<>::ptr(new block_manager<>(dev_path, nr_blocks, 1, m)); - } - transaction_manager::ptr open_tm(block_manager<>::ptr bm) { space_map::ptr sm(new core_map(bm->get_nr_blocks())); @@ -90,7 +81,7 @@ metadata::metadata(std::string const &dev_path, open_type ot, { switch (ot) { case OPEN: - tm_ = open_tm(open_bm(dev_path, false)); + tm_ = open_tm(open_bm(dev_path, block_io<>::READ_ONLY)); sb_ = read_superblock(tm_->get_bm()); if (sb_.version_ != 1) @@ -115,7 +106,7 @@ metadata::metadata(std::string const &dev_path, open_type ot, break; case CREATE: - tm_ = open_tm(open_bm(dev_path, true)); + tm_ = open_tm(open_bm(dev_path, block_io<>::READ_WRITE)); space_map::ptr core = tm_->get_sm(); metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); copy_space_maps(metadata_sm_, core); @@ -143,7 +134,7 @@ metadata::metadata(std::string const &dev_path, open_type ot, metadata::metadata(std::string const &dev_path, block_address metadata_snap) { - tm_ = open_tm(open_bm(dev_path, false)); + tm_ = open_tm(open_bm(dev_path, block_io<>::READ_ONLY)); sb_ = read_superblock(tm_->get_bm(), metadata_snap); // We don't open the metadata sm for a held root diff --git a/thin-provisioning/metadata_checker.cc b/thin-provisioning/metadata_checker.cc index ae8a6c9..e8bd6d3 100644 --- a/thin-provisioning/metadata_checker.cc +++ b/thin-provisioning/metadata_checker.cc @@ -16,7 +16,7 @@ // with thin-provisioning-tools. If not, see // . -#include "thin-provisioning/file_utils.h" +#include "persistent-data/file_utils.h" #include "thin-provisioning/metadata.h" #include "thin-provisioning/metadata_checker.h" diff --git a/thin-provisioning/thin_check.cc b/thin-provisioning/thin_check.cc index acca39b..c16f597 100644 --- a/thin-provisioning/thin_check.cc +++ b/thin-provisioning/thin_check.cc @@ -25,8 +25,8 @@ #include "base/error_state.h" #include "base/nested_output.h" #include "persistent-data/space-maps/core.h" +#include "persistent-data/file_utils.h" #include "thin-provisioning/device_tree.h" -#include "thin-provisioning/file_utils.h" #include "thin-provisioning/mapping_tree.h" #include "thin-provisioning/superblock.h" diff --git a/thin-provisioning/thin_dump.cc b/thin-provisioning/thin_dump.cc index d2c8950..3d0e8eb 100644 --- a/thin-provisioning/thin_dump.cc +++ b/thin-provisioning/thin_dump.cc @@ -37,8 +37,8 @@ struct flags { }; namespace { - int dump(string const &path, ostream *out, string const &format, struct flags &flags, - block_address metadata_snap = 0) { + int dump_(string const &path, ostream &out, string const &format, struct flags &flags, + block_address metadata_snap) { try { metadata::ptr md(new metadata(path, metadata_snap)); emitter::ptr e; @@ -56,9 +56,9 @@ namespace { } if (format == "xml") - e = create_xml_emitter(*out); + e = create_xml_emitter(out); else if (format == "human_readable") - e = create_human_readable_emitter(*out); + e = create_human_readable_emitter(out); else { cerr << "unknown format '" << format << "'" << endl; exit(1); @@ -74,6 +74,15 @@ namespace { return 0; } + int dump(string const &path, char const *output, string const &format, struct flags &flags, + block_address metadata_snap = 0) { + if (output) { + ofstream out(output); + return dump_(path, out, format, flags, metadata_snap); + } else + return dump_(path, cout, format, flags, metadata_snap); + } + void usage(ostream &out, string const &cmd) { out << "Usage: " << cmd << " [options] {device|file}" << endl << "Options:" << endl @@ -154,5 +163,5 @@ int main(int argc, char **argv) return 1; } - return dump(argv[optind], output ? new ofstream(output) : &cout, format, flags, metadata_snap); + return dump(argv[optind], output, format, flags, metadata_snap); } diff --git a/thin-provisioning/thin_rmap.cc b/thin-provisioning/thin_rmap.cc index 3b90c52..bfbd67f 100644 --- a/thin-provisioning/thin_rmap.cc +++ b/thin-provisioning/thin_rmap.cc @@ -9,7 +9,7 @@ #include "persistent-data/data-structures/btree_damage_visitor.h" #include "persistent-data/run.h" #include "persistent-data/space-maps/core.h" -#include "thin-provisioning/file_utils.h" +#include "persistent-data/file_utils.h" #include "thin-provisioning/superblock.h" #include "thin-provisioning/mapping_tree.h" #include "thin-provisioning/rmap_visitor.h" From dadb32d15f1fa9ebe9c730a8aa68d55e0f3b863b Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 12 Sep 2013 12:33:32 +0100 Subject: [PATCH 14/80] some work on cache_restore --- Makefile.in | 4 ++- caching/cache_restore.cc | 1 + caching/hint_array.h | 31 +++++++++++++++++++ caching/mapping_array.h | 32 ++++++++++++++++++++ caching/metadata.h | 9 ++++++ caching/restore_emitter.cc | 61 ++++++++++++++++++++++++++++++++++++++ caching/restore_emitter.h | 15 ++++++++++ 7 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 caching/hint_array.h create mode 100644 caching/mapping_array.h create mode 100644 caching/restore_emitter.cc create mode 100644 caching/restore_emitter.h diff --git a/Makefile.in b/Makefile.in index 699e00d..a72d18c 100644 --- a/Makefile.in +++ b/Makefile.in @@ -38,6 +38,7 @@ SOURCE=\ \ caching/superblock.cc \ caching/metadata.cc \ + caching/restore_emitter.cc \ caching/xml_format.cc \ \ persistent-data/checksum.cc \ @@ -236,6 +237,7 @@ CACHE_CHECK_SOURCE=\ persistent-data/transaction_manager.cc \ caching/superblock.cc \ caching/metadata.cc \ + caching/restore_emitter.cc \ caching/xml_format.cc CACHE_CHECK_OBJECTS=$(subst .cc,.o,$(CACHE_CHECK_SOURCE)) @@ -248,7 +250,7 @@ CACHE_RESTORE_OBJECTS=$(subst .cc,.o,$(CACHE_RESTORE_SOURCE)) cache_check: $(CACHE_CHECK_OBJECTS) caching/cache_check.o @echo " [LD] $@" - $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) + $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) cache_dump: $(CACHE_DUMP_OBJECTS) caching/cache_dump.o @echo " [LD] $@" diff --git a/caching/cache_restore.cc b/caching/cache_restore.cc index 8211f7e..62c4ea6 100644 --- a/caching/cache_restore.cc +++ b/caching/cache_restore.cc @@ -1,6 +1,7 @@ #include "version.h" #include "caching/metadata.h" +#include "caching/restore_emitter.h" #include "caching/xml_format.h" #include "persistent-data/file_utils.h" diff --git a/caching/hint_array.h b/caching/hint_array.h new file mode 100644 index 0000000..507f362 --- /dev/null +++ b/caching/hint_array.h @@ -0,0 +1,31 @@ +#ifndef CACHE_HINT_ARRAY_H +#define CACHE_HINT_ARRAY_H + +#include "persistent-data/data-structures/array.h" + +#include + +//---------------------------------------------------------------- + +namespace caching { + namespace hint_array_detail { + template + struct hint_traits { + typedef unsigned char byte; + typedef byte disk_type[WIDTH]; + typedef std::string value_type; + typedef no_op_ref_counter ref_counter; + + static void unpack(disk_type const &disk, value_type &value); + static void pack(value_type const &value, disk_type &disk); + }; + + // FIXME: data visitor stuff + } + +// typedef persistent_data::array hint_array; +} + +//---------------------------------------------------------------- + +#endif diff --git a/caching/mapping_array.h b/caching/mapping_array.h new file mode 100644 index 0000000..a938466 --- /dev/null +++ b/caching/mapping_array.h @@ -0,0 +1,32 @@ +#ifndef CACHE_MAPPING_ARRAY_H +#define CACHE_MAPPING_ARRAY_H + +#include "persistent-data/data-structures/array.h" + +//---------------------------------------------------------------- + +namespace caching { + namespace mapping_array_detail { + struct mapping { + uint64_t oblock_; + uint32_t flags_; + }; + + struct mapping_traits { + typedef base::le64 disk_type; + typedef mapping value_type; + typedef no_op_ref_counter ref_counter; + + static void unpack(disk_type const &disk, value_type &value); + static void pack(value_type const &value, disk_type &disk); + }; + + // FIXME: damage visitor stuff + } + + typedef persistent_data::array mapping_array; +} + +//---------------------------------------------------------------- + +#endif diff --git a/caching/metadata.h b/caching/metadata.h index 99f6c17..e94bea2 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -8,6 +8,8 @@ #include "persistent-data/transaction_manager.h" #include "caching/superblock.h" +#include "caching/hint_array.h" +#include "caching/mapping_array.h" //---------------------------------------------------------------- @@ -25,6 +27,13 @@ namespace caching { metadata(block_manager<>::ptr bm, open_type ot); metadata(block_manager<>::ptr bm, block_address metadata_snap); + + typedef persistent_data::transaction_manager tm; + tm::ptr tm_; + superblock_detail::superblock sb_; + checked_space_map::ptr metadata_sm_; + mapping_array::ptr mappings_; + //hint_array::ptr hints_; }; }; diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc new file mode 100644 index 0000000..499f13b --- /dev/null +++ b/caching/restore_emitter.cc @@ -0,0 +1,61 @@ +#include "caching/restore_emitter.h" +#include "caching/superblock.h" + +using namespace std; +using namespace caching; + +//---------------------------------------------------------------- + +namespace { + using namespace superblock_detail; + + class restorer : public emitter { + public: + restorer(metadata::ptr md) + : md_(md) { + } + + virtual void begin_superblock(std::string const &uuid, + pd::block_address block_size, + pd::block_address nr_cache_blocks, + std::string const &policy) { + } + + virtual void end_superblock() { + } + + virtual void begin_mappings() { + } + + virtual void end_mappings() { + } + + virtual void mapping(pd::block_address cblock, + pd::block_address oblock, + bool dirty) { + } + + virtual void begin_hints() { + } + + virtual void end_hints() { + } + + virtual void hint(pd::block_address cblock, + std::string const &data) { + } + + private: + metadata::ptr md_; + }; +} + +//---------------------------------------------------------------- + +emitter::ptr +caching::create_restore_emitter(metadata::ptr md) +{ + return emitter::ptr(new restorer(md)); +} + +//---------------------------------------------------------------- diff --git a/caching/restore_emitter.h b/caching/restore_emitter.h new file mode 100644 index 0000000..658dc68 --- /dev/null +++ b/caching/restore_emitter.h @@ -0,0 +1,15 @@ +#ifndef CACHE_RESTORE_EMITTER_H +#define CACHE_RESTORE_EMITTER_H + +#include "emitter.h" +#include "metadata.h" + +//---------------------------------------------------------------- + +namespace caching { + emitter::ptr create_restore_emitter(metadata::ptr md); +} + +//---------------------------------------------------------------- + +#endif From a05d75611e24a29e2bc9846498fac8d3f0809465 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 12 Sep 2013 14:21:25 +0100 Subject: [PATCH 15/80] mapping_array.cc --- Makefile.in | 2 ++ caching/mapping_array.cc | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 caching/mapping_array.cc diff --git a/Makefile.in b/Makefile.in index a72d18c..058e32e 100644 --- a/Makefile.in +++ b/Makefile.in @@ -37,6 +37,7 @@ SOURCE=\ base/error_state.cc \ \ caching/superblock.cc \ + caching/mapping_array.cc \ caching/metadata.cc \ caching/restore_emitter.cc \ caching/xml_format.cc \ @@ -236,6 +237,7 @@ CACHE_CHECK_SOURCE=\ persistent-data/space-maps/careful_alloc.cc \ persistent-data/transaction_manager.cc \ caching/superblock.cc \ + caching/mapping_array.cc \ caching/metadata.cc \ caching/restore_emitter.cc \ caching/xml_format.cc diff --git a/caching/mapping_array.cc b/caching/mapping_array.cc new file mode 100644 index 0000000..3e43d87 --- /dev/null +++ b/caching/mapping_array.cc @@ -0,0 +1,28 @@ +#include "caching/mapping_array.h" +#include "persistent-data/endian_utils.h" + +using namespace caching::mapping_array_detail; + +//---------------------------------------------------------------- + +namespace { + const uint64_t FLAGS_MASK = (1 << 16) - 1; +} + +void +mapping_traits::unpack(disk_type const &disk, value_type &value) +{ + uint64_t v = base::to_cpu(disk); + value.oblock_ = v >> 16; + value.flags_ = v & FLAGS_MASK; +} + +void +mapping_traits::pack(value_type const &value, disk_type &disk) +{ + uint64_t packed = value.oblock_ << 16; + packed = packed || (value.flags_ & FLAGS_MASK); + disk = base::to_disk(packed); +} + +//---------------------------------------------------------------- From 41864f041fd885a8ab397cbd8991f198de8ef05f Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 12 Sep 2013 14:42:59 +0100 Subject: [PATCH 16/80] hint_array pack/unpack --- caching/hint_array.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/caching/hint_array.h b/caching/hint_array.h index 507f362..ec89aeb 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -13,11 +13,16 @@ namespace caching { struct hint_traits { typedef unsigned char byte; typedef byte disk_type[WIDTH]; - typedef std::string value_type; + typedef byte value_type[WIDTH]; typedef no_op_ref_counter ref_counter; - static void unpack(disk_type const &disk, value_type &value); - static void pack(value_type const &value, disk_type &disk); + static void unpack(disk_type const &disk, value_type &value) { + ::memcpy(value, disk, sizeof(value)); + } + + static void pack(value_type const &value, disk_type &disk) { + ::memcpy(disk, value, sizeof(disk)); + } }; // FIXME: data visitor stuff From 4124460c843329e4e5ad6e4adea8e70775d1140f Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 16 Sep 2013 13:04:31 +0100 Subject: [PATCH 17/80] Update Gemfile.lock --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 7684f60..1a713da 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -8,22 +8,22 @@ GEM builder (3.2.2) childprocess (0.3.9) ffi (~> 1.0, >= 1.0.11) - cucumber (1.3.7) + cucumber (1.3.8) builder (>= 2.1.2) diff-lcs (>= 1.1.3) gherkin (~> 2.12.1) - multi_json (~> 1.7.5) + multi_json (>= 1.7.5, < 2.0) multi_test (>= 0.0.2) diff-lcs (1.2.4) ejt_command_line (0.0.2) ffi (1.9.0) gherkin (2.12.1) multi_json (~> 1.3) - multi_json (1.7.9) + multi_json (1.8.0) multi_test (0.0.2) rspec-expectations (2.14.2) diff-lcs (>= 1.1.3, < 2.0) - thinp_xml (0.0.8) + thinp_xml (0.0.9) ejt_command_line (= 0.0.2) PLATFORMS From 93b107701750468f456bc6f745b78cec691c292a Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 16 Sep 2013 13:05:58 +0100 Subject: [PATCH 18/80] Fix bad thinp_xml command line --- features/support/world_extensions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/support/world_extensions.rb b/features/support/world_extensions.rb index 1fc44a9..42b3ab6 100644 --- a/features/support/world_extensions.rb +++ b/features/support/world_extensions.rb @@ -39,7 +39,7 @@ module ThinpWorld # FIXME: we should really break out the xml stuff from # thinp-test-suite and put in a gem in this repo. def write_valid_xml(path) - `thinp_xml create --nr-thins uniform[4..9] --nr-mappings uniform[1000.5000] > #{path}` + `thinp_xml create --nr-thins uniform[4..9] --nr-mappings uniform[1000..5000] > #{path}` end private From 6638f7054f064138b8dcef6d540f55227f2d6ce0 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 16 Sep 2013 13:06:23 +0100 Subject: [PATCH 19/80] Test for missing input file --- features/thin_restore.feature | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/features/thin_restore.feature b/features/thin_restore.feature index a8e75fc..3984339 100644 --- a/features/thin_restore.feature +++ b/features/thin_restore.feature @@ -40,6 +40,10 @@ Feature: thin_restore No input file provided. """ + Scenario: input file not found + When I run thin_restore with -i foo.xml -o metadata.bin + Then it should fail + Scenario: missing output file When I run thin_restore with -i metadata.xml Then it should fail with: From ea424c4134a357f3460587a39df86a29eea7fef4 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 16 Sep 2013 13:41:55 +0100 Subject: [PATCH 20/80] Fixup some feature tests that need metadata.bin present They were failing for the wrong reason. --- features/cache_restore.feature | 2 ++ features/step_definitions/thin_steps.rb | 4 ++++ features/thin_restore.feature | 2 ++ 3 files changed, 8 insertions(+) diff --git a/features/cache_restore.feature b/features/cache_restore.feature index a38d4b4..3e8c016 100644 --- a/features/cache_restore.feature +++ b/features/cache_restore.feature @@ -34,6 +34,7 @@ Feature: thin_restore """ Scenario: missing input file + Given the dev file metadata.bin When I run cache_restore with -o metadata.bin Then it should fail with: """ @@ -41,6 +42,7 @@ Feature: thin_restore """ Scenario: input file not found + Given the dev file metadata.bin When I run cache_restore with -i foo.xml -o metadata.bin Then it should fail diff --git a/features/step_definitions/thin_steps.rb b/features/step_definitions/thin_steps.rb index 8d958d2..38fe6a3 100644 --- a/features/step_definitions/thin_steps.rb +++ b/features/step_definitions/thin_steps.rb @@ -7,6 +7,10 @@ Given(/^valid metadata$/) do run_simple("thin_restore -i #{xml_file} -o #{dev_file}") end +Given(/^the dev file metadata\.bin$/) do + run_simple("dd if=/dev/zero of=#{dev_file} bs=4k count=1024") +end + Given(/^a corrupt superblock$/) do in_current_dir do write_valid_xml(xml_file) diff --git a/features/thin_restore.feature b/features/thin_restore.feature index 3984339..d02ebb0 100644 --- a/features/thin_restore.feature +++ b/features/thin_restore.feature @@ -34,6 +34,7 @@ Feature: thin_restore """ Scenario: missing input file + Given the dev file metadata.bin When I run thin_restore with -o metadata.bin Then it should fail with: """ @@ -41,6 +42,7 @@ Feature: thin_restore """ Scenario: input file not found + Given the dev file metadata.bin When I run thin_restore with -i foo.xml -o metadata.bin Then it should fail From 14122d6875f7be67b9e85fed421bc53fb84a32cd Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 16 Sep 2013 13:42:39 +0100 Subject: [PATCH 21/80] Move check_file_exists() to file_utils.{h,cc} --- caching/cache_restore.cc | 10 ---------- persistent-data/file_utils.cc | 11 +++++++++++ persistent-data/file_utils.h | 5 +++++ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/caching/cache_restore.cc b/caching/cache_restore.cc index 62c4ea6..7e64d4c 100644 --- a/caching/cache_restore.cc +++ b/caching/cache_restore.cc @@ -18,16 +18,6 @@ using namespace std; //---------------------------------------------------------------- namespace { - void check_file_exists(string const &file) { - struct stat info; - int r = ::stat(file.c_str(), &info); - if (r) - throw runtime_error("Couldn't stat file"); - - if (!S_ISREG(info.st_mode)) - throw runtime_error("Not a regular file"); - } - int restore(string const &xml_file, string const &dev) { try { block_manager<>::ptr bm = open_bm(dev, block_io<>::READ_ONLY); diff --git a/persistent-data/file_utils.cc b/persistent-data/file_utils.cc index e5fef3f..a96aec7 100644 --- a/persistent-data/file_utils.cc +++ b/persistent-data/file_utils.cc @@ -54,4 +54,15 @@ persistent_data::open_bm(std::string const &dev_path, block_io<>::mode m) return block_manager<>::ptr(new block_manager<>(dev_path, nr_blocks, 1, m)); } +void +persistent_data::check_file_exists(string const &file) { + struct stat info; + int r = ::stat(file.c_str(), &info); + if (r) + throw runtime_error("Couldn't stat file"); + + if (!S_ISREG(info.st_mode)) + throw runtime_error("Not a regular file"); +} + //---------------------------------------------------------------- diff --git a/persistent-data/file_utils.h b/persistent-data/file_utils.h index c2d2d81..be1e492 100644 --- a/persistent-data/file_utils.h +++ b/persistent-data/file_utils.h @@ -3,11 +3,16 @@ #include "persistent-data/block.h" +#include + //---------------------------------------------------------------- +// FIXME: move to a different unit namespace persistent_data { persistent_data::block_address get_nr_blocks(string const &path); block_manager<>::ptr open_bm(std::string const &dev_path, block_io<>::mode m); + + void check_file_exists(std::string const &file); } //---------------------------------------------------------------- From 7ce306cb6d42dbce6712d9cb93ddd717521720cb Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 16 Sep 2013 13:43:06 +0100 Subject: [PATCH 22/80] [thin_restore] check the input file exists. Previously we were hanging if it didn't --- thin-provisioning/thin_restore.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/thin-provisioning/thin_restore.cc b/thin-provisioning/thin_restore.cc index ec26ad3..2aa9173 100644 --- a/thin-provisioning/thin_restore.cc +++ b/thin-provisioning/thin_restore.cc @@ -16,6 +16,7 @@ // with thin-provisioning-tools. If not, see // . +#include "persistent-data/file_utils.h" #include "thin-provisioning/emitter.h" #include "thin-provisioning/human_readable_format.h" #include "thin-provisioning/metadata.h" @@ -45,6 +46,8 @@ namespace { // The block size gets updated by the restorer. metadata::ptr md(new metadata(dev, metadata::CREATE, 128, 0)); emitter::ptr restorer = create_restore_emitter(md); + + check_file_exists(backup_file); ifstream in(backup_file.c_str(), ifstream::in); parse_xml(in, restorer); From cce3b5980ffaa0f575e2ff6962b064739903f6fd Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 16 Sep 2013 13:46:53 +0100 Subject: [PATCH 23/80] bump version to 0.2.7 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 3a4036f..b003284 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.2.5 +0.2.7 From c476de1756264d76f22d2fff7a06792dc7f09e68 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 18 Sep 2013 13:00:26 +0100 Subject: [PATCH 24/80] [cache_restore] more wip --- caching/metadata.cc | 73 +++++++++++++++++++++++-- caching/metadata.h | 3 +- caching/restore_emitter.cc | 46 +++++++++++++++- persistent-data/data-structures/array.h | 7 +-- 4 files changed, 118 insertions(+), 11 deletions(-) diff --git a/caching/metadata.cc b/caching/metadata.cc index 5985080..f9c02f2 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -1,17 +1,80 @@ #include "caching/metadata.h" +#include "persistent-data/space-maps/core.h" using namespace caching; //---------------------------------------------------------------- -metadata::metadata(block_manager<>::ptr bm, open_type ot) -{ +namespace { + using namespace superblock_detail; -} + unsigned const METADATA_CACHE_SIZE = 1024; -metadata::metadata(block_manager<>::ptr bm, block_address metadata_snap) -{ + // FIXME: duplication + transaction_manager::ptr + open_tm(block_manager<>::ptr bm) { + space_map::ptr sm(new core_map(bm->get_nr_blocks())); + sm->inc(SUPERBLOCK_LOCATION); + transaction_manager::ptr tm(new transaction_manager(bm, sm)); + return tm; + } + void + copy_space_maps(space_map::ptr lhs, space_map::ptr rhs) { + for (block_address b = 0; b < rhs->get_nr_blocks(); b++) { + uint32_t count = rhs->get_count(b); + if (count > 0) + lhs->set_count(b, rhs->get_count(b)); + } + } + + void init_superblock(superblock &sb) { +#if 0 + sb.magic_ = SUPERBLOCK_MAGIC; + sb.version_ = 1; + sb.data_mapping_root_ = mappings_->get_root(); + sb.device_details_root_ = details_->get_root(); + sb.data_block_size_ = data_block_size; + sb.metadata_block_size_ = MD_BLOCK_SIZE; + sb.metadata_nr_blocks_ = tm_->get_bm()->get_nr_blocks(); +#endif + } +} + +//---------------------------------------------------------------- + +metadata::metadata(block_manager<>::ptr bm, open_type ot) +{ + switch (ot) { + case OPEN: + throw runtime_error("not implemented"); + break; + + case CREATE: + tm_ = open_tm(bm); + space_map::ptr core = tm_->get_sm(); + metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); + copy_space_maps(metadata_sm_, core); + tm_->set_sm(metadata_sm_); + + mappings_ = mapping_array::ptr(new mapping_array(tm_, mapping_array::ref_counter())); +// hints_ = hint_array::ptr(new hint_array(tm_)); + + ::memset(&sb_, 0, sizeof(sb_)); + init_superblock(sb_); + } +} + +void +metadata::commit() +{ + metadata_sm_->commit(); + metadata_sm_->copy_root(&sb_.metadata_space_map_root, sizeof(sb_.metadata_space_map_root)); + sb_.mapping_root = mappings_->get_root(); + + write_ref superblock = tm_->get_bm()->superblock_zero(SUPERBLOCK_LOCATION, superblock_validator()); + superblock_disk *disk = reinterpret_cast(superblock.data().raw()); + superblock_traits::pack(sb_, *disk); } //---------------------------------------------------------------- diff --git a/caching/metadata.h b/caching/metadata.h index e94bea2..7b956a5 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -26,7 +26,8 @@ namespace caching { typedef boost::shared_ptr ptr; metadata(block_manager<>::ptr bm, open_type ot); - metadata(block_manager<>::ptr bm, block_address metadata_snap); + + void commit(); typedef persistent_data::transaction_manager tm; tm::ptr tm_; diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index 499f13b..efca1de 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -12,27 +12,70 @@ namespace { class restorer : public emitter { public: restorer(metadata::ptr md) - : md_(md) { + : in_superblock_(false), + md_(md) { + } + + virtual ~restorer() { } virtual void begin_superblock(std::string const &uuid, pd::block_address block_size, pd::block_address nr_cache_blocks, std::string const &policy) { + + superblock &sb = md_->sb_; + + sb.flags = 0; + memset(sb.uuid, 0, sizeof(sb.uuid)); + sb.magic = caching::superblock_detail::SUPERBLOCK_MAGIC; + sb.version = 0; // FIXME: fix + // strncpy(sb.policy_name, policy.c_str(), sizeof(sb.policy_name)); + memset(sb.policy_version, 0, sizeof(sb.policy_version)); + sb.policy_hint_size = 0; // FIXME: fix + + memset(sb.metadata_space_map_root, 0, sizeof(sb.metadata_space_map_root)); + sb.mapping_root = 0; + sb.hint_root = 0; + + sb.discard_root = 0; + sb.discard_block_size = 0; + sb.discard_nr_blocks = 0; + + sb.data_block_size = block_size; + sb.metadata_block_size = 0; + sb.cache_blocks = nr_cache_blocks; + + sb.compat_flags = 0; + sb.compat_ro_flags = 0; + sb.incompat_flags = 0; + + sb.read_hits = 0; + sb.read_misses = 0; + sb.write_hits = 0; + sb.write_misses = 0; } virtual void end_superblock() { + md_->commit(); } virtual void begin_mappings() { + // noop } virtual void end_mappings() { + // noop } virtual void mapping(pd::block_address cblock, pd::block_address oblock, bool dirty) { + mapping_array_detail::mapping m; + m.oblock_ = oblock; + m.flags_ = 0; + + md_->mappings_->set(cblock, m); } virtual void begin_hints() { @@ -46,6 +89,7 @@ namespace { } private: + bool in_superblock_; metadata::ptr md_; }; } diff --git a/persistent-data/data-structures/array.h b/persistent-data/data-structures/array.h index f69d627..9b4c9bd 100644 --- a/persistent-data/data-structures/array.h +++ b/persistent-data/data-structures/array.h @@ -128,9 +128,9 @@ namespace persistent_data { typedef boost::shared_ptr > ptr; typedef typename ValueTraits::value_type value_type; + typedef typename ValueTraits::ref_counter ref_counter; - array(tm_ptr tm, - typename ValueTraits::ref_counter rc) + array(tm_ptr tm, ref_counter rc) : tm_(tm), entries_per_block_(rblock::calc_max_entries()), nr_entries_(0), @@ -140,8 +140,7 @@ namespace persistent_data { validator_(new block_manager<>::noop_validator()) { } - array(tm_ptr tm, - typename ValueTraits::ref_counter rc, + array(tm_ptr tm, ref_counter rc, block_address root, unsigned nr_entries) : tm_(tm), From 61e90998c077e07369c76d31a35f8a75bae3e631 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 19 Sep 2013 13:45:56 +0100 Subject: [PATCH 25/80] [cache_dump, cache_restore] restore/dump cycle works --- Gemfile.lock | 2 +- caching/cache_dump.cc | 34 ++++++++++-- caching/cache_restore.cc | 2 +- caching/mapping_array.h | 5 ++ caching/metadata.cc | 71 +++++++++++++++--------- caching/metadata.h | 6 ++ caching/restore_emitter.cc | 12 +++- caching/superblock.cc | 21 ++++++- caching/superblock.h | 7 +++ features/cache_dump.feature | 1 + features/cache_restore.feature | 1 + features/step_definitions/cache_steps.rb | 4 +- thin-provisioning/superblock.h | 2 + 13 files changed, 129 insertions(+), 39 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1a713da..f7aad93 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,7 +23,7 @@ GEM multi_test (0.0.2) rspec-expectations (2.14.2) diff-lcs (>= 1.1.3, < 2.0) - thinp_xml (0.0.9) + thinp_xml (0.0.10) ejt_command_line (= 0.0.2) PLATFORMS diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index 73467cd..31ca00a 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -4,14 +4,24 @@ #include #include "version.h" +#include "caching/metadata.h" #include "caching/xml_format.h" +#include "persistent-data/file_utils.h" using namespace std; using namespace caching; +using namespace caching::superblock_detail; //---------------------------------------------------------------- namespace { + string to_string(unsigned char const *data) { + // FIXME: we're assuming the data is zero terminated here + return std::string(reinterpret_cast(data)); + } + + //-------------------------------- + string const STDOUT_PATH("-"); bool want_stdout(string const &output) { @@ -19,16 +29,30 @@ namespace { } int dump_(string const &dev, ostream &out) { + block_manager<>::ptr bm = open_bm(dev, block_io<>::READ_ONLY); + metadata::ptr md(new metadata(bm, metadata::OPEN)); emitter::ptr e = create_xml_emitter(out); + + superblock const &sb = md->sb_; + e->begin_superblock(to_string(sb.uuid), sb.data_block_size, sb.cache_blocks, to_string(sb.policy_name)); + e->end_superblock(); + return 0; } int dump(string const &dev, string const &output) { - if (want_stdout(output)) { - ofstream out(output.c_str()); - return dump_(dev, out); - } else - return dump_(dev, cout); + try { + if (want_stdout(output)) + return dump_(dev, cout); + else { + ofstream out(output.c_str()); + return dump_(dev, out); + } + + } catch (std::exception &e) { + cerr << e.what() << endl; + return 1; + } } void usage(ostream &out, string const &cmd) { diff --git a/caching/cache_restore.cc b/caching/cache_restore.cc index 7e64d4c..22cc452 100644 --- a/caching/cache_restore.cc +++ b/caching/cache_restore.cc @@ -20,7 +20,7 @@ using namespace std; namespace { int restore(string const &xml_file, string const &dev) { try { - block_manager<>::ptr bm = open_bm(dev, block_io<>::READ_ONLY); + block_manager<>::ptr bm = open_bm(dev, block_io<>::READ_WRITE); metadata::ptr md(new metadata(bm, metadata::CREATE)); emitter::ptr restorer = create_restore_emitter(md); diff --git a/caching/mapping_array.h b/caching/mapping_array.h index a938466..030ec4f 100644 --- a/caching/mapping_array.h +++ b/caching/mapping_array.h @@ -7,6 +7,11 @@ namespace caching { namespace mapping_array_detail { + enum mapping_flags { + M_VALID = 1, + M_DIRTY = 2 + }; + struct mapping { uint64_t oblock_; uint32_t flags_; diff --git a/caching/metadata.cc b/caching/metadata.cc index f9c02f2..d152874 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -1,4 +1,5 @@ #include "caching/metadata.h" +#include "caching/superblock.h" #include "persistent-data/space-maps/core.h" using namespace caching; @@ -27,18 +28,6 @@ namespace { lhs->set_count(b, rhs->get_count(b)); } } - - void init_superblock(superblock &sb) { -#if 0 - sb.magic_ = SUPERBLOCK_MAGIC; - sb.version_ = 1; - sb.data_mapping_root_ = mappings_->get_root(); - sb.device_details_root_ = details_->get_root(); - sb.data_block_size_ = data_block_size; - sb.metadata_block_size_ = MD_BLOCK_SIZE; - sb.metadata_nr_blocks_ = tm_->get_bm()->get_nr_blocks(); -#endif - } } //---------------------------------------------------------------- @@ -46,22 +35,16 @@ namespace { metadata::metadata(block_manager<>::ptr bm, open_type ot) { switch (ot) { - case OPEN: - throw runtime_error("not implemented"); + case CREATE: + create_metadata(bm); break; - case CREATE: - tm_ = open_tm(bm); - space_map::ptr core = tm_->get_sm(); - metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); - copy_space_maps(metadata_sm_, core); - tm_->set_sm(metadata_sm_); + case OPEN: + open_metadata(bm); + break; - mappings_ = mapping_array::ptr(new mapping_array(tm_, mapping_array::ref_counter())); -// hints_ = hint_array::ptr(new hint_array(tm_)); - - ::memset(&sb_, 0, sizeof(sb_)); - init_superblock(sb_); + default: + throw runtime_error("unhandled open_type"); } } @@ -77,4 +60,42 @@ metadata::commit() superblock_traits::pack(sb_, *disk); } +void +metadata::init_superblock() +{ +#if 0 + sb_.magic_ = SUPERBLOCK_MAGIC; + sb_.version_ = 1; + sb_.data_mapping_root_ = mappings_->get_root(); + sb_.device_details_root_ = details_->get_root(); + sb_.data_block_size_ = data_block_size; + sb_.metadata_block_size_ = MD_BLOCK_SIZE; + sb_.metadata_nr_blocks_ = tm_->get_bm()->get_nr_blocks(); +#endif +} + +void +metadata::create_metadata(block_manager<>::ptr bm) +{ + tm_ = open_tm(bm); + + ::memset(&sb_, 0, sizeof(sb_)); + init_superblock(); + + space_map::ptr core = tm_->get_sm(); + metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); + copy_space_maps(metadata_sm_, core); + tm_->set_sm(metadata_sm_); + + mappings_ = mapping_array::ptr(new mapping_array(tm_, mapping_array::ref_counter())); +// hints_ = hint_array::ptr(new hint_array(tm_)); +} + +void +metadata::open_metadata(block_manager<>::ptr bm) +{ + tm_ = open_tm(bm); + sb_ = read_superblock(tm_->get_bm()); +} + //---------------------------------------------------------------- diff --git a/caching/metadata.h b/caching/metadata.h index 7b956a5..20cb3d5 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -35,6 +35,12 @@ namespace caching { checked_space_map::ptr metadata_sm_; mapping_array::ptr mappings_; //hint_array::ptr hints_; + + private: + void init_superblock(); + + void create_metadata(block_manager<>::ptr bm); + void open_metadata(block_manager<>::ptr bm); }; }; diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index efca1de..f871e61 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -1,14 +1,15 @@ #include "caching/restore_emitter.h" #include "caching/superblock.h" +#include "caching/mapping_array.h" -using namespace std; using namespace caching; +using namespace mapping_array_detail; +using namespace std; +using namespace superblock_detail; //---------------------------------------------------------------- namespace { - using namespace superblock_detail; - class restorer : public emitter { public: restorer(metadata::ptr md) @@ -54,6 +55,11 @@ namespace { sb.read_misses = 0; sb.write_hits = 0; sb.write_misses = 0; + + struct mapping unmapped_value; + unmapped_value.oblock_ = 0; + unmapped_value.flags_ = 0; + md_->mappings_->grow(nr_cache_blocks, unmapped_value); } virtual void end_superblock() { diff --git a/caching/superblock.cc b/caching/superblock.cc index 4a8b153..e783e16 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -141,8 +141,7 @@ caching::superblock_validator() //-------------------------------- superblock_detail::superblock -caching::read_superblock(persistent_data::block_manager<>::ptr bm, - persistent_data::block_address location) +caching::read_superblock(block_manager<>::ptr bm, block_address location) { using namespace superblock_detail; @@ -160,6 +159,24 @@ caching::read_superblock(persistent_data::block_manager<>::ptr bm) return read_superblock(bm, SUPERBLOCK_LOCATION); } +void +caching::write_superblock(block_manager<>::ptr bm, + block_address location, + superblock_detail::superblock const &sb) +{ + using namespace superblock_detail; + + block_manager<>::write_ref w = bm->write_lock(location, superblock_validator()); + superblock_traits::pack(sb, *reinterpret_cast(&w.data())); +} + +void +caching::write_superblock(block_manager<>::ptr bm, + superblock_detail::superblock const &sb) +{ + write_superblock(bm, SUPERBLOCK_LOCATION, sb); +} + void caching::check_superblock(persistent_data::block_manager<>::ptr bm, superblock_detail::damage_visitor &visitor) diff --git a/caching/superblock.h b/caching/superblock.h index f6b9a0c..92db272 100644 --- a/caching/superblock.h +++ b/caching/superblock.h @@ -130,6 +130,13 @@ namespace caching { superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm); superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm, persistent_data::block_address location); + + void write_superblock(persistent_data::block_manager<>::ptr bm, + superblock_detail::superblock const &sb); + void write_superblock(persistent_data::block_manager<>::ptr bm, + persistent_data::block_address location, + superblock_detail::superblock const &sb); + void check_superblock(persistent_data::block_manager<>::ptr bm, superblock_detail::damage_visitor &visitor); } diff --git a/features/cache_dump.feature b/features/cache_dump.feature index 15bcecd..6ca41f6 100644 --- a/features/cache_dump.feature +++ b/features/cache_dump.feature @@ -41,6 +41,7 @@ Feature: cache_dump No input file provided. """ + @announce Scenario: dump/restore is a noop Given valid cache metadata When I cache_dump diff --git a/features/cache_restore.feature b/features/cache_restore.feature index 3e8c016..6b649cb 100644 --- a/features/cache_restore.feature +++ b/features/cache_restore.feature @@ -53,6 +53,7 @@ Feature: thin_restore No output file provided. """ + @announce Scenario: successfully restores a valid xml file Given a small xml file And an empty dev file diff --git a/features/step_definitions/cache_steps.rb b/features/step_definitions/cache_steps.rb index 342fd74..68c5739 100644 --- a/features/step_definitions/cache_steps.rb +++ b/features/step_definitions/cache_steps.rb @@ -74,14 +74,14 @@ end Given(/^valid cache metadata$/) do in_current_dir do - system("cache_xml create --nr-cache-blocks uniform[1000-5000] --nr-mappings uniform[500-1000] > #{xml_file}") + system("cache_xml create --nr-cache-blocks uniform[1000..5000] --nr-mappings uniform[500..1000] > #{xml_file}") end run_simple("dd if=/dev/zero of=#{dev_file} bs=4k count=1024") run_simple("cache_restore -i #{xml_file} -o #{dev_file}") end -Then(/^cache dumps (\d+) and (\d+) should be identical$/) do |arg1, arg2| +Then(/^cache dumps (\d+) and (\d+) should be identical$/) do |d1, d2| run_simple("diff -ub #{dump_files[d1.to_i]} #{dump_files[d2.to_i]}", true) end diff --git a/thin-provisioning/superblock.h b/thin-provisioning/superblock.h index a9377b0..d6d78e3 100644 --- a/thin-provisioning/superblock.h +++ b/thin-provisioning/superblock.h @@ -122,6 +122,8 @@ namespace thin_provisioning { persistent_data::block_manager<>::validator::ptr superblock_validator(); + // FIXME: should we put init_superblock in here too? + superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm); superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm, persistent_data::block_address location); void check_superblock(persistent_data::block_manager<>::ptr bm, From dc97e0ea4efc907dadb998c0245124af3420ee9f Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 19 Sep 2013 13:52:29 +0100 Subject: [PATCH 26/80] Update ignore file --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index e06e498..7b84141 100644 --- a/.gitignore +++ b/.gitignore @@ -1,17 +1,20 @@ *~ *.o +*.a *.gmo *_t *.d *.d.[0-9]* test.data cachegrind.* +\#*\# thin_check thin_dump thin_restore thin_repair thin_rmap +thin_metadata_size cache_check cache_dump From 2e58332e49943cc308231246f1fac29f63c3f1a6 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 23 Sep 2013 11:15:41 +0100 Subject: [PATCH 27/80] [cache_restore/dump] mappings dump restore ok now --- caching/cache_dump.cc | 14 ++++++++++++++ caching/mapping_array.cc | 2 +- caching/metadata.cc | 37 ++++++++++++++++++++++++++++++------- caching/metadata.h | 4 ++++ caching/restore_emitter.cc | 5 ++++- 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index 31ca00a..0cfb806 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -4,12 +4,14 @@ #include #include "version.h" +#include "caching/mapping_array.h" #include "caching/metadata.h" #include "caching/xml_format.h" #include "persistent-data/file_utils.h" using namespace std; using namespace caching; +using namespace caching::mapping_array_detail; using namespace caching::superblock_detail; //---------------------------------------------------------------- @@ -35,6 +37,18 @@ namespace { superblock const &sb = md->sb_; e->begin_superblock(to_string(sb.uuid), sb.data_block_size, sb.cache_blocks, to_string(sb.policy_name)); + + e->begin_mappings(); + + for (unsigned cblock = 0; cblock < sb.cache_blocks; cblock++) { + mapping m = md->mappings_->get(cblock); + + if (m.flags_ & M_VALID) + e->mapping(cblock, m.oblock_, m.flags_ & M_DIRTY); + } + + e->end_mappings(); + e->end_superblock(); return 0; diff --git a/caching/mapping_array.cc b/caching/mapping_array.cc index 3e43d87..c26460e 100644 --- a/caching/mapping_array.cc +++ b/caching/mapping_array.cc @@ -21,7 +21,7 @@ void mapping_traits::pack(value_type const &value, disk_type &disk) { uint64_t packed = value.oblock_ << 16; - packed = packed || (value.flags_ & FLAGS_MASK); + packed = packed | (value.flags_ & FLAGS_MASK); disk = base::to_disk(packed); } diff --git a/caching/metadata.cc b/caching/metadata.cc index d152874..66954d5 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -51,13 +51,9 @@ metadata::metadata(block_manager<>::ptr bm, open_type ot) void metadata::commit() { - metadata_sm_->commit(); - metadata_sm_->copy_root(&sb_.metadata_space_map_root, sizeof(sb_.metadata_space_map_root)); - sb_.mapping_root = mappings_->get_root(); - - write_ref superblock = tm_->get_bm()->superblock_zero(SUPERBLOCK_LOCATION, superblock_validator()); - superblock_disk *disk = reinterpret_cast(superblock.data().raw()); - superblock_traits::pack(sb_, *disk); + commit_space_map(); + commit_mappings(); + commit_superblock(); } void @@ -96,6 +92,33 @@ metadata::open_metadata(block_manager<>::ptr bm) { tm_ = open_tm(bm); sb_ = read_superblock(tm_->get_bm()); + + mappings_ = mapping_array::ptr( + new mapping_array(tm_, + mapping_array::ref_counter(), + sb_.mapping_root, + sb_.cache_blocks)); +} + +void +metadata::commit_space_map() +{ + metadata_sm_->commit(); + metadata_sm_->copy_root(&sb_.metadata_space_map_root, sizeof(sb_.metadata_space_map_root)); +} + +void +metadata::commit_mappings() +{ + sb_.mapping_root = mappings_->get_root(); +} + +void +metadata::commit_superblock() +{ + write_ref superblock = tm_->get_bm()->superblock_zero(SUPERBLOCK_LOCATION, superblock_validator()); + superblock_disk *disk = reinterpret_cast(superblock.data().raw()); + superblock_traits::pack(sb_, *disk); } //---------------------------------------------------------------- diff --git a/caching/metadata.h b/caching/metadata.h index 20cb3d5..0556125 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -41,6 +41,10 @@ namespace caching { void create_metadata(block_manager<>::ptr bm); void open_metadata(block_manager<>::ptr bm); + + void commit_space_map(); + void commit_mappings(); + void commit_superblock(); }; }; diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index f871e61..c705a25 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -79,7 +79,10 @@ namespace { bool dirty) { mapping_array_detail::mapping m; m.oblock_ = oblock; - m.flags_ = 0; + m.flags_ = M_VALID; + + if (dirty) + m.flags_ = m.flags_ | M_DIRTY; md_->mappings_->set(cblock, m); } From 17f7c982f253a4a4739e11edc8302b905bee61b4 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 24 Sep 2013 12:00:09 +0100 Subject: [PATCH 28/80] [caching] hint support --- Makefile.in | 2 + caching/hint_array.cc | 147 ++++++++++++++++++++++++ caching/hint_array.h | 30 ++++- caching/metadata.cc | 7 ++ caching/metadata.h | 3 +- persistent-data/data-structures/array.h | 9 +- 6 files changed, 192 insertions(+), 6 deletions(-) create mode 100644 caching/hint_array.cc diff --git a/Makefile.in b/Makefile.in index 058e32e..ddabacb 100644 --- a/Makefile.in +++ b/Makefile.in @@ -36,6 +36,7 @@ all: $(PROGRAMS) SOURCE=\ base/error_state.cc \ \ + caching/hint_array.cc \ caching/superblock.cc \ caching/mapping_array.cc \ caching/metadata.cc \ @@ -236,6 +237,7 @@ CACHE_CHECK_SOURCE=\ persistent-data/space-maps/recursive.cc \ persistent-data/space-maps/careful_alloc.cc \ persistent-data/transaction_manager.cc \ + caching/hint_array.cc \ caching/superblock.cc \ caching/mapping_array.cc \ caching/metadata.cc \ diff --git a/caching/hint_array.cc b/caching/hint_array.cc new file mode 100644 index 0000000..f0cdddf --- /dev/null +++ b/caching/hint_array.cc @@ -0,0 +1,147 @@ +#include "caching/hint_array.h" + +using namespace boost; +using namespace caching; +using namespace persistent_data; + +//---------------------------------------------------------------- + +namespace { + using namespace caching::hint_array_detail; + + // We've got into a bit of a mess here. Templates are compile + // time, and we don't know the hint width until run time. We're + // going to have to provide specialisation for all legal widths and + // use the appropriate one. + +#define all_widths \ + xx(4); xx(8); xx(12); xx(16); xx(20); xx(24); xx(28); xx(32);\ + xx(36); xx(40); xx(44); xx(48); xx(52); xx(56); xx(60); xx(64); \ + xx(68); xx(72); xx(76); xx(80); xx(84); xx(88); xx(92); xx(96); \ + xx(100); xx(104); xx(108); xx(112); xx(116); xx(120); xx(124); xx(128); + + template + shared_ptr mk_array(transaction_manager::ptr tm) { + typedef hint_traits traits; + typedef array ha; + + shared_ptr r = typename ha::ptr(new ha(tm, typename traits::ref_counter())); + + return r; + } + + shared_ptr mk_array(transaction_manager::ptr tm, uint32_t width) { + switch (width) { +#define xx(n) case n: return mk_array(tm) + + all_widths +#undef xx + } + + // never get here + return shared_ptr(); + } + + //-------------------------------- + + template + shared_ptr mk_array(transaction_manager::ptr tm, block_address root, unsigned nr_entries) { + typedef hint_traits traits; + typedef array ha; + + shared_ptr r = typename ha::ptr(new ha(tm, typename traits::ref_counter(), root, nr_entries)); + + return r; + } + + shared_ptr mk_array(transaction_manager::ptr tm, uint32_t width, block_address root, unsigned nr_entries) { + switch (width) { +#define xx(n) case n: return mk_array(tm, root, nr_entries) + all_widths +#undef xx + } + + // never get here + return shared_ptr(); + } + + //-------------------------------- + + template + void get_hint(shared_ptr base, unsigned index, vector &data) { + typedef hint_traits traits; + typedef array ha; + + shared_ptr a = dynamic_pointer_cast(base); + if (!a) + throw runtime_error("internal error: couldn't cast hint array"); + + data = a->get(index); + } + + void get_hint_(uint32_t width, shared_ptr base, unsigned index, vector &data) { + switch (width) { +#define xx(n) case n: return get_hint(base, index, data) + all_widths +#undef xx + } + } + + //-------------------------------- + + template + void set_hint(shared_ptr base, unsigned index, vector const &data) { + typedef hint_traits traits; + typedef array ha; + + shared_ptr a = dynamic_pointer_cast(base); + if (!a) + throw runtime_error("internal error: couldn't cast hint array"); + + a->set(index, data); + } + + void set_hint_(uint32_t width, shared_ptr base, + unsigned index, vector const &data) { + switch (width) { +#define xx(n) case n: return set_hint(base, index, data) + all_widths +#undef xx + } + } +} + +//---------------------------------------------------------------- + +hint_array::hint_array(tm_ptr tm, unsigned width) + : width_(width), + impl_(mk_array(tm, width)) +{ +} + +hint_array::hint_array(typename hint_array::tm_ptr tm, unsigned width, + block_address root, unsigned nr_entries) + : width_(width), + impl_(mk_array(tm, width, root, nr_entries)) +{ +} + +block_address +hint_array::get_root() const +{ + return impl_->get_root(); +} + +void +hint_array::get_hint(unsigned index, vector &data) const +{ + get_hint_(width_, impl_, index, data); +} + +void +hint_array::set_hint(unsigned index, vector const &data) +{ + set_hint_(width_, impl_, index, data); +} + +//---------------------------------------------------------------- diff --git a/caching/hint_array.h b/caching/hint_array.h index ec89aeb..dab0613 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -13,22 +13,44 @@ namespace caching { struct hint_traits { typedef unsigned char byte; typedef byte disk_type[WIDTH]; - typedef byte value_type[WIDTH]; + typedef vector value_type; typedef no_op_ref_counter ref_counter; + // FIXME: slow copying for now static void unpack(disk_type const &disk, value_type &value) { - ::memcpy(value, disk, sizeof(value)); + for (unsigned byte = 0; byte < WIDTH; byte++) + value[byte] = disk[byte]; } static void pack(value_type const &value, disk_type &disk) { - ::memcpy(disk, value, sizeof(disk)); + for (unsigned byte = 0; byte < WIDTH; byte++) + disk[byte] = value[byte]; } }; // FIXME: data visitor stuff } -// typedef persistent_data::array hint_array; + class hint_array { + public: + typedef boost::shared_ptr ptr; + typedef typename persistent_data::transaction_manager::ptr tm_ptr; + + hint_array(tm_ptr tm, unsigned width); + hint_array(tm_ptr tm, unsigned width, block_address root, unsigned nr_entries); + + unsigned get_nr_entries() const; + + void grow(unsigned new_nr_entries, void const *v); + + block_address get_root() const; + void get_hint(unsigned index, vector &data) const; + void set_hint(unsigned index, vector const &data); + + private: + unsigned width_; + boost::shared_ptr impl_; + }; } //---------------------------------------------------------------- diff --git a/caching/metadata.cc b/caching/metadata.cc index 66954d5..4e26657 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -53,6 +53,7 @@ metadata::commit() { commit_space_map(); commit_mappings(); + commit_hints(); commit_superblock(); } @@ -113,6 +114,12 @@ metadata::commit_mappings() sb_.mapping_root = mappings_->get_root(); } +void +metadata::commit_hints() +{ + sb_.hint_root = hints_->get_root(); +} + void metadata::commit_superblock() { diff --git a/caching/metadata.h b/caching/metadata.h index 0556125..d4a6cd2 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -34,7 +34,7 @@ namespace caching { superblock_detail::superblock sb_; checked_space_map::ptr metadata_sm_; mapping_array::ptr mappings_; - //hint_array::ptr hints_; + hint_array::ptr hints_; private: void init_superblock(); @@ -44,6 +44,7 @@ namespace caching { void commit_space_map(); void commit_mappings(); + void commit_hints(); void commit_superblock(); }; }; diff --git a/persistent-data/data-structures/array.h b/persistent-data/data-structures/array.h index 9b4c9bd..7c4c4fe 100644 --- a/persistent-data/data-structures/array.h +++ b/persistent-data/data-structures/array.h @@ -67,8 +67,15 @@ namespace persistent_data { }; } + class array_base { + public: + virtual ~array_base() {} + virtual void set_root(block_address root) = 0; + virtual block_address get_root() const = 0; + }; + template - class array { + class array : public array_base { public: class block_ref_counter : public ref_counter { public: From 8408ab12f808a4e5c7b0e5508972c165ec9e0d84 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 24 Sep 2013 12:12:57 +0100 Subject: [PATCH 29/80] [caching] add grow method to the hint_array wrapper --- caching/hint_array.cc | 29 +++++++++++++++++++++++++++++ caching/hint_array.h | 2 ++ 2 files changed, 31 insertions(+) diff --git a/caching/hint_array.cc b/caching/hint_array.cc index f0cdddf..0c6c39d 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -106,6 +106,29 @@ namespace { switch (width) { #define xx(n) case n: return set_hint(base, index, data) all_widths +#undef xx + } + } + + //-------------------------------- + + template + void grow(shared_ptr base, unsigned new_nr_entries, vector const &value) { + typedef hint_traits traits; + typedef array ha; + + shared_ptr a = dynamic_pointer_cast(base); + if (!a) + throw runtime_error("internal error: couldn't cast hint array"); + a->grow(new_nr_entries, value); + } + + void grow_(uint32_t width, shared_ptr base, + unsigned new_nr_entries, vector const &value) + { + switch (width) { +#define xx(n) case n: return grow(base, new_nr_entries, value) + all_widths #undef xx } } @@ -144,4 +167,10 @@ hint_array::set_hint(unsigned index, vector const &data) set_hint_(width_, impl_, index, data); } +void +hint_array::grow(unsigned new_nr_entries, vector const &value) +{ + grow_(width_, impl_, new_nr_entries, value); +} + //---------------------------------------------------------------- diff --git a/caching/hint_array.h b/caching/hint_array.h index dab0613..dd8d2a5 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -47,6 +47,8 @@ namespace caching { void get_hint(unsigned index, vector &data) const; void set_hint(unsigned index, vector const &data); + void grow(unsigned new_nr_entries, vector const &value); + private: unsigned width_; boost::shared_ptr impl_; From bd1a18929833afcc88f170e7a878370b97320900 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 24 Sep 2013 12:37:27 +0100 Subject: [PATCH 30/80] [caching] change the emitter->hint value type --- caching/emitter.h | 8 ++++---- caching/restore_emitter.cc | 5 ++++- caching/xml_format.cc | 10 +++++----- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/caching/emitter.h b/caching/emitter.h index a2a84dc..c70b56e 100644 --- a/caching/emitter.h +++ b/caching/emitter.h @@ -1,11 +1,11 @@ #ifndef CACHE_EMITTER_H #define CACHE_EMITTER_H -#include -#include - #include "persistent-data/block.h" +#include +#include + //---------------------------------------------------------------- namespace caching { @@ -35,7 +35,7 @@ namespace caching { virtual void end_hints() = 0; virtual void hint(pd::block_address cblock, - std::string const &data) = 0; + std::vector const &data) = 0; }; } diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index c705a25..8269ad7 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -88,13 +88,16 @@ namespace { } virtual void begin_hints() { + // noop } virtual void end_hints() { + // noop } virtual void hint(pd::block_address cblock, - std::string const &data) { + vector const &data) { + } private: diff --git a/caching/xml_format.cc b/caching/xml_format.cc index 9d0f439..f3b9610 100644 --- a/caching/xml_format.cc +++ b/caching/xml_format.cc @@ -12,12 +12,12 @@ using namespace std; namespace { // base64 encoding? - string encode(string const &data) { - return data; + string encode(vector const &data) { + return ""; } - string decode(string const &data) { - return data; + vector decode(string const &data) { + return vector(); } //-------------------------------- @@ -84,7 +84,7 @@ namespace { } virtual void hint(block_address cblock, - std::string const &data) { + vector const &data) { out_ << " Date: Thu, 26 Sep 2013 11:36:01 +0100 Subject: [PATCH 31/80] [cache_restore/dump] hint_width work. dump/restore cycle works again. --- Gemfile.lock | 4 ++-- caching/cache_dump.cc | 4 +++- caching/emitter.h | 3 ++- caching/hint_array.cc | 4 ++++ caching/hint_array.h | 4 ++-- caching/metadata.cc | 12 +++++++++++- caching/metadata.h | 2 ++ caching/restore_emitter.cc | 14 ++++++++++---- caching/superblock.cc | 1 + caching/xml_format.cc | 9 ++++++--- 10 files changed, 43 insertions(+), 14 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f7aad93..5e49ad7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -21,9 +21,9 @@ GEM multi_json (~> 1.3) multi_json (1.8.0) multi_test (0.0.2) - rspec-expectations (2.14.2) + rspec-expectations (2.14.3) diff-lcs (>= 1.1.3, < 2.0) - thinp_xml (0.0.10) + thinp_xml (0.0.12) ejt_command_line (= 0.0.2) PLATFORMS diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index 0cfb806..2702820 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -36,7 +36,9 @@ namespace { emitter::ptr e = create_xml_emitter(out); superblock const &sb = md->sb_; - e->begin_superblock(to_string(sb.uuid), sb.data_block_size, sb.cache_blocks, to_string(sb.policy_name)); + e->begin_superblock(to_string(sb.uuid), sb.data_block_size, + sb.cache_blocks, to_string(sb.policy_name), + sb.policy_hint_size); e->begin_mappings(); diff --git a/caching/emitter.h b/caching/emitter.h index c70b56e..1563cdc 100644 --- a/caching/emitter.h +++ b/caching/emitter.h @@ -20,7 +20,8 @@ namespace caching { virtual void begin_superblock(std::string const &uuid, pd::block_address block_size, pd::block_address nr_cache_blocks, - std::string const &policy) = 0; + std::string const &policy, + size_t hint_width) = 0; virtual void end_superblock() = 0; diff --git a/caching/hint_array.cc b/caching/hint_array.cc index 0c6c39d..5e2375a 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -36,6 +36,8 @@ namespace { all_widths #undef xx + default: + throw runtime_error("invalid hint width"); } // never get here @@ -59,6 +61,8 @@ namespace { #define xx(n) case n: return mk_array(tm, root, nr_entries) all_widths #undef xx + default: + throw runtime_error("invalid hint width"); } // never get here diff --git a/caching/hint_array.h b/caching/hint_array.h index dd8d2a5..6bb9884 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -19,12 +19,12 @@ namespace caching { // FIXME: slow copying for now static void unpack(disk_type const &disk, value_type &value) { for (unsigned byte = 0; byte < WIDTH; byte++) - value[byte] = disk[byte]; + value.at(byte) = disk[byte]; } static void pack(value_type const &value, disk_type &disk) { for (unsigned byte = 0; byte < WIDTH; byte++) - disk[byte] = value[byte]; + disk[byte] = value.at(byte); } }; diff --git a/caching/metadata.cc b/caching/metadata.cc index 4e26657..90a1dd3 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -57,6 +57,14 @@ metadata::commit() commit_superblock(); } +void +metadata::setup_hint_array(size_t width) +{ + if (width > 0) + hints_ = hint_array::ptr( + new hint_array(tm_, width)); +} + void metadata::init_superblock() { @@ -85,7 +93,9 @@ metadata::create_metadata(block_manager<>::ptr bm) tm_->set_sm(metadata_sm_); mappings_ = mapping_array::ptr(new mapping_array(tm_, mapping_array::ref_counter())); -// hints_ = hint_array::ptr(new hint_array(tm_)); + + // We can't instantiate the hint array yet, since we don't know the + // hint width. } void diff --git a/caching/metadata.h b/caching/metadata.h index d4a6cd2..e4fe965 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -28,6 +28,8 @@ namespace caching { metadata(block_manager<>::ptr bm, open_type ot); void commit(); + void setup_hint_array(size_t width); + typedef persistent_data::transaction_manager tm; tm::ptr tm_; diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index 8269ad7..35654da 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -23,7 +23,8 @@ namespace { virtual void begin_superblock(std::string const &uuid, pd::block_address block_size, pd::block_address nr_cache_blocks, - std::string const &policy) { + std::string const &policy, + size_t hint_width) { superblock &sb = md_->sb_; @@ -31,11 +32,13 @@ namespace { memset(sb.uuid, 0, sizeof(sb.uuid)); sb.magic = caching::superblock_detail::SUPERBLOCK_MAGIC; sb.version = 0; // FIXME: fix - // strncpy(sb.policy_name, policy.c_str(), sizeof(sb.policy_name)); + strncpy((char *) sb.policy_name, policy.c_str(), sizeof(sb.policy_name)); memset(sb.policy_version, 0, sizeof(sb.policy_version)); - sb.policy_hint_size = 0; // FIXME: fix + sb.policy_hint_size = hint_width; + md_->setup_hint_array(hint_width); - memset(sb.metadata_space_map_root, 0, sizeof(sb.metadata_space_map_root)); + memset(sb.metadata_space_map_root, 0, + sizeof(sb.metadata_space_map_root)); sb.mapping_root = 0; sb.hint_root = 0; @@ -60,6 +63,9 @@ namespace { unmapped_value.oblock_ = 0; unmapped_value.flags_ = 0; md_->mappings_->grow(nr_cache_blocks, unmapped_value); + + vector hint_value(hint_width, '\0'); + md_->hints_->grow(nr_cache_blocks, hint_value); } virtual void end_superblock() { diff --git a/caching/superblock.cc b/caching/superblock.cc index e783e16..2766c24 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -22,6 +22,7 @@ superblock_traits::unpack(superblock_disk const &disk, superblock &core) core.policy_version[i] = to_cpu(disk.policy_version[i]); core.policy_hint_size = to_cpu(disk.policy_hint_size); + cerr << "unpacking: hint width = " << core.policy_hint_size << endl; ::memcpy(core.metadata_space_map_root, disk.metadata_space_map_root, diff --git a/caching/xml_format.cc b/caching/xml_format.cc index f3b9610..50180ff 100644 --- a/caching/xml_format.cc +++ b/caching/xml_format.cc @@ -33,12 +33,14 @@ namespace { void begin_superblock(std::string const &uuid, block_address block_size, block_address nr_cache_blocks, - std::string const &policy) { + std::string const &policy, + size_t hint_width) { indent(); out_ << "" << endl; + << " policy=\"" << policy << "\"" + << " hint_width=\"" << hint_width << "\">" << endl; inc(); } @@ -164,7 +166,8 @@ namespace { e->begin_superblock(get_attr(attr, "uuid"), get_attr(attr, "block_size"), get_attr(attr, "nr_cache_blocks"), - get_attr(attr, "policy")); + get_attr(attr, "policy"), + get_attr(attr, "hint_width")); } bool to_bool(string const &str) { From 9c65f2f30bed470319af16055a957fafddbf4b70 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 26 Sep 2013 12:05:53 +0100 Subject: [PATCH 32/80] [cache_dump.feature] correct -h expectations --- features/cache_dump.feature | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/features/cache_dump.feature b/features/cache_dump.feature index 6ca41f6..4011c20 100644 --- a/features/cache_dump.feature +++ b/features/cache_dump.feature @@ -7,25 +7,26 @@ Feature: cache_dump When I run cache_dump with --version Then it should pass with version + @announce Scenario: print help (-h) When I run cache_dump with -h Then it should pass with: - """ Usage: cache_dump [options] {device|file} Options: {-h|--help} + {-o } {-V|--version} """ Scenario: print help (--help) When I run cache_dump with -h Then it should pass with: - """ Usage: cache_dump [options] {device|file} Options: {-h|--help} + {-o } {-V|--version} """ @@ -41,7 +42,6 @@ Feature: cache_dump No input file provided. """ - @announce Scenario: dump/restore is a noop Given valid cache metadata When I cache_dump From 326134a7aa0db54cf8678e41f64c8f91e4fbdfe8 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 2 Oct 2013 10:44:57 +0100 Subject: [PATCH 33/80] [array] add visit method for values and damage --- persistent-data/data-structures/array.h | 85 ++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/persistent-data/data-structures/array.h b/persistent-data/data-structures/array.h index 7c4c4fe..c5ed021 100644 --- a/persistent-data/data-structures/array.h +++ b/persistent-data/data-structures/array.h @@ -21,12 +21,11 @@ #include "persistent-data/math_utils.h" #include "persistent-data/data-structures/btree.h" +#include "persistent-data/data-structures/btree_damage_visitor.h" #include "persistent-data/data-structures/array_block.h" //---------------------------------------------------------------- -// FIXME: we need an array checker - namespace persistent_data { namespace array_detail { uint32_t const ARRAY_CSUM_XOR = 595846735; @@ -65,6 +64,25 @@ namespace persistent_data { unsigned nr_entries_in_last_block; unsigned nr_total_blocks; }; + + struct damage { + typedef boost::shared_ptr ptr; + + damage(run lost_keys, + std::string const &desc) + : lost_keys_(lost_keys), + desc_(desc) { + } + + run lost_keys_; + std::string desc_; + }; + + inline std::ostream &operator <<(std::ostream &out, damage const &d) { + out << "array damage[lost_keys = " << d.lost_keys_ + << ", \"" << d.desc_ << "\"]"; + return out; + } } class array_base { @@ -125,6 +143,60 @@ namespace persistent_data { } }; + template + struct block_value_visitor { + block_value_visitor(array const &a, ValueVisitor &vv) + : a_(a), + vv_(vv) { + } + + void visit(btree_path const &p, + typename block_traits::value_type const &v) { + a_.visit_value(vv_, p, v); + } + + private: + array const &a_; + ValueVisitor &vv_; + }; + + template + void visit_value(ValueVisitor &vv, + btree_path const &p, + typename block_traits::value_type const &v) const { + rblock rb(tm_->read_lock(v, validator_), rc_); + + for (uint32_t i = 0; i < rb.nr_entries(); i++) + vv.visit(p[0] * rb.max_entries() + i, rb.get(i)); + } + + template + struct block_damage_visitor { + block_damage_visitor(DamageVisitor &dv, unsigned entries_per_block) + : dv_(dv), + entries_per_block_(entries_per_block) { + } + + void visit(btree_path const &path, btree_detail::damage const &d) { + dv_.visit(array_detail::damage(convert_run(d.lost_keys_), d.desc_)); + } + + private: + run::maybe convert_maybe(run::maybe const &v) const { + if (v) + return run::maybe(*v * entries_per_block_); + + return run::maybe(); + } + + run convert_run(run const &v) const { + return run(convert_maybe(v.begin_), convert_maybe(v.end_)); + } + + DamageVisitor &dv_; + unsigned entries_per_block_; + }; + typedef typename persistent_data::transaction_manager::ptr tm_ptr; typedef block_manager<>::write_ref write_ref; @@ -191,6 +263,15 @@ namespace persistent_data { b.set(index % entries_per_block_, value); } + template + void visit_values(ValueVisitor &value_visitor, + DamageVisitor &damage_visitor) const { + block_counter counter; + block_value_visitor bvisitor(*this, value_visitor); + block_damage_visitor dvisitor(damage_visitor, entries_per_block_); + btree_visit_values(block_tree_, counter, bvisitor, dvisitor); + } + private: struct resizer { From b1f58075139f037b39e3fde526578ad74473ba04 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 2 Oct 2013 10:50:13 +0100 Subject: [PATCH 34/80] [caching/mapping_array] add damage visitor --- caching/mapping_array.cc | 46 ++++++++++++++++++++++++++++++++++++++++ caching/mapping_array.h | 29 ++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/caching/mapping_array.cc b/caching/mapping_array.cc index c26460e..93e2d09 100644 --- a/caching/mapping_array.cc +++ b/caching/mapping_array.cc @@ -26,3 +26,49 @@ mapping_traits::pack(value_type const &value, disk_type &disk) } //---------------------------------------------------------------- + +missing_mappings::missing_mappings(run const &keys, + std::string const &desc) + : keys_(keys), + desc_(desc) +{ + +} + +void +missing_mappings::visit(damage_visitor &v) const +{ + v.visit(*this); +} + +namespace { + struct no_op_visitor { + virtual void visit(uint32_t index, + mapping_traits::value_type const &v) { + } + }; + + class ll_damage_visitor { + public: + ll_damage_visitor(damage_visitor &v) + : v_(v) { + } + + virtual void visit(array_detail::damage const &d) { + v_.visit(missing_mappings(d.lost_keys_, d.desc_)); + } + + private: + damage_visitor &v_; + }; +} + +void +caching::check_mapping_array(mapping_array const &array, damage_visitor &visitor) +{ + no_op_visitor vv; + ll_damage_visitor ll(visitor); + array.visit_values(vv, ll); +} + +//---------------------------------------------------------------- diff --git a/caching/mapping_array.h b/caching/mapping_array.h index 030ec4f..9d22b26 100644 --- a/caching/mapping_array.h +++ b/caching/mapping_array.h @@ -26,10 +26,37 @@ namespace caching { static void pack(value_type const &value, disk_type &disk); }; - // FIXME: damage visitor stuff + class damage_visitor; + + struct damage { + virtual ~damage() {} + virtual void visit(damage_visitor &v) const = 0; + }; + + struct missing_mappings : public damage { + missing_mappings(run const &keys, std::string const &desc); + virtual void visit(damage_visitor &v) const; + + run keys_; + std::string desc_; + }; + + class damage_visitor { + public: + virtual ~damage_visitor() {} + + virtual void visit(mapping_array_detail::damage const &d) { + d.visit(*this); + } + + virtual void visit(missing_mappings const &d) = 0; + }; } typedef persistent_data::array mapping_array; + + void check_mapping_array(mapping_array const &array, + mapping_array_detail::damage_visitor &visitor); } //---------------------------------------------------------------- From e7223037d4be680cecf7f1d3569b0a0b9ec9521e Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 2 Oct 2013 10:50:42 +0100 Subject: [PATCH 35/80] [cache_check] wip --- Makefile.in | 2 + caching/cache_check.cc | 173 ++++++++++++++++++++++++++++++++++------- 2 files changed, 148 insertions(+), 27 deletions(-) diff --git a/Makefile.in b/Makefile.in index ddabacb..1781abf 100644 --- a/Makefile.in +++ b/Makefile.in @@ -226,9 +226,11 @@ thin_metadata_size: thin-provisioning/thin_metadata_size.o # Cache tools CACHE_CHECK_SOURCE=\ + base/error_state.cc \ persistent-data/checksum.cc \ persistent-data/endian_utils.cc \ persistent-data/error_set.cc \ + persistent-data/file_utils.cc \ persistent-data/hex_dump.cc \ persistent-data/lock_tracker.cc \ persistent-data/data-structures/btree.cc \ diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 3de52dc..5cb2c7d 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -14,12 +14,108 @@ #include "base/error_state.h" #include "base/nested_output.h" +#include "caching/metadata.h" +#include "persistent-data/block.h" +#include "persistent-data/file_utils.h" +#include "persistent-data/space-maps/core.h" +using namespace boost; +using namespace caching; +using namespace persistent_data; using namespace std; //---------------------------------------------------------------- namespace { + + class reporter_base { + public: + reporter_base(nested_output &o) + : out_(o), + err_(NO_ERROR) { + } + + virtual ~reporter_base() {} + + nested_output &out() { + return out_; + } + + nested_output::nest push() { + return out_.push(); + } + + base::error_state get_error() const { + return err_; + } + + void mplus_error(error_state err) { + err_ = combine_errors(err_, err); + } + + private: + nested_output &out_; + error_state err_; + }; + + class superblock_reporter : public superblock_detail::damage_visitor, reporter_base { + public: + superblock_reporter(nested_output &o) + : reporter_base(o) { + } + + virtual void visit(superblock_detail::superblock_corruption const &d) { + out() << "superblock is corrupt" << end_message(); + { + nested_output::nest _ = push(); + out() << d.desc_ << end_message(); + } + + mplus_error(FATAL); + } + + using reporter_base::get_error; + }; + + class mapping_reporter : public reporter_base { + public: + mapping_reporter(nested_output &o) + : reporter_base(o) { + } + }; + + class hint_reporter : public reporter_base { + public: + hint_reporter(nested_output &o) + : reporter_base(o) { + } + }; + + //-------------------------------- + + transaction_manager::ptr open_tm(block_manager<>::ptr bm) { + space_map::ptr sm(new core_map(bm->get_nr_blocks())); + sm->inc(superblock_detail::SUPERBLOCK_LOCATION); + transaction_manager::ptr tm(new transaction_manager(bm, sm)); + return tm; + } + + //-------------------------------- + + struct flags { + flags() + : check_mappings_(false), + check_hints_(false), + ignore_non_fatal_errors_(false), + quiet_(false) { + } + + bool check_mappings_; + bool check_hints_; + bool ignore_non_fatal_errors_; + bool quiet_; + }; + struct stat guarded_stat(string const &path) { struct stat info; @@ -36,21 +132,49 @@ namespace { return info; } - int open_file(string const &path, int mode) { - int fd = open(path.c_str(), mode); - if (fd < 0) { - ostringstream msg; - char buffer[128], *ptr; + error_state metadata_check(block_manager<>::ptr bm, flags const &fs) { + error_state err = NO_ERROR; - ptr = strerror_r(errno, buffer, sizeof(buffer)); - msg << path << ": " << ptr; - throw runtime_error(msg.str()); + nested_output out(cerr, 2); + if (fs.quiet_) + out.disable(); + + superblock_reporter sb_rep(out); + mapping_reporter mapping_rep(out); + hint_reporter hint_rep(out); + + out << "examining superblock" << end_message(); + { + nested_output::nest _ = out.push(); + check_superblock(bm, sb_rep); } - return fd; + if (sb_rep.get_error() == FATAL) + return FATAL; + + superblock_detail::superblock sb = read_superblock(bm); + transaction_manager::ptr tm = open_tm(bm); + + if (fs.check_mappings_) { + out << "examining mapping array" << end_message(); + { + nested_output::nest _ = out.push(); + mapping_array ma(tm, mapping_array::ref_counter(), sb.mapping_root, sb.cache_blocks); + // check_mapping_array(ma, mapping_rep); + } + } + + if (fs.check_hints_) { + out << "examining hint array" << end_message(); + { + nested_output::nest _ = out.push(); + } + } + + return err; } - int check(string const &path, bool quiet) { + int check(string const &path, flags const &fs) { struct stat info = guarded_stat(path); if (!S_ISREG(info.st_mode) && !S_ISBLK(info.st_mode)) { @@ -59,31 +183,26 @@ namespace { throw runtime_error(msg.str()); } - int fd = open_file(path, O_RDONLY); - - ostringstream msg; - msg << path << ": " << "No superblock found"; - throw runtime_error(msg.str()); - return 0; - -#if 0 try { - metadata::ptr md(new metadata(path, metadata::OPEN)); + block_manager<>::ptr bm = open_bm(path, block_io<>::READ_ONLY); + //metadata::ptr md(new metadata(bm, metadata::OPEN)); - optional maybe_errors = metadata_check(md); + error_state err = metadata_check(bm, fs); +#if 0 if (maybe_errors) { - if (!quiet) + if (!fs.quiet_) cerr << error_selector(*maybe_errors, 3); return 1; } +#endif + } catch (std::exception &e) { - if (!quiet) + if (!fs.quiet_) cerr << e.what() << endl; return 1; } return 0; -#endif } void usage(ostream &out, string const &cmd) { @@ -102,7 +221,7 @@ namespace { int main(int argc, char **argv) { int c; - bool quiet = false; + flags fs; const char shortopts[] = "qhV"; const struct option longopts[] = { { "quiet", no_argument, NULL, 'q'}, @@ -118,7 +237,7 @@ int main(int argc, char **argv) return 0; case 'q': - quiet = true; + fs.quiet_ = true; break; case 'V': @@ -138,9 +257,9 @@ int main(int argc, char **argv) } try { - check(argv[optind], quiet); + check(argv[optind], fs); - } catch (exception const &e) { + } catch (std::exception const &e) { cerr << e.what() << endl; return 1; } From 3bc798c1f8b60e6dbb995e81baba84f554eab164 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 2 Oct 2013 11:07:33 +0100 Subject: [PATCH 36/80] [array] wire up the correct validator --- persistent-data/data-structures/array.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/persistent-data/data-structures/array.h b/persistent-data/data-structures/array.h index c5ed021..d2ae2ab 100644 --- a/persistent-data/data-structures/array.h +++ b/persistent-data/data-structures/array.h @@ -216,7 +216,7 @@ namespace persistent_data { block_rc_(tm->get_sm(), *this), block_tree_(tm, block_rc_), rc_(rc), - validator_(new block_manager<>::noop_validator()) { + validator_(new array_detail::array_block_validator) { } array(tm_ptr tm, ref_counter rc, @@ -228,7 +228,7 @@ namespace persistent_data { block_rc_(tm->get_sm(), *this), block_tree_(tm, root, block_rc_), rc_(rc), - validator_(new block_manager<>::noop_validator()) { + validator_(new array_detail::array_block_validator) { } unsigned get_nr_entries() const { From ce8e19fa75999d688b237db0eb058297be919daf Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 2 Oct 2013 11:08:09 +0100 Subject: [PATCH 37/80] remove old fixme --- persistent-data/data-structures/array.h | 1 - 1 file changed, 1 deletion(-) diff --git a/persistent-data/data-structures/array.h b/persistent-data/data-structures/array.h index d2ae2ab..374a1bb 100644 --- a/persistent-data/data-structures/array.h +++ b/persistent-data/data-structures/array.h @@ -30,7 +30,6 @@ namespace persistent_data { namespace array_detail { uint32_t const ARRAY_CSUM_XOR = 595846735; - // FIXME: this isn't used! struct array_block_validator : public block_manager<>::validator { virtual void check(buffer<> const &b, block_address location) const { array_block_disk const *data = reinterpret_cast(&b); From fd7c539a581d6d839b91e311ba2a772505d855d7 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 4 Oct 2013 16:08:31 +0100 Subject: [PATCH 38/80] [cache_check] Errors weren't being propagated up to the exit code --- caching/cache_check.cc | 17 ++++------------- features/cache_check.feature | 13 +++---------- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 5cb2c7d..f1667c3 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -175,6 +175,7 @@ namespace { } int check(string const &path, flags const &fs) { + error_state err; struct stat info = guarded_stat(path); if (!S_ISREG(info.st_mode) && !S_ISBLK(info.st_mode)) { @@ -185,9 +186,7 @@ namespace { try { block_manager<>::ptr bm = open_bm(path, block_io<>::READ_ONLY); - //metadata::ptr md(new metadata(bm, metadata::OPEN)); - - error_state err = metadata_check(bm, fs); + err = metadata_check(bm, fs); #if 0 if (maybe_errors) { if (!fs.quiet_) @@ -202,7 +201,7 @@ namespace { return 1; } - return 0; + return err == NO_ERROR ? 0 : 1; } void usage(ostream &out, string const &cmd) { @@ -256,15 +255,7 @@ int main(int argc, char **argv) return 1; } - try { - check(argv[optind], fs); - - } catch (std::exception const &e) { - cerr << e.what() << endl; - return 1; - } - - return 0; + return check(argv[optind], fs); } //---------------------------------------------------------------- diff --git a/features/cache_check.feature b/features/cache_check.feature index 773aaa3..28687af 100644 --- a/features/cache_check.feature +++ b/features/cache_check.feature @@ -56,25 +56,18 @@ Feature: cache_check Scenario: Metadata file exists, but can't be opened Given input without read permissions - When I run `cache_check input` - Then it should fail And the stderr should contain: """ - input: Permission denied + Permission denied """ Scenario: Metadata file full of zeroes Given input file And block 1 is zeroed - When I run `cache_check input` - - And the stderr should contain: - """ - input: No superblock found - """ + Then it should fail Scenario: A valid metadata area passes Given metadata containing: @@ -82,5 +75,5 @@ Feature: cache_check """ When I run cache_check + Then it should pass - Then it should pass \ No newline at end of file From e7816d2f4336e2a1f2d6e4b4d8e6e8af999fd243 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 7 Oct 2013 09:58:17 +0100 Subject: [PATCH 39/80] [block] Open block files with O_DIRECT | O_SYNC. Putting this back in. Dumping metadata snapshots fails without. So there's more caching in the kernel than I expected. --- persistent-data/block.tcc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/persistent-data/block.tcc b/persistent-data/block.tcc index 36e872d..09cd313 100644 --- a/persistent-data/block.tcc +++ b/persistent-data/block.tcc @@ -38,8 +38,8 @@ namespace { int const DEFAULT_MODE = 0666; - // FIXME: these will slow it down until we start doing asyn io. O_DIRECT | O_SYNC; - int const OPEN_FLAGS = 0; + // FIXME: these will slow it down until we start doing async io. + int const OPEN_FLAGS = O_DIRECT | O_SYNC; // FIXME: introduce a new exception for this, or at least lift this // to exception.h From 7cd4728fbfc3cbd3d8389045964cd7361b3525de Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 7 Oct 2013 14:27:08 +0100 Subject: [PATCH 40/80] [build] add LDFLAGS to thin_metadata_size --- Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.in b/Makefile.in index 1781abf..f240904 100644 --- a/Makefile.in +++ b/Makefile.in @@ -220,7 +220,7 @@ thin_rmap: $(THIN_RMAP_OBJECTS) thin-provisioning/thin_rmap.o thin_metadata_size: thin-provisioning/thin_metadata_size.o @echo " [LD] $@" - $(V) $(CC) $(CFLAGS) -o $@ $+ -lm + $(V) $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $+ -lm #---------------------------------------------------------------- # Cache tools From 42fd6b928bfdd70012dfac9036ffb6f6dd10d9dc Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 7 Oct 2013 15:21:45 +0100 Subject: [PATCH 41/80] [cache_check] A whole bunch of superblock checks --- caching/cache_check.cc | 22 ++- caching/metadata.cc | 6 +- caching/metadata.h | 2 +- caching/restore_emitter.cc | 28 +--- caching/superblock.cc | 221 ++++++++++++++++++++++++++----- caching/superblock.h | 161 ++++++++++------------ unit-tests/Makefile.in | 1 + unit-tests/cache_superblock_t.cc | 140 ++++++++++++++++++++ 8 files changed, 416 insertions(+), 165 deletions(-) create mode 100644 unit-tests/cache_superblock_t.cc diff --git a/caching/cache_check.cc b/caching/cache_check.cc index f1667c3..2cbd0e4 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -58,17 +58,27 @@ namespace { error_state err_; }; - class superblock_reporter : public superblock_detail::damage_visitor, reporter_base { + class superblock_reporter : public superblock_damage::damage_visitor, reporter_base { public: superblock_reporter(nested_output &o) : reporter_base(o) { } - virtual void visit(superblock_detail::superblock_corruption const &d) { + virtual void visit(superblock_damage::superblock_corrupt const &d) { out() << "superblock is corrupt" << end_message(); { nested_output::nest _ = push(); - out() << d.desc_ << end_message(); + out() << d.get_desc() << end_message(); + } + + mplus_error(FATAL); + } + + virtual void visit(superblock_damage::superblock_invalid const &d) { + out() << "superblock is invalid" << end_message(); + { + nested_output::nest _ = push(); + out() << d.get_desc() << end_message(); } mplus_error(FATAL); @@ -95,7 +105,7 @@ namespace { transaction_manager::ptr open_tm(block_manager<>::ptr bm) { space_map::ptr sm(new core_map(bm->get_nr_blocks())); - sm->inc(superblock_detail::SUPERBLOCK_LOCATION); + sm->inc(SUPERBLOCK_LOCATION); transaction_manager::ptr tm(new transaction_manager(bm, sm)); return tm; } @@ -146,13 +156,13 @@ namespace { out << "examining superblock" << end_message(); { nested_output::nest _ = out.push(); - check_superblock(bm, sb_rep); + check_superblock(bm, bm->get_nr_blocks(), sb_rep); } if (sb_rep.get_error() == FATAL) return FATAL; - superblock_detail::superblock sb = read_superblock(bm); + superblock sb = read_superblock(bm); transaction_manager::ptr tm = open_tm(bm); if (fs.check_mappings_) { diff --git a/caching/metadata.cc b/caching/metadata.cc index 90a1dd3..8ffe533 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -7,8 +7,6 @@ using namespace caching; //---------------------------------------------------------------- namespace { - using namespace superblock_detail; - unsigned const METADATA_CACHE_SIZE = 1024; // FIXME: duplication @@ -133,9 +131,7 @@ metadata::commit_hints() void metadata::commit_superblock() { - write_ref superblock = tm_->get_bm()->superblock_zero(SUPERBLOCK_LOCATION, superblock_validator()); - superblock_disk *disk = reinterpret_cast(superblock.data().raw()); - superblock_traits::pack(sb_, *disk); + write_superblock(tm_->get_bm(), sb_); } //---------------------------------------------------------------- diff --git a/caching/metadata.h b/caching/metadata.h index e4fe965..418d0e6 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -33,7 +33,7 @@ namespace caching { typedef persistent_data::transaction_manager tm; tm::ptr tm_; - superblock_detail::superblock sb_; + superblock sb_; checked_space_map::ptr metadata_sm_; mapping_array::ptr mappings_; hint_array::ptr hints_; diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index 35654da..cdf1f4c 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -5,7 +5,7 @@ using namespace caching; using namespace mapping_array_detail; using namespace std; -using namespace superblock_detail; +using namespace superblock_damage; //---------------------------------------------------------------- @@ -27,38 +27,14 @@ namespace { size_t hint_width) { superblock &sb = md_->sb_; - - sb.flags = 0; - memset(sb.uuid, 0, sizeof(sb.uuid)); - sb.magic = caching::superblock_detail::SUPERBLOCK_MAGIC; - sb.version = 0; // FIXME: fix strncpy((char *) sb.policy_name, policy.c_str(), sizeof(sb.policy_name)); - memset(sb.policy_version, 0, sizeof(sb.policy_version)); + memset(sb.policy_version, 0, sizeof(sb.policy_version)); // FIXME: should come from xml sb.policy_hint_size = hint_width; md_->setup_hint_array(hint_width); - memset(sb.metadata_space_map_root, 0, - sizeof(sb.metadata_space_map_root)); - sb.mapping_root = 0; - sb.hint_root = 0; - - sb.discard_root = 0; - sb.discard_block_size = 0; - sb.discard_nr_blocks = 0; - sb.data_block_size = block_size; - sb.metadata_block_size = 0; sb.cache_blocks = nr_cache_blocks; - sb.compat_flags = 0; - sb.compat_ro_flags = 0; - sb.incompat_flags = 0; - - sb.read_hits = 0; - sb.read_misses = 0; - sb.write_hits = 0; - sb.write_misses = 0; - struct mapping unmapped_value; unmapped_value.oblock_ = 0; unmapped_value.flags_ = 0; diff --git a/caching/superblock.cc b/caching/superblock.cc index 2766c24..b3cfaee 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -1,7 +1,92 @@ #include "caching/superblock.h" using namespace caching; -using namespace superblock_detail; +using namespace superblock_damage; + +//---------------------------------------------------------------- + +namespace { + using namespace base; + + struct superblock_disk { + le32 csum; + le32 flags; + le64 blocknr; + + __u8 uuid[16]; + le64 magic; + le32 version; + + __u8 policy_name[CACHE_POLICY_NAME_SIZE]; + le32 policy_version[CACHE_POLICY_VERSION_SIZE]; + le32 policy_hint_size; + + __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; + + le64 mapping_root; + le64 hint_root; + + le64 discard_root; + le64 discard_block_size; + le64 discard_nr_blocks; + + le32 data_block_size; /* in 512-byte sectors */ + le32 metadata_block_size; /* in 512-byte sectors */ + le32 cache_blocks; + + le32 compat_flags; + le32 compat_ro_flags; + le32 incompat_flags; + + le32 read_hits; + le32 read_misses; + le32 write_hits; + le32 write_misses; + } __attribute__ ((packed)); + + struct superblock_traits { + typedef superblock_disk disk_type; + typedef superblock value_type; + + static void unpack(superblock_disk const &disk, superblock &value); + static void pack(superblock const &value, superblock_disk &disk); + }; + + uint32_t const SUPERBLOCK_MAGIC = 06142003; + uint32_t const VERSION_BEGIN = 1; + uint32_t const VERSION_END = 2; +} + +//---------------------------------------------------------------- + +superblock::superblock() + : csum(0), + flags(0), + blocknr(SUPERBLOCK_LOCATION), + magic(SUPERBLOCK_MAGIC), + version(VERSION_BEGIN), + policy_hint_size(0), + mapping_root(0), + hint_root(0), + discard_root(0), + discard_block_size(0), + discard_nr_blocks(0), + data_block_size(0), + metadata_block_size(8), + cache_blocks(0), + compat_flags(0), + compat_ro_flags(0), + incompat_flags(0), + read_hits(0), + read_misses(0), + write_hits(0), + write_misses(0) +{ + ::memset(uuid, 0, sizeof(uuid)); + ::memset(policy_name, 0, sizeof(policy_name)); + ::memset(policy_version, 0, sizeof(policy_version)); + ::memset(metadata_space_map_root, 0, sizeof(metadata_space_map_root)); +} //---------------------------------------------------------------- @@ -94,22 +179,33 @@ superblock_traits::pack(superblock const &core, superblock_disk &disk) //-------------------------------- -superblock_corruption::superblock_corruption(std::string const &desc) - : desc_(desc) +superblock_corrupt::superblock_corrupt(std::string const &desc) + : damage(desc) { } void -superblock_corruption::visit(damage_visitor &v) const +superblock_corrupt::visit(damage_visitor &v) const +{ + v.visit(*this); +} + +superblock_invalid::superblock_invalid(std::string const &desc) + : damage(desc) +{ +} + +void +superblock_invalid::visit(damage_visitor &v) const { v.visit(*this); } //-------------------------------- -namespace { +// anonymous namespace doesn't work for some reason +namespace validator { using namespace persistent_data; - using namespace superblock_detail; uint32_t const VERSION = 1; unsigned const SECTOR_TO_BLOCK_SHIFT = 3; @@ -131,65 +227,122 @@ namespace { sbd->csum = to_disk(sum.get_sum()); } }; -} -persistent_data::block_manager<>::validator::ptr -caching::superblock_validator() -{ - return block_manager<>::validator::ptr(new sb_validator); + block_manager<>::validator::ptr mk_v() { + return block_manager<>::validator::ptr(new sb_validator); + } } //-------------------------------- -superblock_detail::superblock +superblock caching::read_superblock(block_manager<>::ptr bm, block_address location) { - using namespace superblock_detail; - superblock sb; - block_manager<>::read_ref r = bm->read_lock(location, superblock_validator()); + block_manager<>::read_ref r = bm->read_lock(location, validator::mk_v()); superblock_disk const *sbd = reinterpret_cast(&r.data()); superblock_traits::unpack(*sbd, sb); return sb; } -superblock_detail::superblock -caching::read_superblock(persistent_data::block_manager<>::ptr bm) +void +caching::write_superblock(block_manager<>::ptr bm, superblock const &sb, block_address location) { - return read_superblock(bm, SUPERBLOCK_LOCATION); + block_manager<>::write_ref w = bm->superblock_zero(location, validator::mk_v()); + superblock_traits::pack(sb, *reinterpret_cast(w.data().raw())); } void -caching::write_superblock(block_manager<>::ptr bm, - block_address location, - superblock_detail::superblock const &sb) +caching::check_superblock(superblock const &sb, + block_address nr_metadata_blocks, + damage_visitor &visitor) { - using namespace superblock_detail; + if (sb.flags != 0) { + ostringstream msg; + msg << "invalid flags: " << hex << sb.flags; + visitor.visit(superblock_invalid(msg.str())); + } - block_manager<>::write_ref w = bm->write_lock(location, superblock_validator()); - superblock_traits::pack(sb, *reinterpret_cast(&w.data())); -} + if (sb.blocknr >= nr_metadata_blocks) { + ostringstream msg; + msg << "blocknr out of bounds: " << sb.blocknr << " >= " << nr_metadata_blocks; + visitor.visit(superblock_invalid(msg.str())); + } -void -caching::write_superblock(block_manager<>::ptr bm, - superblock_detail::superblock const &sb) -{ - write_superblock(bm, SUPERBLOCK_LOCATION, sb); + if (sb.magic != SUPERBLOCK_MAGIC) { + ostringstream msg; + msg << "magic in incorrect: " << sb.magic; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.version >= VERSION_END) { + ostringstream msg; + msg << "version incorrect: " << sb.version; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.version < VERSION_BEGIN) { + ostringstream msg; + msg << "version incorrect: " << sb.version; + visitor.visit(superblock_invalid(msg.str())); + } + + if (::strnlen((char const *) sb.policy_name, CACHE_POLICY_NAME_SIZE) == CACHE_POLICY_NAME_SIZE) { + visitor.visit(superblock_invalid("policy name is not null terminated")); + } + + if (sb.policy_hint_size % 4 || sb.policy_hint_size > 128) { + ostringstream msg; + msg << "policy hint size invalid: " << sb.policy_hint_size; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.metadata_block_size != 8) { + ostringstream msg; + msg << "metadata block size incorrect: " << sb.metadata_block_size; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.compat_flags != 0) { + ostringstream msg; + msg << "compat_flags invalid (can only be 0): " << sb.compat_flags; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.compat_ro_flags != 0) { + ostringstream msg; + msg << "compat_ro_flags invalid (can only be 0): " << sb.compat_ro_flags; + visitor.visit(superblock_invalid(msg.str())); + } + + if (sb.incompat_flags != 0) { + ostringstream msg; + msg << "incompat_flags invalid (can only be 0): " << sb.incompat_flags; + visitor.visit(superblock_invalid(msg.str())); + } } void caching::check_superblock(persistent_data::block_manager<>::ptr bm, - superblock_detail::damage_visitor &visitor) + block_address nr_metadata_blocks, + damage_visitor &visitor) { - using namespace superblock_detail; + superblock sb; try { - bm->read_lock(SUPERBLOCK_LOCATION, superblock_validator()); + sb = read_superblock(bm, SUPERBLOCK_LOCATION); } catch (std::exception const &e) { - visitor.visit(superblock_corruption(e.what())); + + // FIXME: what if it fails due to a zero length file? Not + // really a corruption, so much as an io error. Should we + // separate these? + + visitor.visit(superblock_corrupt(e.what())); } + + check_superblock(sb, nr_metadata_blocks, visitor); } //---------------------------------------------------------------- diff --git a/caching/superblock.h b/caching/superblock.h index 92db272..390970d 100644 --- a/caching/superblock.h +++ b/caching/superblock.h @@ -7,112 +7,82 @@ //---------------------------------------------------------------- namespace caching { - namespace superblock_detail { - using namespace base; + typedef unsigned char __u8; - unsigned const SPACE_MAP_ROOT_SIZE = 128; - unsigned const CACHE_POLICY_NAME_SIZE = 16; - unsigned const CACHE_POLICY_VERSION_SIZE = 3; + unsigned const SPACE_MAP_ROOT_SIZE = 128; + unsigned const CACHE_POLICY_NAME_SIZE = 16; + unsigned const CACHE_POLICY_VERSION_SIZE = 3; + block_address const SUPERBLOCK_LOCATION = 0; - typedef unsigned char __u8; + struct superblock { + superblock(); - struct superblock_disk { - le32 csum; - le32 flags; - le64 blocknr; + uint32_t csum; + uint32_t flags; + uint64_t blocknr; - __u8 uuid[16]; - le64 magic; - le32 version; + __u8 uuid[16]; + uint64_t magic; + uint32_t version; - __u8 policy_name[CACHE_POLICY_NAME_SIZE]; - le32 policy_version[CACHE_POLICY_VERSION_SIZE]; - le32 policy_hint_size; + __u8 policy_name[CACHE_POLICY_NAME_SIZE]; + uint32_t policy_version[CACHE_POLICY_VERSION_SIZE]; + uint32_t policy_hint_size; - __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; + __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; - le64 mapping_root; - le64 hint_root; + uint64_t mapping_root; + uint64_t hint_root; - le64 discard_root; - le64 discard_block_size; - le64 discard_nr_blocks; + uint64_t discard_root; + uint64_t discard_block_size; + uint64_t discard_nr_blocks; - le32 data_block_size; /* in 512-byte sectors */ - le32 metadata_block_size; /* in 512-byte sectors */ - le32 cache_blocks; + uint32_t data_block_size; /* in 512-byte sectors */ + uint32_t metadata_block_size; /* in 512-byte sectors */ + uint32_t cache_blocks; - le32 compat_flags; - le32 compat_ro_flags; - le32 incompat_flags; + uint32_t compat_flags; + uint32_t compat_ro_flags; + uint32_t incompat_flags; - le32 read_hits; - le32 read_misses; - le32 write_hits; - le32 write_misses; - } __attribute__ ((packed)); + uint32_t read_hits; + uint32_t read_misses; + uint32_t write_hits; + uint32_t write_misses; + }; - struct superblock { - uint32_t csum; - uint32_t flags; - uint64_t blocknr; + //-------------------------------- - __u8 uuid[16]; - uint64_t magic; - uint32_t version; - - __u8 policy_name[CACHE_POLICY_NAME_SIZE]; - uint32_t policy_version[CACHE_POLICY_VERSION_SIZE]; - uint32_t policy_hint_size; - - __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; - - uint64_t mapping_root; - uint64_t hint_root; - - uint64_t discard_root; - uint64_t discard_block_size; - uint64_t discard_nr_blocks; - - uint32_t data_block_size; /* in 512-byte sectors */ - uint32_t metadata_block_size; /* in 512-byte sectors */ - uint32_t cache_blocks; - - uint32_t compat_flags; - uint32_t compat_ro_flags; - uint32_t incompat_flags; - - uint32_t read_hits; - uint32_t read_misses; - uint32_t write_hits; - uint32_t write_misses; - }; - - struct superblock_traits { - typedef superblock_disk disk_type; - typedef superblock value_type; - - static void unpack(superblock_disk const &disk, superblock &value); - static void pack(superblock const &value, superblock_disk &disk); - }; - - block_address const SUPERBLOCK_LOCATION = 0; - uint32_t const SUPERBLOCK_MAGIC = 06142003; - - //-------------------------------- + namespace superblock_damage { class damage_visitor; - struct damage { + class damage { + public: + damage(std::string const &desc) + : desc_(desc) { + } + virtual ~damage() {} virtual void visit(damage_visitor &v) const = 0; + + std::string const &get_desc() const { + return desc_; + } + + private: + std::string desc_; }; - struct superblock_corruption : public damage { - superblock_corruption(std::string const &desc); + struct superblock_corrupt : public damage { + superblock_corrupt(std::string const &desc); void visit(damage_visitor &v) const; + }; - std::string desc_; + struct superblock_invalid : public damage { + superblock_invalid(std::string const &desc); + void visit(damage_visitor &v) const; }; class damage_visitor { @@ -121,24 +91,29 @@ namespace caching { void visit(damage const &d); - virtual void visit(superblock_corruption const &d) = 0; + virtual void visit(superblock_corrupt const &d) = 0; + virtual void visit(superblock_invalid const &d) = 0; }; } + //-------------------------------- + persistent_data::block_manager<>::validator::ptr superblock_validator(); - superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm); - superblock_detail::superblock read_superblock(persistent_data::block_manager<>::ptr bm, - persistent_data::block_address location); + superblock read_superblock(persistent_data::block_manager<>::ptr bm, + persistent_data::block_address location = SUPERBLOCK_LOCATION); void write_superblock(persistent_data::block_manager<>::ptr bm, - superblock_detail::superblock const &sb); - void write_superblock(persistent_data::block_manager<>::ptr bm, - persistent_data::block_address location, - superblock_detail::superblock const &sb); + superblock const &sb, + persistent_data::block_address location = SUPERBLOCK_LOCATION); + + void check_superblock(superblock const &sb, + persistent_data::block_address nr_metadata_blocks, + superblock_damage::damage_visitor &visitor); void check_superblock(persistent_data::block_manager<>::ptr bm, - superblock_detail::damage_visitor &visitor); + persistent_data::block_address nr_metadata_blocks, + superblock_damage::damage_visitor &visitor); } //---------------------------------------------------------------- diff --git a/unit-tests/Makefile.in b/unit-tests/Makefile.in index 808e20b..ffc1afc 100644 --- a/unit-tests/Makefile.in +++ b/unit-tests/Makefile.in @@ -53,6 +53,7 @@ TEST_SOURCE=\ unit-tests/btree_damage_visitor_t.cc \ unit-tests/buffer_t.cc \ unit-tests/cache_t.cc \ + unit-tests/cache_superblock_t.cc \ unit-tests/damage_tracker_t.cc \ unit-tests/endian_t.cc \ unit-tests/rmap_visitor_t.cc \ diff --git a/unit-tests/cache_superblock_t.cc b/unit-tests/cache_superblock_t.cc new file mode 100644 index 0000000..c5720fa --- /dev/null +++ b/unit-tests/cache_superblock_t.cc @@ -0,0 +1,140 @@ +#include "gmock/gmock.h" +#include "caching/superblock.h" + +using namespace caching; +using namespace superblock_damage; +using namespace testing; + +//---------------------------------------------------------------- + +namespace { + class damage_visitor_mock : public damage_visitor { + public: + MOCK_METHOD1(visit, void(superblock_corrupt const &)); + MOCK_METHOD1(visit, void(superblock_invalid const &)); + }; + + class CacheSuperblockTests : public Test { + public: + CacheSuperblockTests() { + } + + void check() { + check_superblock(sb_, 100, visitor_); + } + + void expect_invalid() { + EXPECT_CALL(visitor_, visit(Matcher(_))).Times(1); + } + + void check_invalid() { + expect_invalid(); + check(); + } + + damage_visitor_mock visitor_; + superblock sb_; + }; + + bool operator ==(superblock_corrupt const &lhs, superblock_corrupt const &rhs) { + return lhs.get_desc() == rhs.get_desc(); + } + + bool operator ==(superblock_invalid const &lhs, superblock_invalid const &rhs) { + return lhs.get_desc() == rhs.get_desc(); + } + + ostream &operator <<(ostream &out, damage const &d) { + out << d.get_desc(); + return out; + } + + ostream &operator <<(ostream &out, superblock_invalid const &d) { + out << "superblock_invalid: " << d.get_desc(); + return out; + } +} + +//---------------------------------------------------------------- + +TEST_F(CacheSuperblockTests, default_constructed_superblock_is_valid) +{ + sb_.flags = 0; + check(); +} + +TEST_F(CacheSuperblockTests, non_zero_flags_are_invalid) +{ + sb_.flags = 1; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, blocknr_is_in_range) +{ + sb_.blocknr = 101; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, magic_is_checked) +{ + sb_.magic = 12345; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, version_gt_1_is_checked) +{ + sb_.version = 2; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, version_lt_1_is_checked) +{ + sb_.version = 0; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, policy_name_must_be_null_terminated) +{ + for (unsigned i = 0; i < CACHE_POLICY_NAME_SIZE; i++) + sb_.policy_name[i] = 'a'; + + check_invalid(); +} + +TEST_F(CacheSuperblockTests, policy_hint_size_checked) +{ + sb_.policy_hint_size = 3; + check_invalid(); + + sb_.policy_hint_size = 129; + check_invalid(); + + sb_.policy_hint_size = 132; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, metadata_block_size_checked) +{ + sb_.metadata_block_size = 16; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, compat_flags_checked) +{ + sb_.compat_flags = 1; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, compat_ro_flags_checked) +{ + sb_.compat_ro_flags = 1; + check_invalid(); +} + +TEST_F(CacheSuperblockTests, incompat_flags_checked) +{ + sb_.incompat_flags = 1; + check_invalid(); +} + +//---------------------------------------------------------------- From 96143f0bed1d0f399b972d736b6dc26c3550c3cc Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 09:11:04 +0100 Subject: [PATCH 42/80] [caching/mapping_array] namespace tweaking --- caching/cache_dump.cc | 4 ++-- caching/mapping_array.cc | 3 ++- caching/mapping_array.h | 38 +++++++++++++++++++------------------- caching/restore_emitter.cc | 3 +-- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index 2702820..dfe7c2c 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -11,8 +11,8 @@ using namespace std; using namespace caching; -using namespace caching::mapping_array_detail; -using namespace caching::superblock_detail; +using namespace caching::mapping_array_damage; +using namespace caching::superblock_damage; //---------------------------------------------------------------- diff --git a/caching/mapping_array.cc b/caching/mapping_array.cc index 93e2d09..1477431 100644 --- a/caching/mapping_array.cc +++ b/caching/mapping_array.cc @@ -1,7 +1,8 @@ #include "caching/mapping_array.h" #include "persistent-data/endian_utils.h" -using namespace caching::mapping_array_detail; +using namespace caching; +using namespace caching::mapping_array_damage; //---------------------------------------------------------------- diff --git a/caching/mapping_array.h b/caching/mapping_array.h index 9d22b26..fde0c28 100644 --- a/caching/mapping_array.h +++ b/caching/mapping_array.h @@ -6,26 +6,26 @@ //---------------------------------------------------------------- namespace caching { - namespace mapping_array_detail { - enum mapping_flags { - M_VALID = 1, - M_DIRTY = 2 - }; + enum mapping_flags { + M_VALID = 1, + M_DIRTY = 2 + }; - struct mapping { - uint64_t oblock_; - uint32_t flags_; - }; + struct mapping { + uint64_t oblock_; + uint32_t flags_; + }; - struct mapping_traits { - typedef base::le64 disk_type; - typedef mapping value_type; - typedef no_op_ref_counter ref_counter; + struct mapping_traits { + typedef base::le64 disk_type; + typedef mapping value_type; + typedef no_op_ref_counter ref_counter; - static void unpack(disk_type const &disk, value_type &value); - static void pack(value_type const &value, disk_type &disk); - }; + static void unpack(disk_type const &disk, value_type &value); + static void pack(value_type const &value, disk_type &disk); + }; + namespace mapping_array_damage { class damage_visitor; struct damage { @@ -45,7 +45,7 @@ namespace caching { public: virtual ~damage_visitor() {} - virtual void visit(mapping_array_detail::damage const &d) { + virtual void visit(mapping_array_damage::damage const &d) { d.visit(*this); } @@ -53,10 +53,10 @@ namespace caching { }; } - typedef persistent_data::array mapping_array; + typedef persistent_data::array mapping_array; void check_mapping_array(mapping_array const &array, - mapping_array_detail::damage_visitor &visitor); + mapping_array_damage::damage_visitor &visitor); } //---------------------------------------------------------------- diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index cdf1f4c..787163c 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -3,7 +3,6 @@ #include "caching/mapping_array.h" using namespace caching; -using namespace mapping_array_detail; using namespace std; using namespace superblock_damage; @@ -59,7 +58,7 @@ namespace { virtual void mapping(pd::block_address cblock, pd::block_address oblock, bool dirty) { - mapping_array_detail::mapping m; + typename caching::mapping m; m.oblock_ = oblock; m.flags_ = M_VALID; From 81a72c8a35eeb11c5fd26c22420c382f2b2043f6 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 09:36:20 +0100 Subject: [PATCH 43/80] [cache_check] mapping damage reporter --- caching/cache_check.cc | 26 +++++++++++++++++++++++++- caching/mapping_array.cc | 24 ++++++++++++++++++------ caching/mapping_array.h | 26 +++++++++++++++++++++++--- 3 files changed, 66 insertions(+), 10 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 2cbd0e4..b660e01 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -87,11 +87,35 @@ namespace { using reporter_base::get_error; }; - class mapping_reporter : public reporter_base { + class mapping_reporter : public mapping_array_damage::damage_visitor, reporter_base { public: mapping_reporter(nested_output &o) : reporter_base(o) { } + + virtual void visit(mapping_array_damage::missing_mappings const &d) { + out() << "missing mappings:" << end_message(); + { + nested_output::nest _ = push(); + out() << d.get_desc() << end_message(); + } + + mplus_error(FATAL); + } + + virtual void visit(mapping_array_damage::invalid_mapping const &d) { + out() << "invalid mapping:" << end_message(); + { + nested_output::nest _ = push(); + out() << d.get_desc() + << " [cblock = " << d.cblock_ + << ", oblock = " << d.m_.oblock_ + << ", flags = " << d.m_.flags_ + << "]" << end_message(); + } + + mplus_error(FATAL); + } }; class hint_reporter : public reporter_base { diff --git a/caching/mapping_array.cc b/caching/mapping_array.cc index 1477431..d2a7e48 100644 --- a/caching/mapping_array.cc +++ b/caching/mapping_array.cc @@ -28,12 +28,10 @@ mapping_traits::pack(value_type const &value, disk_type &disk) //---------------------------------------------------------------- -missing_mappings::missing_mappings(run const &keys, - std::string const &desc) - : keys_(keys), - desc_(desc) +missing_mappings::missing_mappings(std::string const &desc, run const &keys) + : damage(desc), + keys_(keys) { - } void @@ -42,6 +40,20 @@ missing_mappings::visit(damage_visitor &v) const v.visit(*this); } +invalid_mapping::invalid_mapping(std::string const &desc, + block_address cblock, mapping const &m) + : damage(desc), + cblock_(cblock), + m_(m) +{ +} + +void +invalid_mapping::visit(damage_visitor &v) const +{ + v.visit(*this); +} + namespace { struct no_op_visitor { virtual void visit(uint32_t index, @@ -56,7 +68,7 @@ namespace { } virtual void visit(array_detail::damage const &d) { - v_.visit(missing_mappings(d.lost_keys_, d.desc_)); + v_.visit(missing_mappings(d.desc_, d.lost_keys_)); } private: diff --git a/caching/mapping_array.h b/caching/mapping_array.h index fde0c28..29f9390 100644 --- a/caching/mapping_array.h +++ b/caching/mapping_array.h @@ -28,17 +28,36 @@ namespace caching { namespace mapping_array_damage { class damage_visitor; - struct damage { + class damage { + public: + damage(std::string const &desc) + : desc_(desc) { + } + virtual ~damage() {} virtual void visit(damage_visitor &v) const = 0; + + std::string get_desc() const { + return desc_; + } + + private: + std::string desc_; }; struct missing_mappings : public damage { - missing_mappings(run const &keys, std::string const &desc); + missing_mappings(std::string const &desc, run const &keys); virtual void visit(damage_visitor &v) const; run keys_; - std::string desc_; + }; + + struct invalid_mapping : public damage { + invalid_mapping(std::string const &desc, block_address cblock, mapping const &m); + virtual void visit(damage_visitor &v) const; + + block_address cblock_; + mapping m_; }; class damage_visitor { @@ -50,6 +69,7 @@ namespace caching { } virtual void visit(missing_mappings const &d) = 0; + virtual void visit(invalid_mapping const &d) = 0; }; } From 19b5c6193f6153f08dfcd92a9d2b36b7c14c570a Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 09:59:16 +0100 Subject: [PATCH 44/80] [cache_check] detect duplicate mappings or unknown mapping flags --- caching/mapping_array.cc | 49 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/caching/mapping_array.cc b/caching/mapping_array.cc index d2a7e48..8a7c792 100644 --- a/caching/mapping_array.cc +++ b/caching/mapping_array.cc @@ -1,8 +1,11 @@ #include "caching/mapping_array.h" #include "persistent-data/endian_utils.h" +#include + using namespace caching; using namespace caching::mapping_array_damage; +using namespace std; //---------------------------------------------------------------- @@ -55,10 +58,46 @@ invalid_mapping::visit(damage_visitor &v) const } namespace { - struct no_op_visitor { - virtual void visit(uint32_t index, - mapping_traits::value_type const &v) { + class mapping_visitor { + public: + mapping_visitor(damage_visitor &visitor) + : visitor_(visitor) { } + + virtual void visit(uint32_t index, mapping const &m) { + block_address cblock = index; + + if (!valid_mapping(m)) + return; + + if (seen_oblock(m)) + visitor_.visit(invalid_mapping("origin block already mapped", cblock, m)); + else + record_oblock(m); + + if (unknown_flags(m)) + visitor_.visit(invalid_mapping("unknown flags in mapping", cblock, m)); + } + + private: + static bool valid_mapping(mapping const &m) { + return !!(m.flags_ & M_VALID); + } + + bool seen_oblock(mapping const &m) const { + return seen_oblocks_.find(m.oblock_) != seen_oblocks_.end(); + } + + void record_oblock(mapping const &m) { + seen_oblocks_.insert(m.oblock_); + } + + static bool unknown_flags(mapping const &m) { + return (m.flags_ & ~(M_VALID | M_DIRTY)); + } + + damage_visitor &visitor_; + set seen_oblocks_; }; class ll_damage_visitor { @@ -79,9 +118,9 @@ namespace { void caching::check_mapping_array(mapping_array const &array, damage_visitor &visitor) { - no_op_visitor vv; + mapping_visitor mv(visitor); ll_damage_visitor ll(visitor); - array.visit_values(vv, ll); + array.visit_values(mv, ll); } //---------------------------------------------------------------- From 7da033d5c18f741f349cca2e7cbebd7973a77746 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 10:02:25 +0100 Subject: [PATCH 45/80] [hint_array] move hint_traits to .cc file --- caching/hint_array.cc | 20 +++++++++++++++++++- caching/hint_array.h | 18 ------------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/caching/hint_array.cc b/caching/hint_array.cc index 5e2375a..d67f11d 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -7,7 +7,25 @@ using namespace persistent_data; //---------------------------------------------------------------- namespace { - using namespace caching::hint_array_detail; + template + struct hint_traits { + typedef unsigned char byte; + typedef byte disk_type[WIDTH]; + typedef vector value_type; + typedef no_op_ref_counter ref_counter; + + // FIXME: slow copying for now + static void unpack(disk_type const &disk, value_type &value) { + for (unsigned byte = 0; byte < WIDTH; byte++) + value.at(byte) = disk[byte]; + } + + static void pack(value_type const &value, disk_type &disk) { + for (unsigned byte = 0; byte < WIDTH; byte++) + disk[byte] = value.at(byte); + } + }; + // We've got into a bit of a mess here. Templates are compile // time, and we don't know the hint width until run time. We're diff --git a/caching/hint_array.h b/caching/hint_array.h index 6bb9884..4dfc8d4 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -9,24 +9,6 @@ namespace caching { namespace hint_array_detail { - template - struct hint_traits { - typedef unsigned char byte; - typedef byte disk_type[WIDTH]; - typedef vector value_type; - typedef no_op_ref_counter ref_counter; - - // FIXME: slow copying for now - static void unpack(disk_type const &disk, value_type &value) { - for (unsigned byte = 0; byte < WIDTH; byte++) - value.at(byte) = disk[byte]; - } - - static void pack(value_type const &value, disk_type &disk) { - for (unsigned byte = 0; byte < WIDTH; byte++) - disk[byte] = value.at(byte); - } - }; // FIXME: data visitor stuff } From ddf5765f9cfaf1797e7136d687031aace734578e Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 10:43:20 +0100 Subject: [PATCH 46/80] [caching] damage visitor and checker for hint array --- caching/hint_array.cc | 89 +++++++++++++++++++++++++++++++++++++------ caching/hint_array.h | 40 ++++++++++++++++++- 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/caching/hint_array.cc b/caching/hint_array.cc index d67f11d..0b95e48 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -2,6 +2,7 @@ using namespace boost; using namespace caching; +using namespace caching::hint_array_damage; using namespace persistent_data; //---------------------------------------------------------------- @@ -64,6 +65,18 @@ namespace { //-------------------------------- + template + shared_ptr + downcast_array(shared_ptr base) { + shared_ptr a = dynamic_pointer_cast(base); + if (!a) + throw runtime_error("internal error: couldn't cast hint array"); + + return a; + } + + //-------------------------------- + template shared_ptr mk_array(transaction_manager::ptr tm, block_address root, unsigned nr_entries) { typedef hint_traits traits; @@ -94,10 +107,7 @@ namespace { typedef hint_traits traits; typedef array ha; - shared_ptr a = dynamic_pointer_cast(base); - if (!a) - throw runtime_error("internal error: couldn't cast hint array"); - + shared_ptr a = downcast_array(base); data = a->get(index); } @@ -116,10 +126,7 @@ namespace { typedef hint_traits traits; typedef array ha; - shared_ptr a = dynamic_pointer_cast(base); - if (!a) - throw runtime_error("internal error: couldn't cast hint array"); - + shared_ptr a = downcast_array(base); a->set(index, data); } @@ -139,9 +146,7 @@ namespace { typedef hint_traits traits; typedef array ha; - shared_ptr a = dynamic_pointer_cast(base); - if (!a) - throw runtime_error("internal error: couldn't cast hint array"); + shared_ptr a = downcast_array(base); a->grow(new_nr_entries, value); } @@ -154,6 +159,62 @@ namespace { #undef xx } } + + //-------------------------------- + + template + struct no_op_visitor { + virtual void visit(uint32_t index, ValueType const &v) { + } + }; + + class ll_damage_visitor { + public: + ll_damage_visitor(damage_visitor &v) + : v_(v) { + } + + virtual void visit(array_detail::damage const &d) { + v_.visit(missing_hints(d.desc_, d.lost_keys_)); + } + + private: + damage_visitor &v_; + }; + + template + void check_hints(shared_ptr base, damage_visitor &visitor) { + typedef hint_traits traits; + typedef array ha; + + shared_ptr a = downcast_array(base); + no_op_visitor nv; + ll_damage_visitor ll(visitor); + a->visit_values(nv, ll); + } + + void check_hints_(uint32_t width, shared_ptr base, + damage_visitor &visitor) { + switch (width) { +#define xx(n) case n: check_hints(base, visitor); break + all_widths +#undef xx + } + } +} + +//---------------------------------------------------------------- + +missing_hints::missing_hints(std::string const desc, run const &keys) + : damage(desc), + keys_(keys) +{ +} + +void +missing_hints::visit(damage_visitor &v) const +{ + v.visit(*this); } //---------------------------------------------------------------- @@ -195,4 +256,10 @@ hint_array::grow(unsigned new_nr_entries, vector const &value) grow_(width_, impl_, new_nr_entries, value); } +void +hint_array::check_hint_array(hint_array_damage::damage_visitor &visitor) +{ + check_hints_(width_, impl_, visitor); +} + //---------------------------------------------------------------- diff --git a/caching/hint_array.h b/caching/hint_array.h index 4dfc8d4..e8483ac 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -8,9 +8,43 @@ //---------------------------------------------------------------- namespace caching { - namespace hint_array_detail { + namespace hint_array_damage { + class damage_visitor; - // FIXME: data visitor stuff + class damage { + public: + damage(std::string const &desc) + : desc_(desc) { + } + + virtual ~damage() {} + virtual void visit(damage_visitor &v) const = 0; + + std::string get_desc() const { + return desc_; + } + + private: + std::string desc_; + }; + + struct missing_hints : public damage { + missing_hints(std::string const desc, run const &keys); + virtual void visit(damage_visitor &v) const; + + run keys_; + }; + + class damage_visitor { + public: + virtual ~damage_visitor() {} + + virtual void visit(damage const &d) { + d.visit(*this); + } + + virtual void visit(missing_hints const &d) = 0; + }; } class hint_array { @@ -23,6 +57,7 @@ namespace caching { unsigned get_nr_entries() const; + void grow(unsigned new_nr_entries, void const *v); block_address get_root() const; @@ -30,6 +65,7 @@ namespace caching { void set_hint(unsigned index, vector const &data); void grow(unsigned new_nr_entries, vector const &value); + void check_hint_array(hint_array_damage::damage_visitor &visitor); private: unsigned width_; From be22981a7d9dd87e24fe6faa8eec5430c0dab54d Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 11:00:05 +0100 Subject: [PATCH 47/80] [ache_check] hint array damage reporting --- caching/cache_check.cc | 28 ++++++++++++++++++++++------ caching/hint_array.cc | 2 +- caching/hint_array.h | 2 +- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index b660e01..304037b 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -94,7 +94,7 @@ namespace { } virtual void visit(mapping_array_damage::missing_mappings const &d) { - out() << "missing mappings:" << end_message(); + out() << "missing mappings " << d.keys_ << ":" << end_message(); { nested_output::nest _ = push(); out() << d.get_desc() << end_message(); @@ -116,13 +116,27 @@ namespace { mplus_error(FATAL); } + + using reporter_base::get_error; }; - class hint_reporter : public reporter_base { + class hint_reporter : public hint_array_damage::damage_visitor, reporter_base { public: hint_reporter(nested_output &o) : reporter_base(o) { } + + virtual void visit(hint_array_damage::missing_hints const &d) { + out() << "missing mappings " << d.keys_ << ":" << end_message(); + { + nested_output::nest _ = push(); + out() << d.get_desc() << end_message(); + } + + mplus_error(FATAL); + } + + using reporter_base::get_error; }; //-------------------------------- @@ -167,8 +181,6 @@ namespace { } error_state metadata_check(block_manager<>::ptr bm, flags const &fs) { - error_state err = NO_ERROR; - nested_output out(cerr, 2); if (fs.quiet_) out.disable(); @@ -194,7 +206,7 @@ namespace { { nested_output::nest _ = out.push(); mapping_array ma(tm, mapping_array::ref_counter(), sb.mapping_root, sb.cache_blocks); - // check_mapping_array(ma, mapping_rep); + check_mapping_array(ma, mapping_rep); } } @@ -202,10 +214,14 @@ namespace { out << "examining hint array" << end_message(); { nested_output::nest _ = out.push(); + hint_array ha(tm, sb.policy_hint_size, sb.hint_root, sb.cache_blocks); + ha.check(hint_rep); } } - return err; + return combine_errors(sb_rep.get_error(), + combine_errors(mapping_rep.get_error(), + hint_rep.get_error())); } int check(string const &path, flags const &fs) { diff --git a/caching/hint_array.cc b/caching/hint_array.cc index 0b95e48..effbd08 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -257,7 +257,7 @@ hint_array::grow(unsigned new_nr_entries, vector const &value) } void -hint_array::check_hint_array(hint_array_damage::damage_visitor &visitor) +hint_array::check(hint_array_damage::damage_visitor &visitor) { check_hints_(width_, impl_, visitor); } diff --git a/caching/hint_array.h b/caching/hint_array.h index e8483ac..fc9194e 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -65,7 +65,7 @@ namespace caching { void set_hint(unsigned index, vector const &data); void grow(unsigned new_nr_entries, vector const &value); - void check_hint_array(hint_array_damage::damage_visitor &visitor); + void check(hint_array_damage::damage_visitor &visitor); private: unsigned width_; From a20b50815e45dd2fb9046ff99bb942049a73406d Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 11:17:20 +0100 Subject: [PATCH 48/80] [cache_check] remove some dead code --- caching/cache_check.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 304037b..4deb346 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -237,13 +237,6 @@ namespace { try { block_manager<>::ptr bm = open_bm(path, block_io<>::READ_ONLY); err = metadata_check(bm, fs); -#if 0 - if (maybe_errors) { - if (!fs.quiet_) - cerr << error_selector(*maybe_errors, 3); - return 1; - } -#endif } catch (std::exception &e) { if (!fs.quiet_) From bd06f58f6ff5f7efa5650bba240e852b39e70a20 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 11:21:31 +0100 Subject: [PATCH 49/80] [cache_check] feature to test --quiet --- features/cache_check.feature | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/features/cache_check.feature b/features/cache_check.feature index 28687af..fc3516d 100644 --- a/features/cache_check.feature +++ b/features/cache_check.feature @@ -69,6 +69,13 @@ Feature: cache_check When I run `cache_check input` Then it should fail + Scenario: --quiet is observed + Given input file + And block 1 is zeroed + When I run `cache_check --quiet input` + Then it should fail + And it should give no output + Scenario: A valid metadata area passes Given metadata containing: """ From 505cf951098105fbf5f598886d5f9448221f534c Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 11:34:10 +0100 Subject: [PATCH 50/80] [cache_check] --super-block-only, --skip-mappings, --skip-hints --- caching/cache_check.cc | 28 ++++++++++++++++++++---- features/cache_check.feature | 7 ++++++ features/step_definitions/cache_steps.rb | 3 +++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 4deb346..5bcec34 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -252,7 +252,11 @@ namespace { << "Options:" << endl << " {-q|--quiet}" << endl << " {-h|--help}" << endl - << " {-V|--version}" << endl; + << " {-V|--version}" << endl + << " {--super-block-only}" << endl + << " {--skip-mappings}" << endl + << " {--skip-hints}" << endl; + } char const *TOOLS_VERSION = "0.1.6"; @@ -266,14 +270,30 @@ int main(int argc, char **argv) flags fs; const char shortopts[] = "qhV"; const struct option longopts[] = { - { "quiet", no_argument, NULL, 'q'}, - { "help", no_argument, NULL, 'h'}, - { "version", no_argument, NULL, 'V'}, + { "quiet", no_argument, NULL, 'q' }, + { "superblock-only", no_argument, NULL, 1 }, + { "skip-mappings", no_argument, NULL, 2 }, + { "skip-hints", no_argument, NULL, 3 }, + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, 'V' }, { NULL, no_argument, NULL, 0 } }; while ((c = getopt_long(argc, argv, shortopts, longopts, NULL)) != -1) { switch(c) { + case 1: + fs.check_mappings_ = false; + fs.check_hints_ = false; + break; + + case 2: + fs.check_mappings_ = false; + break; + + case 3: + fs.check_hints_ = false; + break; + case 'h': usage(cout, basename(argv[0])); return 0; diff --git a/features/cache_check.feature b/features/cache_check.feature index fc3516d..14e48fa 100644 --- a/features/cache_check.feature +++ b/features/cache_check.feature @@ -76,6 +76,13 @@ Feature: cache_check Then it should fail And it should give no output + Scenario: -q is observed + Given input file + And block 1 is zeroed + When I run `cache_check -q input` + Then it should fail + And it should give no output + Scenario: A valid metadata area passes Given metadata containing: """ diff --git a/features/step_definitions/cache_steps.rb b/features/step_definitions/cache_steps.rb index 68c5739..9635893 100644 --- a/features/step_definitions/cache_steps.rb +++ b/features/step_definitions/cache_steps.rb @@ -46,6 +46,9 @@ Options: {-q|--quiet} {-h|--help} {-V|--version} + {--super-block-only} + {--skip-mappings} + {--skip-hints} EOF Then /^usage to stdout$/ do From 2ecf0513367cc74d689d349815824556f94d559f Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 11:54:22 +0100 Subject: [PATCH 51/80] [cache_dump] add --repair flag (not functional yet) --- caching/cache_dump.cc | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index dfe7c2c..812d3b6 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -17,6 +17,14 @@ using namespace caching::superblock_damage; //---------------------------------------------------------------- namespace { + struct flags { + flags() + : repair_(false) { + } + + bool repair_; + }; + string to_string(unsigned char const *data) { // FIXME: we're assuming the data is zero terminated here return std::string(reinterpret_cast(data)); @@ -30,7 +38,7 @@ namespace { return output == STDOUT_PATH; } - int dump_(string const &dev, ostream &out) { + int dump_(string const &dev, ostream &out, flags const &fs) { block_manager<>::ptr bm = open_bm(dev, block_io<>::READ_ONLY); metadata::ptr md(new metadata(bm, metadata::OPEN)); emitter::ptr e = create_xml_emitter(out); @@ -56,13 +64,13 @@ namespace { return 0; } - int dump(string const &dev, string const &output) { + int dump(string const &dev, string const &output, flags const &fs) { try { if (want_stdout(output)) - return dump_(dev, cout); + return dump_(dev, cout, fs); else { ofstream out(output.c_str()); - return dump_(dev, out); + return dump_(dev, out, fs); } } catch (std::exception &e) { @@ -76,7 +84,8 @@ namespace { << "Options:" << endl << " {-h|--help}" << endl << " {-o }" << endl - << " {-V|--version}" << endl; + << " {-V|--version}" << endl + << " {--repair}" << endl; } } @@ -85,18 +94,24 @@ namespace { int main(int argc, char **argv) { int c; + flags fs; string output("-"); char const shortopts[] = "ho:V"; option const longopts[] = { - { "help", no_argument, NULL, 'h'}, - { "output", required_argument, NULL, 'o'}, - { "version", no_argument, NULL, 'V'}, + { "help", no_argument, NULL, 'h' }, + { "output", required_argument, NULL, 'o' }, + { "version", no_argument, NULL, 'V' }, + { "repair", no_argument, NULL, 1 }, { NULL, no_argument, NULL, 0 } }; while ((c = getopt_long(argc, argv, shortopts, longopts, NULL)) != -1) { switch(c) { + case 1: + fs.repair_ = true; + break; + case 'h': usage(cout, basename(argv[0])); return 0; @@ -121,7 +136,7 @@ int main(int argc, char **argv) return 1; } - return dump(argv[optind], output); + return dump(argv[optind], output, fs); } //---------------------------------------------------------------- From 08142c0c6659f2e9b2ad3d06d9d9e372911d6f88 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 12:12:01 +0100 Subject: [PATCH 52/80] [caching] introduce walk_mapping_array() --- caching/mapping_array.cc | 22 ++++++++++++++-------- caching/mapping_array.h | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/caching/mapping_array.cc b/caching/mapping_array.cc index 8a7c792..d31c2c9 100644 --- a/caching/mapping_array.cc +++ b/caching/mapping_array.cc @@ -58,15 +58,13 @@ invalid_mapping::visit(damage_visitor &v) const } namespace { - class mapping_visitor { + class check_mapping_visitor : public mapping_visitor { public: - mapping_visitor(damage_visitor &visitor) + check_mapping_visitor(damage_visitor &visitor) : visitor_(visitor) { } - virtual void visit(uint32_t index, mapping const &m) { - block_address cblock = index; - + virtual void visit(block_address cblock, mapping const &m) { if (!valid_mapping(m)) return; @@ -116,11 +114,19 @@ namespace { } void -caching::check_mapping_array(mapping_array const &array, damage_visitor &visitor) +caching::walk_mapping_array(mapping_array const &array, + mapping_visitor &mv, + damage_visitor &dv) { - mapping_visitor mv(visitor); - ll_damage_visitor ll(visitor); + ll_damage_visitor ll(dv); array.visit_values(mv, ll); } +void +caching::check_mapping_array(mapping_array const &array, damage_visitor &visitor) +{ + check_mapping_visitor mv(visitor); + walk_mapping_array(array, mv, visitor); +} + //---------------------------------------------------------------- diff --git a/caching/mapping_array.h b/caching/mapping_array.h index 29f9390..573d7f6 100644 --- a/caching/mapping_array.h +++ b/caching/mapping_array.h @@ -75,6 +75,21 @@ namespace caching { typedef persistent_data::array mapping_array; + class mapping_visitor { + public: + virtual ~mapping_visitor() {} + + void visit(uint32_t index, mapping const &m) { + visit(static_cast(index), m); + } + + virtual void visit(block_address cblock, mapping const &m) = 0; + }; + + void walk_mapping_array(mapping_array const &array, + mapping_visitor &mv, + mapping_array_damage::damage_visitor &dv); + void check_mapping_array(mapping_array const &array, mapping_array_damage::damage_visitor &visitor); } From 024216a5ff6fc543b1e20be18f0f729fbe3ad9e7 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 12:39:34 +0100 Subject: [PATCH 53/80] [caching] a couple of methods didn't need to be virtual --- caching/hint_array.h | 2 +- caching/mapping_array.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/caching/hint_array.h b/caching/hint_array.h index fc9194e..2818b80 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -39,7 +39,7 @@ namespace caching { public: virtual ~damage_visitor() {} - virtual void visit(damage const &d) { + void visit(damage const &d) { d.visit(*this); } diff --git a/caching/mapping_array.h b/caching/mapping_array.h index 573d7f6..00d58d8 100644 --- a/caching/mapping_array.h +++ b/caching/mapping_array.h @@ -64,7 +64,7 @@ namespace caching { public: virtual ~damage_visitor() {} - virtual void visit(mapping_array_damage::damage const &d) { + void visit(mapping_array_damage::damage const &d) { d.visit(*this); } From d02b3b6399fac8747bc012937b623789063140c4 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 8 Oct 2013 12:41:40 +0100 Subject: [PATCH 54/80] [cache_dump] dumping mappings respects --repair --- caching/cache_dump.cc | 62 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index 812d3b6..edc44e5 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -32,6 +32,48 @@ namespace { //-------------------------------- + class mapping_emitter : public mapping_visitor { + public: + mapping_emitter(emitter::ptr e) + : e_(e) { + } + + void visit(block_address cblock, mapping const &m) { + if (m.flags_ & M_VALID) + e_->mapping(cblock, m.oblock_, m.flags_ & M_DIRTY); + } + + private: + emitter::ptr e_; + }; + + struct ignore_mapping_damage : public mapping_array_damage::damage_visitor { + virtual void visit(missing_mappings const &d) { + } + + virtual void visit(invalid_mapping const &d) { + } + }; + + class fatal_mapping_damage : public mapping_array_damage::damage_visitor { + public: + virtual void visit(missing_mappings const &d) { + raise(); + } + + virtual void visit(invalid_mapping const &d) { + raise(); + } + + private: + static void raise() { + throw std::runtime_error("metadata contains errors (run cache_check for details).\n" + "perhaps you wanted to run with --repair"); + } + }; + + //-------------------------------- + string const STDOUT_PATH("-"); bool want_stdout(string const &output) { @@ -49,16 +91,22 @@ namespace { sb.policy_hint_size); e->begin_mappings(); - - for (unsigned cblock = 0; cblock < sb.cache_blocks; cblock++) { - mapping m = md->mappings_->get(cblock); - - if (m.flags_ & M_VALID) - e->mapping(cblock, m.oblock_, m.flags_ & M_DIRTY); + { + mapping_emitter me(e); + ignore_mapping_damage ignore; + fatal_mapping_damage fatal; + mapping_array_damage::damage_visitor &dv = fs.repair_ ? + static_cast(ignore) : + static_cast(fatal); + walk_mapping_array(*md->mappings_, me, dv); } - e->end_mappings(); + // walk hints + + + // walk discards + e->end_superblock(); return 0; From 19a1596591736b75bd251ff5ba32249b4f3fc28c Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 9 Oct 2013 09:45:17 +0100 Subject: [PATCH 55/80] [caching/hint_array] add walk method --- caching/hint_array.cc | 42 +++++++++++++++++++++++++++++++----------- caching/hint_array.h | 7 +++++++ 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/caching/hint_array.cc b/caching/hint_array.cc index effbd08..cb7b2bf 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -162,9 +162,22 @@ namespace { //-------------------------------- - template - struct no_op_visitor { - virtual void visit(uint32_t index, ValueType const &v) { + class value_adapter { + public: + value_adapter(hint_visitor &v) + : v_(v) { + } + + virtual void visit(uint32_t index, std::vector const &v) { + v_.visit(static_cast(index), v); + } + + private: + hint_visitor &v_; + }; + + struct no_op_visitor : public hint_visitor { + virtual void visit(block_address cblock, std::vector const &v) { } }; @@ -183,20 +196,20 @@ namespace { }; template - void check_hints(shared_ptr base, damage_visitor &visitor) { + void walk_hints(shared_ptr base, hint_visitor &hv, damage_visitor &dv) { typedef hint_traits traits; typedef array ha; shared_ptr a = downcast_array(base); - no_op_visitor nv; - ll_damage_visitor ll(visitor); - a->visit_values(nv, ll); + value_adapter vv(hv); + ll_damage_visitor ll(dv); + a->visit_values(vv, ll); } - void check_hints_(uint32_t width, shared_ptr base, - damage_visitor &visitor) { + void walk_hints_(uint32_t width, shared_ptr base, + hint_visitor &hv, damage_visitor &dv) { switch (width) { -#define xx(n) case n: check_hints(base, visitor); break +#define xx(n) case n: walk_hints(base, hv, dv); break all_widths #undef xx } @@ -256,10 +269,17 @@ hint_array::grow(unsigned new_nr_entries, vector const &value) grow_(width_, impl_, new_nr_entries, value); } +void +hint_array::walk(hint_visitor &hv, hint_array_damage::damage_visitor &dv) +{ + walk_hints_(width_, impl_, hv, dv); +} + void hint_array::check(hint_array_damage::damage_visitor &visitor) { - check_hints_(width_, impl_, visitor); + no_op_visitor vv; + walk(vv, visitor); } //---------------------------------------------------------------- diff --git a/caching/hint_array.h b/caching/hint_array.h index 2818b80..fccf7b3 100644 --- a/caching/hint_array.h +++ b/caching/hint_array.h @@ -47,6 +47,12 @@ namespace caching { }; } + class hint_visitor { + public: + virtual ~hint_visitor() {} + virtual void visit(block_address cblock, std::vector const &data) = 0; + }; + class hint_array { public: typedef boost::shared_ptr ptr; @@ -65,6 +71,7 @@ namespace caching { void set_hint(unsigned index, vector const &data); void grow(unsigned new_nr_entries, vector const &value); + void walk(hint_visitor &hv, hint_array_damage::damage_visitor &dv); void check(hint_array_damage::damage_visitor &visitor); private: From 97f4f0b0bb909b4423a4c4f34b96c11903a2a7ab Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 9 Oct 2013 09:48:20 +0100 Subject: [PATCH 56/80] [cache_dump] hints are dumped, and respect --repair --- caching/cache_dump.cc | 70 ++++++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index edc44e5..18d36c6 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -11,8 +11,6 @@ using namespace std; using namespace caching; -using namespace caching::mapping_array_damage; -using namespace caching::superblock_damage; //---------------------------------------------------------------- @@ -30,6 +28,11 @@ namespace { return std::string(reinterpret_cast(data)); } + void raise_metadata_damage() { + throw std::runtime_error("metadata contains errors (run cache_check for details).\n" + "perhaps you wanted to run with --repair"); + } + //-------------------------------- class mapping_emitter : public mapping_visitor { @@ -48,27 +51,44 @@ namespace { }; struct ignore_mapping_damage : public mapping_array_damage::damage_visitor { - virtual void visit(missing_mappings const &d) { - } - - virtual void visit(invalid_mapping const &d) { - } + virtual void visit(mapping_array_damage::missing_mappings const &d) {} + virtual void visit(mapping_array_damage::invalid_mapping const &d) {} }; class fatal_mapping_damage : public mapping_array_damage::damage_visitor { public: - virtual void visit(missing_mappings const &d) { - raise(); + virtual void visit(mapping_array_damage::missing_mappings const &d) { + raise_metadata_damage(); } - virtual void visit(invalid_mapping const &d) { - raise(); + virtual void visit(mapping_array_damage::invalid_mapping const &d) { + raise_metadata_damage(); + } + }; + + //-------------------------------- + + class hint_emitter : public hint_visitor { + public: + hint_emitter(emitter::ptr e) + : e_(e) { + } + + virtual void visit(block_address cblock, std::vector const &data) { + e_->hint(cblock, data); } private: - static void raise() { - throw std::runtime_error("metadata contains errors (run cache_check for details).\n" - "perhaps you wanted to run with --repair"); + emitter::ptr e_; + }; + + struct ignore_hint_damage : public hint_array_damage::damage_visitor { + virtual void visit(hint_array_damage::missing_hints const &d) {} + }; + + class fatal_hint_damage : public hint_array_damage::damage_visitor { + virtual void visit(hint_array_damage::missing_hints const &d) { + raise_metadata_damage(); } }; @@ -92,20 +112,34 @@ namespace { e->begin_mappings(); { + namespace mad = mapping_array_damage; + mapping_emitter me(e); ignore_mapping_damage ignore; fatal_mapping_damage fatal; - mapping_array_damage::damage_visitor &dv = fs.repair_ ? - static_cast(ignore) : - static_cast(fatal); + mad::damage_visitor &dv = fs.repair_ ? + static_cast(ignore) : + static_cast(fatal); walk_mapping_array(*md->mappings_, me, dv); } e->end_mappings(); // walk hints + e->begin_hints(); + { + using namespace hint_array_damage; + hint_emitter he(e); + ignore_hint_damage ignore; + fatal_hint_damage fatal; + damage_visitor &dv = fs.repair_ ? + static_cast(ignore) : + static_cast(fatal); + md->hints_->walk(he, dv); + } + e->end_hints(); - // walk discards + // FIXME: walk discards e->end_superblock(); From 9b9b7771e94a25dcc62a2e5109304a6dfdee17c7 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 9 Oct 2013 10:22:06 +0100 Subject: [PATCH 57/80] [caching] factor metadata_dump out of cache_dump --- Makefile.in | 2 + caching/cache_dump.cc | 131 ++++----------------------------------- caching/metadata_dump.cc | 124 ++++++++++++++++++++++++++++++++++++ caching/metadata_dump.h | 17 +++++ 4 files changed, 155 insertions(+), 119 deletions(-) create mode 100644 caching/metadata_dump.cc create mode 100644 caching/metadata_dump.h diff --git a/Makefile.in b/Makefile.in index f240904..5fc8c84 100644 --- a/Makefile.in +++ b/Makefile.in @@ -40,6 +40,7 @@ SOURCE=\ caching/superblock.cc \ caching/mapping_array.cc \ caching/metadata.cc \ + caching/metadata_dump.cc \ caching/restore_emitter.cc \ caching/xml_format.cc \ \ @@ -243,6 +244,7 @@ CACHE_CHECK_SOURCE=\ caching/superblock.cc \ caching/mapping_array.cc \ caching/metadata.cc \ + caching/metadata_dump.cc \ caching/restore_emitter.cc \ caching/xml_format.cc diff --git a/caching/cache_dump.cc b/caching/cache_dump.cc index 18d36c6..ea13c55 100644 --- a/caching/cache_dump.cc +++ b/caching/cache_dump.cc @@ -6,6 +6,7 @@ #include "version.h" #include "caching/mapping_array.h" #include "caching/metadata.h" +#include "caching/metadata_dump.h" #include "caching/xml_format.h" #include "persistent-data/file_utils.h" @@ -23,75 +24,6 @@ namespace { bool repair_; }; - string to_string(unsigned char const *data) { - // FIXME: we're assuming the data is zero terminated here - return std::string(reinterpret_cast(data)); - } - - void raise_metadata_damage() { - throw std::runtime_error("metadata contains errors (run cache_check for details).\n" - "perhaps you wanted to run with --repair"); - } - - //-------------------------------- - - class mapping_emitter : public mapping_visitor { - public: - mapping_emitter(emitter::ptr e) - : e_(e) { - } - - void visit(block_address cblock, mapping const &m) { - if (m.flags_ & M_VALID) - e_->mapping(cblock, m.oblock_, m.flags_ & M_DIRTY); - } - - private: - emitter::ptr e_; - }; - - struct ignore_mapping_damage : public mapping_array_damage::damage_visitor { - virtual void visit(mapping_array_damage::missing_mappings const &d) {} - virtual void visit(mapping_array_damage::invalid_mapping const &d) {} - }; - - class fatal_mapping_damage : public mapping_array_damage::damage_visitor { - public: - virtual void visit(mapping_array_damage::missing_mappings const &d) { - raise_metadata_damage(); - } - - virtual void visit(mapping_array_damage::invalid_mapping const &d) { - raise_metadata_damage(); - } - }; - - //-------------------------------- - - class hint_emitter : public hint_visitor { - public: - hint_emitter(emitter::ptr e) - : e_(e) { - } - - virtual void visit(block_address cblock, std::vector const &data) { - e_->hint(cblock, data); - } - - private: - emitter::ptr e_; - }; - - struct ignore_hint_damage : public hint_array_damage::damage_visitor { - virtual void visit(hint_array_damage::missing_hints const &d) {} - }; - - class fatal_hint_damage : public hint_array_damage::damage_visitor { - virtual void visit(hint_array_damage::missing_hints const &d) { - raise_metadata_damage(); - } - }; - //-------------------------------- string const STDOUT_PATH("-"); @@ -100,65 +32,26 @@ namespace { return output == STDOUT_PATH; } - int dump_(string const &dev, ostream &out, flags const &fs) { - block_manager<>::ptr bm = open_bm(dev, block_io<>::READ_ONLY); - metadata::ptr md(new metadata(bm, metadata::OPEN)); - emitter::ptr e = create_xml_emitter(out); - - superblock const &sb = md->sb_; - e->begin_superblock(to_string(sb.uuid), sb.data_block_size, - sb.cache_blocks, to_string(sb.policy_name), - sb.policy_hint_size); - - e->begin_mappings(); - { - namespace mad = mapping_array_damage; - - mapping_emitter me(e); - ignore_mapping_damage ignore; - fatal_mapping_damage fatal; - mad::damage_visitor &dv = fs.repair_ ? - static_cast(ignore) : - static_cast(fatal); - walk_mapping_array(*md->mappings_, me, dv); - } - e->end_mappings(); - - // walk hints - e->begin_hints(); - { - using namespace hint_array_damage; - - hint_emitter he(e); - ignore_hint_damage ignore; - fatal_hint_damage fatal; - damage_visitor &dv = fs.repair_ ? - static_cast(ignore) : - static_cast(fatal); - md->hints_->walk(he, dv); - } - e->end_hints(); - - // FIXME: walk discards - - e->end_superblock(); - - return 0; - } - int dump(string const &dev, string const &output, flags const &fs) { try { - if (want_stdout(output)) - return dump_(dev, cout, fs); - else { + block_manager<>::ptr bm = open_bm(dev, block_io<>::READ_ONLY); + metadata::ptr md(new metadata(bm, metadata::OPEN)); + + if (want_stdout(output)) { + emitter::ptr e = create_xml_emitter(cout); + metadata_dump(md, e, fs.repair_); + } else { ofstream out(output.c_str()); - return dump_(dev, out, fs); + emitter::ptr e = create_xml_emitter(out); + metadata_dump(md, e, fs.repair_); } } catch (std::exception &e) { cerr << e.what() << endl; return 1; } + + return 0; } void usage(ostream &out, string const &cmd) { diff --git a/caching/metadata_dump.cc b/caching/metadata_dump.cc new file mode 100644 index 0000000..7521809 --- /dev/null +++ b/caching/metadata_dump.cc @@ -0,0 +1,124 @@ +#include "caching/metadata_dump.h" + +using namespace std; +using namespace caching; + +//---------------------------------------------------------------- + +namespace { + string to_string(unsigned char const *data) { + // FIXME: we're assuming the data is zero terminated here + return std::string(reinterpret_cast(data)); + } + + void raise_metadata_damage() { + throw std::runtime_error("metadata contains errors (run cache_check for details).\n" + "perhaps you wanted to run with --repair"); + } + + //-------------------------------- + + class mapping_emitter : public mapping_visitor { + public: + mapping_emitter(emitter::ptr e) + : e_(e) { + } + + void visit(block_address cblock, mapping const &m) { + if (m.flags_ & M_VALID) + e_->mapping(cblock, m.oblock_, m.flags_ & M_DIRTY); + } + + private: + emitter::ptr e_; + }; + + struct ignore_mapping_damage : public mapping_array_damage::damage_visitor { + virtual void visit(mapping_array_damage::missing_mappings const &d) {} + virtual void visit(mapping_array_damage::invalid_mapping const &d) {} + }; + + class fatal_mapping_damage : public mapping_array_damage::damage_visitor { + public: + virtual void visit(mapping_array_damage::missing_mappings const &d) { + raise_metadata_damage(); + } + + virtual void visit(mapping_array_damage::invalid_mapping const &d) { + raise_metadata_damage(); + } + }; + + //-------------------------------- + + class hint_emitter : public hint_visitor { + public: + hint_emitter(emitter::ptr e) + : e_(e) { + } + + virtual void visit(block_address cblock, std::vector const &data) { + e_->hint(cblock, data); + } + + private: + emitter::ptr e_; + }; + + struct ignore_hint_damage : public hint_array_damage::damage_visitor { + virtual void visit(hint_array_damage::missing_hints const &d) {} + }; + + class fatal_hint_damage : public hint_array_damage::damage_visitor { + virtual void visit(hint_array_damage::missing_hints const &d) { + raise_metadata_damage(); + } + }; +} + +//---------------------------------------------------------------- + +void +caching::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair) +{ + superblock const &sb = md->sb_; + e->begin_superblock(to_string(sb.uuid), sb.data_block_size, + sb.cache_blocks, to_string(sb.policy_name), + sb.policy_hint_size); + + e->begin_mappings(); + { + namespace mad = mapping_array_damage; + + mapping_emitter me(e); + ignore_mapping_damage ignore; + fatal_mapping_damage fatal; + mad::damage_visitor &dv = repair ? + static_cast(ignore) : + static_cast(fatal); + walk_mapping_array(*md->mappings_, me, dv); + } + e->end_mappings(); + + // walk hints + e->begin_hints(); + { + using namespace hint_array_damage; + + hint_emitter he(e); + ignore_hint_damage ignore; + fatal_hint_damage fatal; + damage_visitor &dv = repair ? + static_cast(ignore) : + static_cast(fatal); + md->hints_->walk(he, dv); + } + e->end_hints(); + + // FIXME: walk discards + + e->end_superblock(); +} + +//---------------------------------------------------------------- + diff --git a/caching/metadata_dump.h b/caching/metadata_dump.h new file mode 100644 index 0000000..7dc1d38 --- /dev/null +++ b/caching/metadata_dump.h @@ -0,0 +1,17 @@ +#ifndef CACHING_METADATA_DUMP_H +#define CACHING_METADATA_DUMP_H + +#include "caching/metadata.h" +#include "caching/emitter.h" + +//---------------------------------------------------------------- + +namespace caching { + // If 'repair' is set then any metadata damage will be stepped + // around. Otherwise an exception will be thrown. + void metadata_dump(metadata::ptr md, emitter::ptr out, bool repair); +} + +//---------------------------------------------------------------- + +#endif From c6487fd283d5e6dfd449858d296a87ddc622c0ea Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 9 Oct 2013 10:42:14 +0100 Subject: [PATCH 58/80] [cache_repair] written --- Makefile.in | 8 +++ caching/cache_repair.cc | 108 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 caching/cache_repair.cc diff --git a/Makefile.in b/Makefile.in index 5fc8c84..ac3ce75 100644 --- a/Makefile.in +++ b/Makefile.in @@ -23,6 +23,7 @@ PROGRAMS=\ cache_check \ cache_dump \ cache_restore \ + cache_repair \ \ thin_check \ thin_dump \ @@ -253,6 +254,9 @@ CACHE_CHECK_OBJECTS=$(subst .cc,.o,$(CACHE_CHECK_SOURCE)) CACHE_DUMP_SOURCE=$(SOURCE) CACHE_DUMP_OBJECTS=$(subst .cc,.o,$(CACHE_DUMP_SOURCE)) +CACHE_REPAIR_SOURCE=$(SOURCE) +CACHE_REPAIR_OBJECTS=$(subst .cc,.o,$(CACHE_REPAIR_SOURCE)) + CACHE_RESTORE_SOURCE=$(SOURCE) CACHE_RESTORE_OBJECTS=$(subst .cc,.o,$(CACHE_RESTORE_SOURCE)) @@ -264,6 +268,10 @@ cache_dump: $(CACHE_DUMP_OBJECTS) caching/cache_dump.o @echo " [LD] $@" $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) +cache_repair: $(CACHE_REPAIR_OBJECTS) caching/cache_repair.o + @echo " [LD] $@" + $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) + cache_restore: $(CACHE_RESTORE_OBJECTS) caching/cache_restore.o @echo " [LD] $@" $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) diff --git a/caching/cache_repair.cc b/caching/cache_repair.cc new file mode 100644 index 0000000..5d1aafa --- /dev/null +++ b/caching/cache_repair.cc @@ -0,0 +1,108 @@ +#include +#include +#include + +#include "caching/metadata.h" +#include "caching/metadata_dump.h" +#include "caching/restore_emitter.h" +#include "persistent-data/file_utils.h" +#include "version.h" + +using namespace persistent_data; +using namespace std; +using namespace caching; + +//---------------------------------------------------------------- + +namespace { + metadata::ptr open_metadata_for_read(string const &path) { + block_manager<>::ptr bm = open_bm(path, block_io<>::READ_ONLY); + return metadata::ptr(new metadata(bm, metadata::OPEN)); + } + + emitter::ptr output_emitter(string const &path) { + block_manager<>::ptr bm = open_bm(path, block_io<>::READ_WRITE); + metadata::ptr md(new metadata(bm, metadata::CREATE)); + return create_restore_emitter(md); + } + + int repair(string const &old_path, string const &new_path) { + try { + metadata_dump(open_metadata_for_read(old_path), + output_emitter(new_path), + true); + + } catch (std::exception &e) { + cerr << e.what() << endl; + return 1; + } + + return 0; + } + + void usage(ostream &out, string const &cmd) { + out << "Usage: " << cmd << " [options] {device|file}" << endl + << "Options:" << endl + << " {-h|--help}" << endl + << " {-i|--input} " << endl + << " {-o|--output} " << endl + << " {-V|--version}" << endl; + } +} + +//---------------------------------------------------------------- + +int main(int argc, char **argv) +{ + int c; + boost::optional input_path, output_path; + const char shortopts[] = "hi:o:V"; + + const struct option longopts[] = { + { "help", no_argument, NULL, 'h'}, + { "input", required_argument, NULL, 'i'}, + { "output", required_argument, NULL, 'o'}, + { "version", no_argument, NULL, 'V'}, + { NULL, no_argument, NULL, 0 } + }; + + while ((c = getopt_long(argc, argv, shortopts, longopts, NULL)) != -1) { + switch(c) { + case 'h': + usage(cout, basename(argv[0])); + return 0; + + case 'i': + input_path = optarg; + break; + + case 'o': + output_path = optarg; + break; + + case 'V': + cout << THIN_PROVISIONING_TOOLS_VERSION << endl; + return 0; + + default: + usage(cerr, basename(argv[0])); + return 1; + } + } + + if (!input_path) { + cerr << "no input file provided" << endl; + usage(cerr, basename(argv[0])); + return 1; + } + + if (!output_path) { + cerr << "no output file provided" << endl; + usage(cerr, basename(argv[0])); + return 1; + } + + return repair(*input_path, *output_path); +} + +//---------------------------------------------------------------- From 83f1e4bdd9b1dac477339af29085ccc3b2c6f6ee Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Wed, 9 Oct 2013 10:49:53 +0100 Subject: [PATCH 59/80] [caching/metadata_dump] only dump valid hints --- caching/metadata_dump.cc | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/caching/metadata_dump.cc b/caching/metadata_dump.cc index 7521809..d0e5367 100644 --- a/caching/metadata_dump.cc +++ b/caching/metadata_dump.cc @@ -1,5 +1,7 @@ #include "caching/metadata_dump.h" +#include + using namespace std; using namespace caching; @@ -20,17 +22,26 @@ namespace { class mapping_emitter : public mapping_visitor { public: - mapping_emitter(emitter::ptr e) - : e_(e) { + mapping_emitter(emitter::ptr e, set &valid_blocks) + : e_(e), + valid_blocks_(valid_blocks) { } void visit(block_address cblock, mapping const &m) { - if (m.flags_ & M_VALID) + if (m.flags_ & M_VALID) { e_->mapping(cblock, m.oblock_, m.flags_ & M_DIRTY); + mark_valid(cblock); + } } private: + void mark_valid(block_address cblock) { + valid_blocks_.insert(cblock); + } + + emitter::ptr e_; + set &valid_blocks_; }; struct ignore_mapping_damage : public mapping_array_damage::damage_visitor { @@ -53,16 +64,23 @@ namespace { class hint_emitter : public hint_visitor { public: - hint_emitter(emitter::ptr e) - : e_(e) { + hint_emitter(emitter::ptr e, set valid_blocks) + : e_(e), + valid_blocks_(valid_blocks) { } virtual void visit(block_address cblock, std::vector const &data) { - e_->hint(cblock, data); + if (valid(cblock)) + e_->hint(cblock, data); } private: + bool valid(block_address cblock) const { + return valid_blocks_.find(cblock) != valid_blocks_.end(); + } + emitter::ptr e_; + set &valid_blocks_; }; struct ignore_hint_damage : public hint_array_damage::damage_visitor { @@ -81,6 +99,8 @@ namespace { void caching::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair) { + set valid_blocks; + superblock const &sb = md->sb_; e->begin_superblock(to_string(sb.uuid), sb.data_block_size, sb.cache_blocks, to_string(sb.policy_name), @@ -90,7 +110,7 @@ caching::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair) { namespace mad = mapping_array_damage; - mapping_emitter me(e); + mapping_emitter me(e, valid_blocks); ignore_mapping_damage ignore; fatal_mapping_damage fatal; mad::damage_visitor &dv = repair ? @@ -105,7 +125,7 @@ caching::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair) { using namespace hint_array_damage; - hint_emitter he(e); + hint_emitter he(e, valid_blocks); ignore_hint_damage ignore; fatal_hint_damage fatal; damage_visitor &dv = repair ? From a29b5c8d07df1885694f899d0fc013fba6d03b19 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 10:18:46 +0100 Subject: [PATCH 60/80] [base] bse64 encoder Really slow implementation. Speed up on a rainy day. --- Makefile.in | 2 + base/base64.cc | 186 +++++++++++++++++++++++++++++++++++++++++ base/base64.h | 20 +++++ unit-tests/Makefile.in | 1 + unit-tests/base64_t.cc | 121 +++++++++++++++++++++++++++ 5 files changed, 330 insertions(+) create mode 100644 base/base64.cc create mode 100644 base/base64.h create mode 100644 unit-tests/base64_t.cc diff --git a/Makefile.in b/Makefile.in index ac3ce75..0835d87 100644 --- a/Makefile.in +++ b/Makefile.in @@ -35,6 +35,7 @@ PROGRAMS=\ all: $(PROGRAMS) SOURCE=\ + base/base64.cc \ base/error_state.cc \ \ caching/hint_array.cc \ @@ -228,6 +229,7 @@ thin_metadata_size: thin-provisioning/thin_metadata_size.o # Cache tools CACHE_CHECK_SOURCE=\ + base/base64.cc \ base/error_state.cc \ persistent-data/checksum.cc \ persistent-data/endian_utils.cc \ diff --git a/base/base64.cc b/base/base64.cc new file mode 100644 index 0000000..311d74f --- /dev/null +++ b/base/base64.cc @@ -0,0 +1,186 @@ +#include "base/base64.h" + +#include +#include +#include + +using namespace base; +using namespace boost; +using namespace std; + +//---------------------------------------------------------------- + +namespace { + char const *table_ = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + + struct index_set { + unsigned nr_valid_; + unsigned index_[4]; + }; + + index_set split1(unsigned char c) { + index_set r; + + r.nr_valid_ = 2; + r.index_[0] = c >> 2; + r.index_[1] = (c & 3) << 4; + + return r; + } + + index_set split2(unsigned char c1, unsigned char c2) { + index_set r; + + r.nr_valid_ = 3; + r.index_[0] = c1 >> 2; + r.index_[1] = ((c1 & 3) << 4) | (c2 >> 4); + r.index_[2] = (c2 & 15) << 2; + + return r; + } + + index_set split3(unsigned char c1, unsigned char c2, unsigned c3) { + index_set r; + + r.nr_valid_ = 4; + r.index_[0] = c1 >> 2; + r.index_[1] = ((c1 & 3) << 4) | (c2 >> 4); + r.index_[2] = ((c2 & 15) << 2) | (c3 >> 6); + r.index_[3] = c3 & 63; + + return r; + } + + index_set split(vector const &raw, unsigned index) { + unsigned remaining = std::min(raw.size() - index, 3); + + switch (remaining) { + case 1: + return split1(raw.at(index)); + + case 2: + return split2(raw.at(index), raw.at(index + 1)); + + case 3: + return split3(raw.at(index), raw.at(index + 1), raw.at(index + 2)); + } + + throw std::runtime_error("internal error, in split"); + } + + optional char_to_index(char c) { + // FIXME: very slow + for (unsigned i = 0; i < 64; i++) + if (table_[i] == c) + return optional(i); + + return optional(); + } + + decoded_or_error success(vector const &decoded) { + return decoded_or_error(decoded); + } + + decoded_or_error fail(string msg) { + return decoded_or_error(msg); + } + + decoded_or_error fail_char(char c) { + ostringstream msg; + msg << "bad input character: '" << c << "'"; + return fail(msg.str()); + } + + decoded_or_error decode_quad(char c1, char c2, char c3, char c4) { + typedef optional oi; + unsigned char d1, d2, d3; + vector decoded; + + oi i1 = char_to_index(c1); + if (!i1) + return fail_char(c1); + + oi i2 = char_to_index(c2); + if (!i2) + return fail_char(c2); + + d1 = (*i1 << 2) | (*i2 >> 4); + decoded.push_back(d1); + + d2 = (*i2 & 15) << 4; + + if (c3 == '=') { + // FIXME: I really think the push should be here +// decoded.push_back(d2); + return success(decoded); + } + + oi i3 = char_to_index(c3); + if (!i3) + return fail_char(c3); + + d2 = d2 | (*i3 >> 2); + decoded.push_back(d2); + + d3 = (*i3 & 3) << 6; + + if (c4 == '=') { + // FIXME: I really think the push should be here +// decoded.push_back(d3); + return success(decoded); + } + + oi i4 = char_to_index(c4); + if (!i4) + return fail_char(c4); + + d3 = d3 | *i4; + decoded.push_back(d3); + + return success(decoded); + } +} + +//---------------------------------------------------------------- + +string +base::base64_encode(vector const &raw) +{ + string r; + + for (unsigned i = 0; i < raw.size(); i += 3) { + unsigned j; + index_set is = split(raw, i); + + for (j = 0; j < is.nr_valid_; j++) + r.push_back(table_[is.index_[j]]); + + for (; j < 4; j++) + r.push_back('='); + } + + return r; +} + +base::decoded_or_error +base::base64_decode(string const &encoded) +{ + if (encoded.length() % 4) + return decoded_or_error("bad input length"); + + vector decoded; + + for (unsigned i = 0; i < encoded.length(); i += 4) { + decoded_or_error doe = decode_quad(encoded[i], encoded[i + 1], encoded[i + 2], encoded[i + 3]); + + vector *v = get >(&doe); + if (!v) + return doe; + + decoded.insert(decoded.end(), v->begin(), v->end()); + } + + return decoded_or_error(decoded); +} + +//---------------------------------------------------------------- diff --git a/base/base64.h b/base/base64.h new file mode 100644 index 0000000..340ae6c --- /dev/null +++ b/base/base64.h @@ -0,0 +1,20 @@ +#ifndef BASE_BASE64_H +#define BASE_BASE64_H + +#include +#include +#include + +//---------------------------------------------------------------- + +namespace base { + std::string base64_encode(std::vector const &raw); + + // Returns either the decoded data or an error string + typedef boost::variant, std::string> decoded_or_error; + decoded_or_error base64_decode(std::string const &encoded); +} + +//---------------------------------------------------------------- + +#endif diff --git a/unit-tests/Makefile.in b/unit-tests/Makefile.in index ffc1afc..db790e6 100644 --- a/unit-tests/Makefile.in +++ b/unit-tests/Makefile.in @@ -47,6 +47,7 @@ TEST_SOURCE=\ \ unit-tests/array_block_t.cc \ unit-tests/array_t.cc \ + unit-tests/base64_t.cc \ unit-tests/bitset_t.cc \ unit-tests/block_t.cc \ unit-tests/btree_t.cc \ diff --git a/unit-tests/base64_t.cc b/unit-tests/base64_t.cc new file mode 100644 index 0000000..c98dbe4 --- /dev/null +++ b/unit-tests/base64_t.cc @@ -0,0 +1,121 @@ +#include "gmock/gmock.h" +#include "base/base64.h" + +#include +#include + +using namespace base; +using namespace boost; +using namespace std; +using namespace testing; + +//---------------------------------------------------------------- + +namespace { + typedef vector bytes; + + char const *wikipedia_examples[] = { + "any carnal pleasure.", "YW55IGNhcm5hbCBwbGVhc3VyZS4=", + "any carnal pleasure", "YW55IGNhcm5hbCBwbGVhc3VyZQ==", + "any carnal pleasur", "YW55IGNhcm5hbCBwbGVhc3Vy", + "any carnal pleasu", "YW55IGNhcm5hbCBwbGVhc3U=", + "any carnal pleas", "YW55IGNhcm5hbCBwbGVhcw==", + "pleasure.", "cGxlYXN1cmUu", + "leasure.", "bGVhc3VyZS4=", + "easure.", "ZWFzdXJlLg==", + "asure.", "YXN1cmUu", + "sure.", "c3VyZS4=" + }; + + void assert_fails(decoded_or_error const &eoe, string const &msg) { + ASSERT_THAT(get(eoe), Eq(msg)); + } +}; + +//---------------------------------------------------------------- + +TEST(Base64Tests, encoding_an_empty_string) +{ + bytes bs; + ASSERT_THAT(base64_encode(bs), Eq(string())); +} + +TEST(Base64Tests, decoding_an_empty_string) +{ + bytes bs; + ASSERT_THAT(get >(base64_decode("")), Eq(bs)); +} + +TEST(Base64Tests, encode_single_byte) +{ + bytes bs(1); + bs[0] = 0; + + ASSERT_THAT(base64_encode(bs), Eq(string("AA=="))); +} + +TEST(Base64Tests, encode_double_byte) +{ + bytes bs(2, 0); + ASSERT_THAT(base64_encode(bs), Eq(string("AAA="))); +} + +TEST(Base64Tests, encode_triple_byte) +{ + bytes bs(3, 0); + ASSERT_THAT(base64_encode(bs), Eq(string("AAAA"))); +} + +TEST(Base64Tests, longer_encodings) +{ + for (unsigned example = 0; example < 5; example++) { + char const *in = wikipedia_examples[example * 2]; + char const *out = wikipedia_examples[example * 2 + 1]; + unsigned len = strlen(in); + bytes bs(len); + for (unsigned b = 0; b < len; b++) + bs.at(b) = in[b]; + + ASSERT_THAT(base64_encode(bs), Eq(string(out))); + } +} + +TEST(Base64Tests, decoding_fails_with_bad_size_input) +{ + char const *err = "bad input length"; + + assert_fails(base64_decode("AAA"), err); + assert_fails(base64_decode("AA"), err); + assert_fails(base64_decode("A"), err); +} + +TEST(Base64Tests, encode_decode_cycle) +{ + for (unsigned example = 0; example < 5; example++) { + char const *in = wikipedia_examples[example * 2]; + unsigned len = strlen(in); + bytes bs(len); + for (unsigned b = 0; b < len; b++) + bs.at(b) = in[b]; + + decoded_or_error doe = base64_decode(base64_encode(bs)); + ASSERT_THAT(get >(doe), Eq(bs)); + } +} + +TEST(Base64Tests, random_data) +{ + for (unsigned len = 1; len < 17; len++) { + for (unsigned example = 0; example < 10000; example++) { + vector raw(len); + + for (unsigned i = 0; i < len; i++) + raw.at(i) = ::rand() % 256; + + decoded_or_error doe = base64_decode(base64_encode(raw)); + ASSERT_THAT(get >(doe), Eq(raw)); + } + } +} + +//---------------------------------------------------------------- From d5a93691e136b06b914effce3a8f2de6a6039d15 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 10:34:37 +0100 Subject: [PATCH 61/80] [cache_dump/restore] Use base64 encoding for the hints --- caching/xml_format.cc | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/caching/xml_format.cc b/caching/xml_format.cc index 50180ff..f731c57 100644 --- a/caching/xml_format.cc +++ b/caching/xml_format.cc @@ -1,3 +1,4 @@ +#include "base/base64.h" #include "caching/xml_format.h" #include @@ -11,15 +12,6 @@ using namespace std; //---------------------------------------------------------------- namespace { - // base64 encoding? - string encode(vector const &data) { - return ""; - } - - vector decode(string const &data) { - return vector(); - } - //-------------------------------- // Emitter //-------------------------------- @@ -87,9 +79,11 @@ namespace { virtual void hint(block_address cblock, vector const &data) { + using namespace base; + out_ << "" << endl; } @@ -187,8 +181,18 @@ namespace { } void parse_hint(emitter *e, attributes const &attr) { - e->hint(get_attr(attr, "cache_block"), - decode(get_attr(attr, "data"))); + using namespace base; + + block_address cblock = get_attr(attr, "cache_block"); + decoded_or_error doe = base64_decode(get_attr(attr, "data")); + if (!get >(&doe)) { + ostringstream msg; + msg << "invalid base64 encoding of hint for cache block " + << cblock << ": " << get(doe); + throw runtime_error(msg.str()); + } + + e->hint(cblock, get >(doe)); } void start_tag(void *data, char const *el, char const **attr) { From 3d8eab297435d7b6c34dc3ccaf3713976219e84d Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 11:25:29 +0100 Subject: [PATCH 62/80] [caching/xml_format] hints weren't being indented --- caching/xml_format.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/caching/xml_format.cc b/caching/xml_format.cc index f731c57..f40d29b 100644 --- a/caching/xml_format.cc +++ b/caching/xml_format.cc @@ -81,6 +81,7 @@ namespace { vector const &data) { using namespace base; + indent(); out_ << " Date: Thu, 10 Oct 2013 11:26:13 +0100 Subject: [PATCH 63/80] [caching/hint_array] resize the destination before unpacking a hint --- caching/hint_array.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/caching/hint_array.cc b/caching/hint_array.cc index cb7b2bf..315ed42 100644 --- a/caching/hint_array.cc +++ b/caching/hint_array.cc @@ -17,6 +17,7 @@ namespace { // FIXME: slow copying for now static void unpack(disk_type const &disk, value_type &value) { + value.resize(WIDTH); for (unsigned byte = 0; byte < WIDTH; byte++) value.at(byte) = disk[byte]; } From 8af2f0cb1fc142fc3ad18a0d2d5ee039e9348db4 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 11:26:55 +0100 Subject: [PATCH 64/80] [caching/metadata] setup the hint array when reopening metadata --- caching/metadata.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/caching/metadata.cc b/caching/metadata.cc index 8ffe533..49e7a4d 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -107,6 +107,11 @@ metadata::open_metadata(block_manager<>::ptr bm) mapping_array::ref_counter(), sb_.mapping_root, sb_.cache_blocks)); + + if (sb_.hint_root) + hints_ = hint_array::ptr( + new hint_array(tm_, sb_.policy_hint_size, + sb_.hint_root, sb_.cache_blocks)); } void From b443000a291988a33cbb2faa416b8343ecf98b1d Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 11:27:34 +0100 Subject: [PATCH 65/80] [caching/metadata_dump] valid_blocks should have been passed by reference --- caching/metadata_dump.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caching/metadata_dump.cc b/caching/metadata_dump.cc index d0e5367..735d09b 100644 --- a/caching/metadata_dump.cc +++ b/caching/metadata_dump.cc @@ -64,7 +64,7 @@ namespace { class hint_emitter : public hint_visitor { public: - hint_emitter(emitter::ptr e, set valid_blocks) + hint_emitter(emitter::ptr e, set &valid_blocks) : e_(e), valid_blocks_(valid_blocks) { } From e9e46e871fec143cedb51f376d1cbba0614cadb0 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 11:38:28 +0100 Subject: [PATCH 66/80] [caching/xml_format] '/' was ommitted from hint tags --- caching/xml_format.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caching/xml_format.cc b/caching/xml_format.cc index f40d29b..71b7823 100644 --- a/caching/xml_format.cc +++ b/caching/xml_format.cc @@ -85,7 +85,7 @@ namespace { out_ << "" << endl; + << "/>" << endl; } private: From 347de67e2d9840b6b12bf9475148e1f753cf0dc3 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 12:17:34 +0100 Subject: [PATCH 67/80] [persistent-data/data-structures/bitset] Use pimpl idiom to hide implementation. --- Makefile.in | 1 + persistent-data/data-structures/bitset.cc | 202 ++++++++++++++++++++++ persistent-data/data-structures/bitset.h | 133 +------------- 3 files changed, 212 insertions(+), 124 deletions(-) create mode 100644 persistent-data/data-structures/bitset.cc diff --git a/Makefile.in b/Makefile.in index 0835d87..e8117a0 100644 --- a/Makefile.in +++ b/Makefile.in @@ -54,6 +54,7 @@ SOURCE=\ persistent-data/lock_tracker.cc \ persistent-data/transaction_manager.cc \ \ + persistent-data/data-structures/bitset.cc \ persistent-data/data-structures/btree.cc \ \ persistent-data/space_map.cc \ diff --git a/persistent-data/data-structures/bitset.cc b/persistent-data/data-structures/bitset.cc new file mode 100644 index 0000000..9400222 --- /dev/null +++ b/persistent-data/data-structures/bitset.cc @@ -0,0 +1,202 @@ +#include "persistent-data/data-structures/array.h" +#include "persistent-data/data-structures/bitset.h" +#include "persistent-data/math_utils.h" + +using namespace persistent_data; +using namespace persistent_data::bitset_detail; +using namespace std; + +//---------------------------------------------------------------- + +namespace { + struct bitset_traits { + typedef base::le64 disk_type; + typedef uint64_t value_type; + typedef no_op_ref_counter ref_counter; + + static void unpack(disk_type const &disk, value_type &value) { + value = base::to_cpu(disk); + } + + static void pack(value_type const &value, disk_type &disk) { + disk = base::to_disk(value); + } + }; +} + +namespace persistent_data { + namespace bitset_detail { + class bitset_impl { + public: + typedef boost::shared_ptr ptr; + typedef typename persistent_data::transaction_manager::ptr tm_ptr; + + bitset_impl(tm_ptr tm) + : nr_bits_(0), + array_(tm, rc_) { + } + + bitset_impl(tm_ptr tm, block_address root, unsigned nr_bits) + : nr_bits_(nr_bits), + array_(tm, rc_, root, nr_bits) { + } + + block_address get_root() const { + return array_.get_root(); + } + + void grow(unsigned new_nr_bits, bool default_value) { + pad_last_block(default_value); + resize_array(new_nr_bits, default_value); + nr_bits_ = new_nr_bits; + } + + void destroy() { + throw runtime_error("bitset.destroy() not implemented"); + } + + // May trigger a flush, so cannot be const + bool get(unsigned n) { + check_bounds(n); + return get_bit(array_.get(word(n)), bit(n)); + } + + void set(unsigned n, bool value) { + check_bounds(n); + unsigned w_index = word(n); + uint64_t w = array_.get(w_index); + if (value) + w = set_bit(w, bit(n)); + else + w = clear_bit(w, bit(n)); + array_.set(w_index, w); + } + + void flush() { + } + + private: + void pad_last_block(bool default_value) { + // Set defaults in the final word + if (bit(nr_bits_)) { + unsigned w_index = word(nr_bits_); + uint64_t w = array_.get(w_index); + + for (unsigned b = bit(nr_bits_); b < 64; b++) + if (default_value) + w = set_bit(w, b); + else + w = clear_bit(w, b); + + array_.set(w_index, w); + } + } + + void resize_array(unsigned new_nr_bits, bool default_value) { + unsigned old_nr_words = words_needed(nr_bits_); + unsigned new_nr_words = words_needed(new_nr_bits); + + if (new_nr_words < old_nr_words) + throw runtime_error("bitset grow actually asked to shrink"); + + if (new_nr_words > old_nr_words) + array_.grow(new_nr_words, default_value ? ~0 : 0); + } + + unsigned words_needed(unsigned nr_bits) const { + return base::div_up(nr_bits, 64u); + } + + unsigned word(unsigned bit) const { + return bit / 64; + } + + uint64_t mask(unsigned bit) const { + return 1ull << bit; + } + + bool get_bit(uint64_t w, unsigned bit) const { + return w & mask(bit); + } + + uint64_t set_bit(uint64_t w, unsigned bit) const { + return w | mask(bit); + } + + uint64_t clear_bit(uint64_t w, unsigned bit) const { + return w & (~mask(bit)); + } + + unsigned bit(unsigned bit) const { + return bit % 64; + } + + // The last word may be only partially full, so we have to + // do our own bounds checking rather than relying on array + // to do it. + void check_bounds(unsigned n) const { + if (n >= nr_bits_) { + std::ostringstream str; + str << "bitset index out of bounds (" + << n << " >= " << nr_bits_ << endl; + throw runtime_error(str.str()); + } + } + + unsigned nr_bits_; + no_op_ref_counter rc_; + array array_; + }; + } +} + +//---------------------------------------------------------------- + +bitset::bitset(tm_ptr tm) + : impl_(new bitset_impl(tm)) +{ +} + +bitset::bitset(tm_ptr tm, block_address root, unsigned nr_bits) + : impl_(new bitset_impl(tm, root, nr_bits)) +{ +} + +block_address +bitset::get_root() const +{ + return impl_->get_root(); +} + +void +bitset::grow(unsigned new_nr_bits, bool default_value) +{ + impl_->grow(new_nr_bits, default_value); +} + +void +bitset::destroy() +{ + impl_->destroy(); +} + +bool +bitset::get(unsigned n) +{ + return impl_->get(n); +} + +void +bitset::set(unsigned n, bool value) +{ + impl_->set(n, value); +} + +void +bitset::flush() +{ + impl_->flush(); +} + +//---------------------------------------------------------------- + diff --git a/persistent-data/data-structures/bitset.h b/persistent-data/data-structures/bitset.h index d0fa572..89fa5e1 100644 --- a/persistent-data/data-structures/bitset.h +++ b/persistent-data/data-structures/bitset.h @@ -19,26 +19,11 @@ #ifndef BITSET_H #define BITSET_H -#include "persistent-data/math_utils.h" -#include "persistent-data/data-structures/array.h" - //---------------------------------------------------------------- namespace persistent_data { namespace bitset_detail { - struct bitset_traits { - typedef base::le64 disk_type; - typedef uint64_t value_type; - typedef no_op_ref_counter ref_counter; - - static void unpack(disk_type const &disk, value_type &value) { - value = base::to_cpu(disk); - } - - static void pack(value_type const &value, disk_type &disk) { - disk = base::to_disk(value); - } - }; + class bitset_impl; } class bitset { @@ -46,119 +31,19 @@ namespace persistent_data { typedef boost::shared_ptr ptr; typedef typename persistent_data::transaction_manager::ptr tm_ptr; - bitset(tm_ptr tm) - : nr_bits_(0), - array_(tm, rc_) { - } - - bitset(tm_ptr tm, block_address root, unsigned nr_bits) - : nr_bits_(nr_bits), - array_(tm, rc_, root, nr_bits) { - } - - block_address get_root() const { - return array_.get_root(); - } - - void grow(unsigned new_nr_bits, bool default_value) { - pad_last_block(default_value); - resize_array(new_nr_bits, default_value); - nr_bits_ = new_nr_bits; - } - + bitset(tm_ptr tm); + bitset(tm_ptr tm, block_address root, unsigned nr_bits); + block_address get_root() const; + void grow(unsigned new_nr_bits, bool default_value); void destroy(); // May trigger a flush, so cannot be const - bool get(unsigned n) { - check_bounds(n); - return get_bit(array_.get(word(n)), bit(n)); - } - - void set(unsigned n, bool value) { - check_bounds(n); - unsigned w_index = word(n); - uint64_t w = array_.get(w_index); - if (value) - w = set_bit(w, bit(n)); - else - w = clear_bit(w, bit(n)); - array_.set(w_index, w); - } - - void flush() { - } + bool get(unsigned n); + void set(unsigned n, bool value); + void flush(); private: - void pad_last_block(bool default_value) { - // Set defaults in the final word - if (bit(nr_bits_)) { - unsigned w_index = word(nr_bits_); - uint64_t w = array_.get(w_index); - - for (unsigned b = bit(nr_bits_); b < 64; b++) - if (default_value) - w = set_bit(w, b); - else - w = clear_bit(w, b); - - array_.set(w_index, w); - } - } - - void resize_array(unsigned new_nr_bits, bool default_value) { - unsigned old_nr_words = words_needed(nr_bits_); - unsigned new_nr_words = words_needed(new_nr_bits); - - if (new_nr_words < old_nr_words) - throw runtime_error("bitset grow actually asked to shrink"); - - if (new_nr_words > old_nr_words) - array_.grow(new_nr_words, default_value ? ~0 : 0); - } - - unsigned words_needed(unsigned nr_bits) const { - return base::div_up(nr_bits, 64u); - } - - unsigned word(unsigned bit) const { - return bit / 64; - } - - uint64_t mask(unsigned bit) const { - return 1ull << bit; - } - - bool get_bit(uint64_t w, unsigned bit) const { - return w & mask(bit); - } - - uint64_t set_bit(uint64_t w, unsigned bit) const { - return w | mask(bit); - } - - uint64_t clear_bit(uint64_t w, unsigned bit) const { - return w & (~mask(bit)); - } - - unsigned bit(unsigned bit) const { - return bit % 64; - } - - // The last word may be only partially full, so we have to - // do our own bounds checking rather than relying on array - // to do it. - void check_bounds(unsigned n) const { - if (n >= nr_bits_) { - std::ostringstream str; - str << "bitset index out of bounds (" - << n << " >= " << nr_bits_ << endl; - throw runtime_error(str.str()); - } - } - - unsigned nr_bits_; - no_op_ref_counter rc_; - array array_; + boost::shared_ptr impl_; }; } From 9b5bf559ec51bbe36a061dd4ce0b12b3e748d297 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 12:55:10 +0100 Subject: [PATCH 68/80] [persistent-data/data-structures] add method to visit values and damage in a bitset --- persistent-data/data-structures/bitset.cc | 52 +++++++++++++++++++++++ persistent-data/data-structures/bitset.h | 22 ++++++++++ 2 files changed, 74 insertions(+) diff --git a/persistent-data/data-structures/bitset.cc b/persistent-data/data-structures/bitset.cc index 9400222..9625dae 100644 --- a/persistent-data/data-structures/bitset.cc +++ b/persistent-data/data-structures/bitset.cc @@ -2,6 +2,7 @@ #include "persistent-data/data-structures/bitset.h" #include "persistent-data/math_utils.h" +using namespace boost; using namespace persistent_data; using namespace persistent_data::bitset_detail; using namespace std; @@ -75,7 +76,52 @@ namespace persistent_data { void flush() { } + void walk_bitset(bitset_visitor &v) const { + bit_visitor vv(v); + damage_visitor dv(v); + array_.visit_values(vv, dv); + } + private: + class bit_visitor { + public: + bit_visitor(bitset_visitor &v) + : v_(v) { + } + + void visit(uint32_t word_index, uint64_t word) { + uint32_t bit_index = word_index * 64; + for (unsigned bit = 0; bit < 64; bit++, bit_index++) + v_.visit(bit_index, !!(word & (1 << bit))); + } + + private: + bitset_visitor &v_; + }; + + class damage_visitor { + public: + damage_visitor(bitset_visitor &v) + : v_(v) { + } + + void visit(array_detail::damage const &d) { + run bits(lifted_mult64(d.lost_keys_.begin_), + lifted_mult64(d.lost_keys_.end_)); + v_.visit(bits); + } + + private: + optional lifted_mult64(optional const &m) { + if (!m) + return m; + + return optional(*m * 64); + } + + bitset_visitor &v_; + }; + void pad_last_block(bool default_value) { // Set defaults in the final word if (bit(nr_bits_)) { @@ -198,5 +244,11 @@ bitset::flush() impl_->flush(); } +void +bitset::walk_bitset(bitset_visitor &v) const +{ + impl_->walk_bitset(v); +} + //---------------------------------------------------------------- diff --git a/persistent-data/data-structures/bitset.h b/persistent-data/data-structures/bitset.h index 89fa5e1..3e26985 100644 --- a/persistent-data/data-structures/bitset.h +++ b/persistent-data/data-structures/bitset.h @@ -19,11 +19,31 @@ #ifndef BITSET_H #define BITSET_H +#include "persistent-data/run.h" + //---------------------------------------------------------------- namespace persistent_data { namespace bitset_detail { class bitset_impl; + + class missing_bits { + public: + missing_bits(base::run const &keys) + : keys_(keys) { + } + + base::run keys_; + }; + + class bitset_visitor { + public: + typedef boost::shared_ptr ptr; + + virtual ~bitset_visitor(); + virtual void visit(uint32_t index, bool value); + virtual void visit(missing_bits const &d); + }; } class bitset { @@ -42,6 +62,8 @@ namespace persistent_data { void set(unsigned n, bool value); void flush(); + void walk_bitset(bitset_detail::bitset_visitor &v) const; + private: boost::shared_ptr impl_; }; From 5dbaf8371ca8216cb223df6424af95e671f27ed6 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Thu, 10 Oct 2013 13:08:04 +0100 Subject: [PATCH 69/80] [caching] start wiring up the discard bitset --- Makefile.in | 1 + caching/metadata.cc | 13 +++++++++++++ caching/metadata.h | 3 +++ 3 files changed, 17 insertions(+) diff --git a/Makefile.in b/Makefile.in index e8117a0..e0ce06f 100644 --- a/Makefile.in +++ b/Makefile.in @@ -239,6 +239,7 @@ CACHE_CHECK_SOURCE=\ persistent-data/hex_dump.cc \ persistent-data/lock_tracker.cc \ persistent-data/data-structures/btree.cc \ + persistent-data/data-structures/bitset.cc \ persistent-data/space_map.cc \ persistent-data/space-maps/disk.cc \ persistent-data/space-maps/recursive.cc \ diff --git a/caching/metadata.cc b/caching/metadata.cc index 49e7a4d..0f9b82f 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -52,6 +52,7 @@ metadata::commit() commit_space_map(); commit_mappings(); commit_hints(); + commit_discard_bits(); commit_superblock(); } @@ -94,6 +95,8 @@ metadata::create_metadata(block_manager<>::ptr bm) // We can't instantiate the hint array yet, since we don't know the // hint width. + + discard_bits_ = bitset::ptr(new bitset(tm_)); } void @@ -112,6 +115,10 @@ metadata::open_metadata(block_manager<>::ptr bm) hints_ = hint_array::ptr( new hint_array(tm_, sb_.policy_hint_size, sb_.hint_root, sb_.cache_blocks)); + + if (sb_.discard_root) + discard_bits_ = bitset::ptr( + new bitset(tm_, sb_.discard_root, sb_.cache_blocks)); } void @@ -133,6 +140,12 @@ metadata::commit_hints() sb_.hint_root = hints_->get_root(); } +void +metadata::commit_discard_bits() +{ + // FIXME: finish +} + void metadata::commit_superblock() { diff --git a/caching/metadata.h b/caching/metadata.h index 418d0e6..611b424 100644 --- a/caching/metadata.h +++ b/caching/metadata.h @@ -3,6 +3,7 @@ #include "persistent-data/block.h" #include "persistent-data/data-structures/array.h" +#include "persistent-data/data-structures/bitset.h" #include "persistent-data/endian_utils.h" #include "persistent-data/space-maps/disk.h" #include "persistent-data/transaction_manager.h" @@ -37,6 +38,7 @@ namespace caching { checked_space_map::ptr metadata_sm_; mapping_array::ptr mappings_; hint_array::ptr hints_; + bitset::ptr discard_bits_; private: void init_superblock(); @@ -47,6 +49,7 @@ namespace caching { void commit_space_map(); void commit_mappings(); void commit_hints(); + void commit_discard_bits(); void commit_superblock(); }; }; From 6b6f2290a74475bbdb7fb9a1d8fd4c04023596aa Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 10:03:51 +0100 Subject: [PATCH 70/80] [cache_dump/restore] add discards to the xml format --- caching/emitter.h | 6 ++++++ caching/restore_emitter.cc | 15 +++++++++++++++ caching/xml_format.cc | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/caching/emitter.h b/caching/emitter.h index 1563cdc..8a01d6c 100644 --- a/caching/emitter.h +++ b/caching/emitter.h @@ -37,6 +37,12 @@ namespace caching { virtual void hint(pd::block_address cblock, std::vector const &data) = 0; + + virtual void begin_discards() = 0; + virtual void end_discards() = 0; + + virtual void discard(pd::block_address dblock_begin, + pd::block_address dblock_end) = 0; }; } diff --git a/caching/restore_emitter.cc b/caching/restore_emitter.cc index 787163c..6c862bf 100644 --- a/caching/restore_emitter.cc +++ b/caching/restore_emitter.cc @@ -78,7 +78,22 @@ namespace { virtual void hint(pd::block_address cblock, vector const &data) { + md_->hints_->set_hint(cblock, data); + } + virtual void begin_discards() { + // noop + } + + virtual void end_discards() { + // noop + } + + virtual void discard(block_address dblock, block_address dblock_e) { + while (dblock != dblock_e) { + md_->discard_bits_->set(dblock, true); + dblock++; + } } private: diff --git a/caching/xml_format.cc b/caching/xml_format.cc index 71b7823..b03e997 100644 --- a/caching/xml_format.cc +++ b/caching/xml_format.cc @@ -88,6 +88,24 @@ namespace { << "/>" << endl; } + virtual void begin_discards() { + indent(); + out_ << "" << endl; + inc(); + } + + virtual void end_discards() { + dec(); + indent(); + out_ << "" << endl; + } + + virtual void discard(block_address dblock_b, block_address dblock_e) { + indent(); + out_ << "" << endl; + } + private: string as_truth(bool v) const { return v ? "true" : "false"; @@ -196,6 +214,12 @@ namespace { e->hint(cblock, get >(doe)); } + // FIXME: why passing e by ptr? + void parse_discard(emitter *e, attributes const &attr) { + e->discard(get_attr(attr, "dbegin"), + get_attr(attr, "dend")); + } + void start_tag(void *data, char const *el, char const **attr) { emitter *e = static_cast(data); attributes a; @@ -217,6 +241,12 @@ namespace { else if (!strcmp(el, "hint")) parse_hint(e, a); + else if (!strcmp(el, "discards")) + e->begin_discards(); + + else if (!strcmp(el, "discard")) + parse_discard(e, a); + else throw runtime_error("unknown tag type"); } @@ -241,6 +271,13 @@ namespace { // do nothing ; + else if (!strcmp(el, "discards")) + e->end_discards(); + + else if (!strcmp(el, "discard")) + // do nothing + ; + else throw runtime_error("unknown tag close"); } From e3d87c538618e6bb8916fb97cf14af29782bd95e Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 10:20:29 +0100 Subject: [PATCH 71/80] update ignore file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7b84141..3ffd3e6 100644 --- a/.gitignore +++ b/.gitignore @@ -19,6 +19,7 @@ thin_metadata_size cache_check cache_dump cache_restore +cache_repair *.metadata bad-metadata From ea15a329d786ba1184032211cb993da7e8ad13cd Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 10:22:02 +0100 Subject: [PATCH 72/80] [cache_check] Now checks discard bitset --- caching/cache_check.cc | 66 ++++++++++++++++++++---- caching/metadata.cc | 2 +- persistent-data/data-structures/bitset.h | 6 +-- 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 5bcec34..0bf4614 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -139,6 +139,24 @@ namespace { using reporter_base::get_error; }; + class discard_reporter : public bitset_detail::bitset_visitor, reporter_base { + public: + discard_reporter(nested_output &o) + : reporter_base(o) { + } + + virtual void visit(uint32_t index, bool value) { + // no op + } + + virtual void visit(bitset_detail::missing_bits const &d) { + out() << "missing discard bits " << d.keys_ << end_message(); + mplus_error(FATAL); + } + + using reporter_base::get_error; + }; + //-------------------------------- transaction_manager::ptr open_tm(block_manager<>::ptr bm) { @@ -152,14 +170,16 @@ namespace { struct flags { flags() - : check_mappings_(false), - check_hints_(false), + : check_mappings_(true), + check_hints_(true), + check_discards_(true), ignore_non_fatal_errors_(false), quiet_(false) { } bool check_mappings_; bool check_hints_; + bool check_discards_; bool ignore_non_fatal_errors_; bool quiet_; }; @@ -188,6 +208,7 @@ namespace { superblock_reporter sb_rep(out); mapping_reporter mapping_rep(out); hint_reporter hint_rep(out); + discard_reporter discard_rep(out); out << "examining superblock" << end_message(); { @@ -211,17 +232,37 @@ namespace { } if (fs.check_hints_) { - out << "examining hint array" << end_message(); - { - nested_output::nest _ = out.push(); - hint_array ha(tm, sb.policy_hint_size, sb.hint_root, sb.cache_blocks); - ha.check(hint_rep); + if (!sb.hint_root) + out << "no hint array present" << end_message(); + + else { + out << "examining hint array" << end_message(); + { + nested_output::nest _ = out.push(); + hint_array ha(tm, sb.policy_hint_size, sb.hint_root, sb.cache_blocks); + ha.check(hint_rep); + } } } + if (fs.check_discards_) { + if (!sb.discard_root) + out << "no discard bitset present" << end_message(); + + else { + out << "examining discard bitset" << end_message(); + { + nested_output::nest _ = out.push(); + bitset discards(tm, sb.discard_root, sb.discard_nr_blocks); + } + } + } + + // FIXME: make an error class that's an instance of mplus return combine_errors(sb_rep.get_error(), combine_errors(mapping_rep.get_error(), - hint_rep.get_error())); + combine_errors(hint_rep.get_error(), + discard_rep.get_error()))); } int check(string const &path, flags const &fs) { @@ -255,8 +296,8 @@ namespace { << " {-V|--version}" << endl << " {--super-block-only}" << endl << " {--skip-mappings}" << endl - << " {--skip-hints}" << endl; - + << " {--skip-hints}" << endl + << " {--skip-discards}" << endl; } char const *TOOLS_VERSION = "0.1.6"; @@ -274,6 +315,7 @@ int main(int argc, char **argv) { "superblock-only", no_argument, NULL, 1 }, { "skip-mappings", no_argument, NULL, 2 }, { "skip-hints", no_argument, NULL, 3 }, + { "skip-discards", no_argument, NULL, 4 }, { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, 'V' }, { NULL, no_argument, NULL, 0 } @@ -294,6 +336,10 @@ int main(int argc, char **argv) fs.check_hints_ = false; break; + case 4: + fs.check_discards_ = false; + break; + case 'h': usage(cout, basename(argv[0])); return 0; diff --git a/caching/metadata.cc b/caching/metadata.cc index 0f9b82f..7ff3acb 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -118,7 +118,7 @@ metadata::open_metadata(block_manager<>::ptr bm) if (sb_.discard_root) discard_bits_ = bitset::ptr( - new bitset(tm_, sb_.discard_root, sb_.cache_blocks)); + new bitset(tm_, sb_.discard_root, sb_.discard_nr_blocks)); } void diff --git a/persistent-data/data-structures/bitset.h b/persistent-data/data-structures/bitset.h index 3e26985..b01845e 100644 --- a/persistent-data/data-structures/bitset.h +++ b/persistent-data/data-structures/bitset.h @@ -40,9 +40,9 @@ namespace persistent_data { public: typedef boost::shared_ptr ptr; - virtual ~bitset_visitor(); - virtual void visit(uint32_t index, bool value); - virtual void visit(missing_bits const &d); + virtual ~bitset_visitor() {} + virtual void visit(uint32_t index, bool value) = 0; + virtual void visit(missing_bits const &d) = 0; }; } From d54bc410265f915d169b502aa94f47dec9eb2ef3 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 10:33:32 +0100 Subject: [PATCH 73/80] [cache_check] fixup version number --- caching/cache_check.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index 0bf4614..dea9f24 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -18,6 +18,7 @@ #include "persistent-data/block.h" #include "persistent-data/file_utils.h" #include "persistent-data/space-maps/core.h" +#include "version.h" using namespace boost; using namespace caching; @@ -299,8 +300,6 @@ namespace { << " {--skip-hints}" << endl << " {--skip-discards}" << endl; } - - char const *TOOLS_VERSION = "0.1.6"; } //---------------------------------------------------------------- @@ -349,7 +348,7 @@ int main(int argc, char **argv) break; case 'V': - cout << TOOLS_VERSION << endl; + cout << THIN_PROVISIONING_TOOLS_VERSION << endl; return 0; default: From 6aae6aabb480e2f3c17d9ebb065fd246bf8bcc6e Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 11:02:04 +0100 Subject: [PATCH 74/80] [caching/metadata] superblock was being zeroed on creation, rather than letting the constructor do it's stuff. --- caching/metadata.cc | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/caching/metadata.cc b/caching/metadata.cc index 7ff3acb..bfb5a30 100644 --- a/caching/metadata.cc +++ b/caching/metadata.cc @@ -64,28 +64,11 @@ metadata::setup_hint_array(size_t width) new hint_array(tm_, width)); } -void -metadata::init_superblock() -{ -#if 0 - sb_.magic_ = SUPERBLOCK_MAGIC; - sb_.version_ = 1; - sb_.data_mapping_root_ = mappings_->get_root(); - sb_.device_details_root_ = details_->get_root(); - sb_.data_block_size_ = data_block_size; - sb_.metadata_block_size_ = MD_BLOCK_SIZE; - sb_.metadata_nr_blocks_ = tm_->get_bm()->get_nr_blocks(); -#endif -} - void metadata::create_metadata(block_manager<>::ptr bm) { tm_ = open_tm(bm); - ::memset(&sb_, 0, sizeof(sb_)); - init_superblock(); - space_map::ptr core = tm_->get_sm(); metadata_sm_ = create_metadata_sm(tm_, tm_->get_bm()->get_nr_blocks()); copy_space_maps(metadata_sm_, core); @@ -143,7 +126,7 @@ metadata::commit_hints() void metadata::commit_discard_bits() { - // FIXME: finish + sb_.discard_root = discard_bits_->get_root(); } void From ead63cd251007b3fa528f9c51eb24a117c835394 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 11:02:54 +0100 Subject: [PATCH 75/80] [caching/superblock] remove some old debug --- caching/superblock.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/caching/superblock.cc b/caching/superblock.cc index b3cfaee..f4d09db 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -107,7 +107,6 @@ superblock_traits::unpack(superblock_disk const &disk, superblock &core) core.policy_version[i] = to_cpu(disk.policy_version[i]); core.policy_hint_size = to_cpu(disk.policy_hint_size); - cerr << "unpacking: hint width = " << core.policy_hint_size << endl; ::memcpy(core.metadata_space_map_root, disk.metadata_space_map_root, From 3a0e07e921f3e56f23b51140dee4b107056b9c0b Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 11:03:26 +0100 Subject: [PATCH 76/80] [cache_check.feature] version nr wasn't being checked properly --- features/cache_check.feature | 14 +++++--------- features/step_definitions/cache_steps.rb | 6 ------ 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/features/cache_check.feature b/features/cache_check.feature index 14e48fa..67ce112 100644 --- a/features/cache_check.feature +++ b/features/cache_check.feature @@ -2,14 +2,12 @@ Feature: cache_check Scenario: print version (-V flag) When I run `cache_check -V` - Then it should pass - And version to stdout + Then it should pass with version Scenario: print version (--version flag) When I run `cache_check --version` - Then it should pass - And version to stdout + Then it should pass with version Scenario: print help When I run `cache_check --help` @@ -83,11 +81,9 @@ Feature: cache_check Then it should fail And it should give no output + @announce Scenario: A valid metadata area passes - Given metadata containing: - """ - """ - - When I run cache_check + Given valid cache metadata + When I run `cache_check metadata.bin` Then it should pass diff --git a/features/step_definitions/cache_steps.rb b/features/step_definitions/cache_steps.rb index 9635893..58217f4 100644 --- a/features/step_definitions/cache_steps.rb +++ b/features/step_definitions/cache_steps.rb @@ -34,12 +34,6 @@ Then /^it should fail$/ do assert_success(false) end -VERSION="0.1.6\n" - -Then /^version to stdout$/ do - assert_exact_output(VERSION, all_stdout) -end - USAGE =< Date: Fri, 11 Oct 2013 11:04:06 +0100 Subject: [PATCH 77/80] [cache_check] tweak exception handling; there was a window where they weren't caught --- caching/cache_check.cc | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/caching/cache_check.cc b/caching/cache_check.cc index dea9f24..cbe4581 100644 --- a/caching/cache_check.cc +++ b/caching/cache_check.cc @@ -276,17 +276,25 @@ namespace { throw runtime_error(msg.str()); } + block_manager<>::ptr bm = open_bm(path, block_io<>::READ_ONLY); + err = metadata_check(bm, fs); + + return err == NO_ERROR ? 0 : 1; + } + + int check_with_exception_handling(string const &path, flags const &fs) { + int r; try { - block_manager<>::ptr bm = open_bm(path, block_io<>::READ_ONLY); - err = metadata_check(bm, fs); + r = check(path, fs); } catch (std::exception &e) { if (!fs.quiet_) cerr << e.what() << endl; - return 1; + r = 1; } - return err == NO_ERROR ? 0 : 1; + return r; + } void usage(ostream &out, string const &cmd) { @@ -363,7 +371,7 @@ int main(int argc, char **argv) return 1; } - return check(argv[optind], fs); + return check_with_exception_handling(argv[optind], fs); } //---------------------------------------------------------------- From d2e81a0a19cbaad8e5cca09d17c7e9ece4efff7f Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 13:32:57 +0100 Subject: [PATCH 78/80] [caching] introduce superblock_flags class --- caching/superblock.cc | 55 +++++++++++++++++++++++++++++++++++++++---- caching/superblock.h | 27 ++++++++++++++++++++- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/caching/superblock.cc b/caching/superblock.cc index f4d09db..f560fdf 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -59,9 +59,53 @@ namespace { //---------------------------------------------------------------- +superblock_flags::superblock_flags() +{ +} + +superblock_flags::superblock_flags(uint32_t bits) +{ + if (bits & (1 << CLEAN_SHUTDOWN_BIT)) { + flags_.insert(CLEAN_SHUTDOWN); + bits &= ~(1 << CLEAN_SHUTDOWN_BIT); + } + + unhandled_flags_ = bits; +} + +void +superblock_flags::set_flag(superblock_flags::flag f) +{ + flags_.insert(f); +} + +bool +superblock_flags::get_flag(flag f) const +{ + return flags_.find(f) != flags_.end(); +} + +uint32_t +superblock_flags::encode() const +{ + uint32_t r = 0; + + if (get_flag(CLEAN_SHUTDOWN)) + r = r | (1 << CLEAN_SHUTDOWN_BIT); + + return r; +} + +uint32_t +superblock_flags::get_unhandled_flags() const +{ + return unhandled_flags_; +} + +//---------------------------------------------------------------- + superblock::superblock() : csum(0), - flags(0), blocknr(SUPERBLOCK_LOCATION), magic(SUPERBLOCK_MAGIC), version(VERSION_BEGIN), @@ -94,7 +138,8 @@ void superblock_traits::unpack(superblock_disk const &disk, superblock &core) { core.csum = to_cpu(disk.csum); - core.flags = to_cpu(disk.flags); + + core.flags = superblock_flags(to_cpu(disk.flags)); core.blocknr = to_cpu(disk.blocknr); ::memcpy(core.uuid, disk.uuid, sizeof(core.uuid)); @@ -137,7 +182,7 @@ void superblock_traits::pack(superblock const &core, superblock_disk &disk) { disk.csum = to_disk(core.csum); - disk.flags = to_disk(core.flags); + disk.flags = to_disk(core.flags.encode()); disk.blocknr = to_disk(core.blocknr); ::memcpy(disk.uuid, core.uuid, sizeof(disk.uuid)); @@ -257,9 +302,9 @@ caching::check_superblock(superblock const &sb, block_address nr_metadata_blocks, damage_visitor &visitor) { - if (sb.flags != 0) { + if (sb.flags.get_unhandled_flags()) { ostringstream msg; - msg << "invalid flags: " << hex << sb.flags; + msg << "invalid flags: " << sb.flags.get_unhandled_flags(); visitor.visit(superblock_invalid(msg.str())); } diff --git a/caching/superblock.h b/caching/superblock.h index 390970d..0b159c3 100644 --- a/caching/superblock.h +++ b/caching/superblock.h @@ -4,6 +4,8 @@ #include "persistent-data/endian_utils.h" #include "persistent-data/data-structures/btree.h" +#include + //---------------------------------------------------------------- namespace caching { @@ -14,11 +16,34 @@ namespace caching { unsigned const CACHE_POLICY_VERSION_SIZE = 3; block_address const SUPERBLOCK_LOCATION = 0; + class superblock_flags { + public: + enum flag { + CLEAN_SHUTDOWN + }; + + enum flag_bits { + CLEAN_SHUTDOWN_BIT = 0 + }; + + superblock_flags(); + superblock_flags(uint32_t bits); + + void set_flag(flag f); + bool get_flag(flag f) const; + uint32_t encode() const; + uint32_t get_unhandled_flags() const; + + private: + uint32_t unhandled_flags_; + std::set flags_; + }; + struct superblock { superblock(); uint32_t csum; - uint32_t flags; + superblock_flags flags; uint64_t blocknr; __u8 uuid[16]; From a31da1eb7180fffa00b448b83d1d8a29f4810478 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 13:33:19 +0100 Subject: [PATCH 79/80] [features] remove an old @announce --- features/cache_check.feature | 1 - 1 file changed, 1 deletion(-) diff --git a/features/cache_check.feature b/features/cache_check.feature index 67ce112..5fbc371 100644 --- a/features/cache_check.feature +++ b/features/cache_check.feature @@ -81,7 +81,6 @@ Feature: cache_check Then it should fail And it should give no output - @announce Scenario: A valid metadata area passes Given valid cache metadata When I run `cache_check metadata.bin` From 9a248672816adab4e05397938ea99fe94ce22553 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 11 Oct 2013 13:35:01 +0100 Subject: [PATCH 80/80] [caching/superblock] policy_version[] was in the wrong place in structure. --- caching/superblock.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/caching/superblock.cc b/caching/superblock.cc index f4d09db..81f99dc 100644 --- a/caching/superblock.cc +++ b/caching/superblock.cc @@ -18,7 +18,6 @@ namespace { le32 version; __u8 policy_name[CACHE_POLICY_NAME_SIZE]; - le32 policy_version[CACHE_POLICY_VERSION_SIZE]; le32 policy_hint_size; __u8 metadata_space_map_root[SPACE_MAP_ROOT_SIZE]; @@ -42,6 +41,8 @@ namespace { le32 read_misses; le32 write_hits; le32 write_misses; + + le32 policy_version[CACHE_POLICY_VERSION_SIZE]; } __attribute__ ((packed)); struct superblock_traits {