From 828f654800d7a96ce2abcde28ffa2f5af93e0623 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Tue, 26 Aug 2014 13:05:21 +0100 Subject: [PATCH] [*_restore] Add progress bar to cache_restore and era_restore. A lot of refactoring common code between the restore tools. --- CHANGES | 4 +- Makefile.in | 10 ++++- base/progress_monitor.cc | 8 ++-- base/progress_monitor.h | 7 ++-- base/xml_utils.cc | 63 ++++++++++++++++++++++++++++++- base/xml_utils.h | 7 ++++ caching/cache_restore.cc | 34 +++++++++++++++-- caching/xml_format.cc | 8 +++- caching/xml_format.h | 4 +- era/era_restore.cc | 21 ++++++++--- era/xml_format.cc | 19 +--------- era/xml_format.h | 5 ++- features/cache_restore.feature | 17 +++++++++ features/era_restore.feature | 18 ++++++++- features/thin_restore.feature | 14 +++++++ thin-provisioning/thin_restore.cc | 24 +----------- thin-provisioning/xml_format.cc | 25 +----------- thin-provisioning/xml_format.h | 3 +- 18 files changed, 199 insertions(+), 92 deletions(-) diff --git a/CHANGES b/CHANGES index 9871c86..0d1d79d 100644 --- a/CHANGES +++ b/CHANGES @@ -4,9 +4,9 @@ v0.4 - All tools switch to using libaio. This gives a large performance boost, especially to the write focused tools like thin_restore. -- Added a progress monitor to thin_restore +- Added a progress monitor to thin_restore, cache_restore and era_restore -- Added a --quiet/-q option to thin_restore to turn off the progress bar +- Added a --quiet/-q option to *_restore to turn off the progress bar - Removed variable hint size support from cache tools. The kernel still only supports a fixed 32bit width. This will have a side diff --git a/Makefile.in b/Makefile.in index 9f95eea..ed9e585 100644 --- a/Makefile.in +++ b/Makefile.in @@ -178,6 +178,7 @@ THIN_CHECK_SOURCE=\ \ base/error_state.cc \ base/endian_utils.cc \ + base/progress_monitor.cc \ base/xml_utils.cc \ \ persistent-data/checksum.cc \ @@ -202,6 +203,7 @@ THIN_DELTA_SOURCE=\ \ base/error_state.cc \ base/endian_utils.cc \ + base/progress_monitor.cc \ base/xml_utils.cc \ \ persistent-data/checksum.cc \ @@ -269,11 +271,11 @@ thin_restore: $(THIN_RESTORE_OBJECTS) thin-provisioning/thin_restore.o thin_check: $(THIN_CHECK_OBJECTS) thin-provisioning/thin_check.o @echo " [LD] $@" - $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) + $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) thin_delta: $(THIN_DELTA_OBJECTS) thin-provisioning/thin_delta.o @echo " [LD] $@" - $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) + $(V) $(CXX) $(CXXFLAGS) $(LDFLAGS) -o $@ $+ $(LIBS) $(LIBEXPAT) thin_rmap: $(THIN_RMAP_OBJECTS) thin-provisioning/thin_rmap.o @echo " [LD] $@" @@ -292,6 +294,7 @@ CACHE_CHECK_SOURCE=\ base/base64.cc \ base/error_state.cc \ base/endian_utils.cc \ + base/progress_monitor.cc \ base/xml_utils.cc \ \ persistent-data/checksum.cc \ @@ -354,6 +357,7 @@ ERA_CHECK_SOURCE=\ base/base64.cc \ base/error_state.cc \ base/endian_utils.cc \ + base/progress_monitor.cc \ base/xml_utils.cc \ \ era/writeset_tree.cc \ @@ -421,6 +425,7 @@ ERA_INVALIDATE_SOURCE=\ base/base64.cc \ base/error_state.cc \ base/endian_utils.cc \ + base/progress_monitor.cc \ base/xml_utils.cc \ \ era/writeset_tree.cc \ @@ -455,6 +460,7 @@ ERA_RESTORE_SOURCE=\ base/base64.cc \ base/error_state.cc \ base/endian_utils.cc \ + base/progress_monitor.cc \ base/xml_utils.cc \ \ era/writeset_tree.cc \ diff --git a/base/progress_monitor.cc b/base/progress_monitor.cc index 4e6792d..e6bf572 100644 --- a/base/progress_monitor.cc +++ b/base/progress_monitor.cc @@ -60,16 +60,16 @@ namespace { //---------------------------------------------------------------- -base::progress_monitor::ptr +std::auto_ptr base::create_progress_bar(std::string const &title) { - return progress_monitor::ptr(new progress_bar(title)); + return auto_ptr(new progress_bar(title)); } -base::progress_monitor::ptr +std::auto_ptr base::create_quiet_progress_monitor() { - return progress_monitor::ptr(new quiet_progress()); + return auto_ptr(new quiet_progress()); } //---------------------------------------------------------------- diff --git a/base/progress_monitor.h b/base/progress_monitor.h index 29b3d36..5472343 100644 --- a/base/progress_monitor.h +++ b/base/progress_monitor.h @@ -2,6 +2,7 @@ #define BASE_PROGRESS_MONITOR_H #include +#include #include //---------------------------------------------------------------- @@ -9,15 +10,13 @@ namespace base { class progress_monitor { public: - typedef boost::shared_ptr ptr; - virtual ~progress_monitor() {} virtual void update_percent(unsigned) = 0; }; - progress_monitor::ptr create_progress_bar(std::string const &title); - progress_monitor::ptr create_quiet_progress_monitor(); + std::auto_ptr create_progress_bar(std::string const &title); + std::auto_ptr create_quiet_progress_monitor(); } //---------------------------------------------------------------- diff --git a/base/xml_utils.cc b/base/xml_utils.cc index cc60f2d..fb34153 100644 --- a/base/xml_utils.cc +++ b/base/xml_utils.cc @@ -1,5 +1,67 @@ #include "xml_utils.h" +#include "persistent-data/file_utils.h" +#include +#include + +using namespace xml_utils; + +//---------------------------------------------------------------- + +void +xml_parser::parse(std::string const &backup_file, bool quiet) +{ + persistent_data::check_file_exists(backup_file); + ifstream in(backup_file.c_str(), ifstream::in); + + std::auto_ptr monitor = create_monitor(quiet); + + size_t total = 0; + size_t input_length = get_file_length(backup_file); + + 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()); + } + + total += len; + monitor->update_percent(total * 100 / input_length); + } +} + +size_t +xml_parser::get_file_length(string const &file) const +{ + struct stat info; + int r; + + r = ::stat(file.c_str(), &info); + if (r) + throw runtime_error("Couldn't stat backup path"); + + return info.st_size; +} + +auto_ptr +xml_parser::create_monitor(bool quiet) +{ + if (!quiet && isatty(fileno(stdout))) + return base::create_progress_bar("Restoring"); + else + return base::create_quiet_progress_monitor(); +} + //---------------------------------------------------------------- void @@ -21,5 +83,4 @@ xml_utils::build_attributes(attributes &a, char const **attr) } } - //---------------------------------------------------------------- diff --git a/base/xml_utils.h b/base/xml_utils.h index 892438e..581e814 100644 --- a/base/xml_utils.h +++ b/base/xml_utils.h @@ -1,9 +1,11 @@ #ifndef BASE_XML_UTILS_H #define BASE_XML_UTILS_H +#include #include #include #include +#include #include using namespace std; @@ -30,7 +32,12 @@ namespace xml_utils { return parser_; } + void parse(std::string const &backup_file, bool quiet); + private: + size_t get_file_length(string const &file) const; + auto_ptr create_monitor(bool quiet); + XML_Parser parser_; }; diff --git a/caching/cache_restore.cc b/caching/cache_restore.cc index d38d748..f9290d6 100644 --- a/caching/cache_restore.cc +++ b/caching/cache_restore.cc @@ -20,11 +20,30 @@ using namespace std; //---------------------------------------------------------------- namespace { + size_t get_file_length(string const &file) { + struct stat info; + int r; + + r = ::stat(file.c_str(), &info); + if (r) + throw runtime_error("Couldn't stat backup path"); + + return info.st_size; + } + + auto_ptr create_monitor(bool quiet) { + if (!quiet && isatty(fileno(stdout))) + return create_progress_bar("Restoring"); + else + return create_quiet_progress_monitor(); + } + struct flags { flags() : metadata_version(1), override_metadata_version(false), - clean_shutdown(true) { + clean_shutdown(true), + quiet(false) { } optional input; @@ -33,6 +52,7 @@ namespace { uint32_t metadata_version; bool override_metadata_version; bool clean_shutdown; + bool quiet; }; int restore(flags const &fs) { @@ -48,7 +68,9 @@ namespace { check_file_exists(*fs.input); ifstream in(fs.input->c_str(), ifstream::in); - parse_xml(in, restorer); + + auto_ptr monitor = create_monitor(fs.quiet); + parse_xml(in, restorer, get_file_length(*fs.input), *monitor); } catch (std::exception &e) { cerr << e.what() << endl; @@ -64,6 +86,7 @@ namespace { << " {-h|--help}" << endl << " {-i|--input} " << endl << " {-o|--output} " << endl + << " {-q|--quiet}" << endl << " {-V|--version}" << endl << endl << " {--debug-override-metadata-version} " << endl @@ -77,13 +100,14 @@ int main(int argc, char **argv) int c; flags fs; char const *prog_name = basename(argv[0]); - char const *short_opts = "hi:o:V"; + char const *short_opts = "hi:o:qV"; option const long_opts[] = { { "debug-override-metadata-version", required_argument, NULL, 0 }, { "omit-clean-shutdown", no_argument, NULL, 1 }, { "help", no_argument, NULL, 'h'}, { "input", required_argument, NULL, 'i' }, { "output", required_argument, NULL, 'o'}, + { "quiet", no_argument, NULL, 'q'}, { "version", no_argument, NULL, 'V'}, { NULL, no_argument, NULL, 0 } }; @@ -111,6 +135,10 @@ int main(int argc, char **argv) fs.output = optional(string(optarg)); break; + case 'q': + fs.quiet = true; + break; + case 'V': cout << THIN_PROVISIONING_TOOLS_VERSION << endl; return 0; diff --git a/caching/xml_format.cc b/caching/xml_format.cc index 57764e7..ced582d 100644 --- a/caching/xml_format.cc +++ b/caching/xml_format.cc @@ -236,13 +236,16 @@ caching::create_xml_emitter(ostream &out) } void -caching::parse_xml(istream &in, emitter::ptr e) +caching::parse_xml(istream &in, emitter::ptr e, + size_t input_length, base::progress_monitor &monitor) { xml_parser p; XML_SetUserData(p.get_parser(), e.get()); XML_SetElementHandler(p.get_parser(), start_tag, end_tag); + size_t total = 0; + while (!in.eof()) { char buffer[4096]; in.read(buffer, sizeof(buffer)); @@ -258,6 +261,9 @@ caching::parse_xml(istream &in, emitter::ptr e) << endl; throw runtime_error(out.str()); } + + total += len; + monitor.update_percent(total * 100 / input_length); } } diff --git a/caching/xml_format.h b/caching/xml_format.h index 6855fb5..1725825 100644 --- a/caching/xml_format.h +++ b/caching/xml_format.h @@ -1,6 +1,7 @@ #ifndef CACHE_XML_FORMAT_H #define CACHE_XML_FORMAT_H +#include "base/progress_monitor.h" #include "emitter.h" #include @@ -9,7 +10,8 @@ namespace caching { emitter::ptr create_xml_emitter(std::ostream &out); - void parse_xml(std::istream &in, emitter::ptr e); + void parse_xml(std::istream &in, emitter::ptr e, + size_t input_len, base::progress_monitor &monitor); } //---------------------------------------------------------------- diff --git a/era/era_restore.cc b/era/era_restore.cc index 3bd37d4..7d11c8c 100644 --- a/era/era_restore.cc +++ b/era/era_restore.cc @@ -21,19 +21,22 @@ using namespace std; namespace { struct flags { + flags() + : quiet(false) { + } + optional input; optional output; + bool quiet; }; - int restore(flags const &fs) { + int restore(flags const &fs, bool quiet) { try { block_manager<>::ptr bm = open_bm(*fs.output, block_manager<>::READ_WRITE); metadata::ptr md(new metadata(bm, metadata::CREATE)); emitter::ptr restorer = create_restore_emitter(*md); - check_file_exists(*fs.input); - ifstream in(fs.input->c_str(), ifstream::in); - parse_xml(in, restorer); + parse_xml(*fs.input, restorer, fs.quiet); } catch (std::exception &e) { cerr << e.what() << endl; @@ -49,6 +52,7 @@ namespace { << " {-h|--help}" << endl << " {-i|--input} " << endl << " {-o|--output} " << endl + << " {-q|--quiet}" << endl << " {-V|--version}" << endl; } } @@ -58,11 +62,12 @@ int main(int argc, char **argv) int c; flags fs; char const *prog_name = basename(argv[0]); - char const *short_opts = "hi:o:V"; + char const *short_opts = "hi:o:qV"; option const long_opts[] = { { "help", no_argument, NULL, 'h'}, { "input", required_argument, NULL, 'i' }, { "output", required_argument, NULL, 'o'}, + { "quiet", no_argument, NULL, 'q'}, { "version", no_argument, NULL, 'V'}, { NULL, no_argument, NULL, 0 } }; @@ -81,6 +86,10 @@ int main(int argc, char **argv) fs.output = optional(string(optarg)); break; + case 'q': + fs.quiet = true; + break; + case 'V': cout << THIN_PROVISIONING_TOOLS_VERSION << endl; return 0; @@ -108,7 +117,7 @@ int main(int argc, char **argv) return 1; } - return restore(fs); + return restore(fs, fs.quiet); } //---------------------------------------------------------------- diff --git a/era/xml_format.cc b/era/xml_format.cc index eeb7903..59aa4a7 100644 --- a/era/xml_format.cc +++ b/era/xml_format.cc @@ -159,29 +159,14 @@ era::create_xml_emitter(std::ostream &out) } void -era::parse_xml(std::istream &in, emitter::ptr e) +era::parse_xml(std::string const &backup_file, emitter::ptr e, bool quiet) { xml_parser p; XML_SetUserData(p.get_parser(), e.get()); XML_SetElementHandler(p.get_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(p.get_parser(), buffer, len, done)) { - ostringstream out; - out << "Parse error at line " - << XML_GetCurrentLineNumber(p.get_parser()) - << ":\n" - << XML_ErrorString(XML_GetErrorCode(p.get_parser())) - << endl; - throw runtime_error(out.str()); - } - } + p.parse(backup_file, quiet); } //---------------------------------------------------------------- diff --git a/era/xml_format.h b/era/xml_format.h index 30b7174..d56220f 100644 --- a/era/xml_format.h +++ b/era/xml_format.h @@ -1,7 +1,8 @@ #ifndef ERA_XML_FORMAT_H #define ERA_XML_FORMAT_H -#include "emitter.h" +#include "base/progress_monitor.h" +#include "era/emitter.h" #include @@ -9,7 +10,7 @@ namespace era { emitter::ptr create_xml_emitter(std::ostream &out); - void parse_xml(std::istream &in, emitter::ptr e); + void parse_xml(std::string const &backup_file, emitter::ptr e, bool quiet); } //---------------------------------------------------------------- diff --git a/features/cache_restore.feature b/features/cache_restore.feature index 1052089..ebe5300 100644 --- a/features/cache_restore.feature +++ b/features/cache_restore.feature @@ -18,6 +18,7 @@ Feature: thin_restore {-h|--help} {-i|--input} {-o|--output} + {-q|--quiet} {-V|--version} {--debug-override-metadata-version} @@ -36,6 +37,7 @@ Feature: thin_restore {-h|--help} {-i|--input} {-o|--output} + {-q|--quiet} {-V|--version} {--debug-override-metadata-version} @@ -80,3 +82,18 @@ Feature: thin_restore And an empty dev file When I run cache_restore with -i metadata.xml -o metadata.bin --omit-clean-shutdown Then it should pass + + Scenario: --quiet is accepted + Given valid metadata + When I run thin_restore with -i metadata.xml -o metadata.bin --quiet + Then it should pass with: + """ + """ + + Scenario: -q is accepted + Given valid metadata + When I run thin_restore with -i metadata.xml -o metadata.bin -q + Then it should pass with: + """ + """ + diff --git a/features/era_restore.feature b/features/era_restore.feature index b62989c..bf7a45c 100644 --- a/features/era_restore.feature +++ b/features/era_restore.feature @@ -18,6 +18,7 @@ Feature: thin_restore {-h|--help} {-i|--input} {-o|--output} + {-q|--quiet} {-V|--version} """ @@ -33,6 +34,7 @@ Feature: thin_restore {-h|--help} {-i|--input} {-o|--output} + {-q|--quiet} {-V|--version} """ @@ -63,4 +65,18 @@ Feature: thin_restore When I run era_restore with -i metadata.xml -o metadata.bin Then it should pass And the metadata should be valid - \ No newline at end of file + + Scenario: --quiet is accepted + Given valid metadata + When I run thin_restore with -i metadata.xml -o metadata.bin --quiet + Then it should pass with: + """ + """ + + Scenario: -q is accepted + Given valid metadata + When I run thin_restore with -i metadata.xml -o metadata.bin -q + Then it should pass with: + """ + """ + diff --git a/features/thin_restore.feature b/features/thin_restore.feature index 7f5b5be..c931649 100644 --- a/features/thin_restore.feature +++ b/features/thin_restore.feature @@ -55,6 +55,20 @@ Feature: thin_restore No output file provided. """ + Scenario: --quiet is accepted + Given valid metadata + When I run thin_restore with -i metadata.xml -o metadata.bin --quiet + Then it should pass with: + """ + """ + + Scenario: -q is accepted + Given valid metadata + When I run thin_restore with -i metadata.xml -o metadata.bin -q + Then it should pass with: + """ + """ + Scenario: dump/restore is a noop Given valid metadata When I dump diff --git a/thin-provisioning/thin_restore.cc b/thin-provisioning/thin_restore.cc index d82eae1..e75cfb6 100644 --- a/thin-provisioning/thin_restore.cc +++ b/thin-provisioning/thin_restore.cc @@ -41,35 +41,13 @@ using namespace thin_provisioning; //---------------------------------------------------------------- namespace { - size_t get_file_length(string const &file) { - struct stat info; - int r; - - r = ::stat(file.c_str(), &info); - if (r) - throw runtime_error("Couldn't stat backup path"); - - return info.st_size; - } - - progress_monitor::ptr create_monitor(bool quiet) { - if (!quiet && isatty(fileno(stdout))) - return create_progress_bar("Restoring"); - else - return create_quiet_progress_monitor(); - } - int restore(string const &backup_file, string const &dev, bool quiet) { try { // 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); - - progress_monitor::ptr monitor = create_monitor(quiet); - parse_xml(in, restorer, get_file_length(backup_file), monitor); + parse_xml(backup_file, restorer, quiet); } catch (std::exception &e) { cerr << e.what() << endl; diff --git a/thin-provisioning/xml_format.cc b/thin-provisioning/xml_format.cc index f300312..333204f 100644 --- a/thin-provisioning/xml_format.cc +++ b/thin-provisioning/xml_format.cc @@ -217,35 +217,14 @@ tp::create_xml_emitter(ostream &out) } void -tp::parse_xml(std::istream &in, emitter::ptr e, - size_t input_length, base::progress_monitor::ptr monitor) +tp::parse_xml(std::string const &backup_file, emitter::ptr e, bool quiet) { xml_parser p; XML_SetUserData(p.get_parser(), e.get()); XML_SetElementHandler(p.get_parser(), start_tag, end_tag); - size_t total = 0; - - while (!in.eof()) { - char buffer[1024 * 1024]; - in.read(buffer, sizeof(buffer)); - size_t len = in.gcount(); - int done = in.eof(); - - if (!XML_Parse(p.get_parser(), buffer, len, done)) { - ostringstream out; - out << "Parse error at line " - << XML_GetCurrentLineNumber(p.get_parser()) - << ":\n" - << XML_ErrorString(XML_GetErrorCode(p.get_parser())) - << endl; - throw runtime_error(out.str()); - } - - total += len; - monitor->update_percent(total * 100 / input_length); - } + p.parse(backup_file, quiet); } //---------------------------------------------------------------- diff --git a/thin-provisioning/xml_format.h b/thin-provisioning/xml_format.h index 7cd7d4e..ae7d77e 100644 --- a/thin-provisioning/xml_format.h +++ b/thin-provisioning/xml_format.h @@ -28,8 +28,7 @@ namespace thin_provisioning { emitter::ptr create_xml_emitter(std::ostream &out); - void parse_xml(std::istream &in, emitter::ptr e, - size_t input_length, base::progress_monitor::ptr p); + void parse_xml(std::string const &backup_file, emitter::ptr e, bool quiet); } //----------------------------------------------------------------