From 4779fb9b80a4348f680acba039520aaa8beac4c4 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 11 Jul 2016 14:53:03 +0100 Subject: [PATCH] [various] Improve documentation Output file must be preallocated. --- Makefile.in | 1 + base/application.cc | 10 ++++- base/output_file_requirements.cc | 51 +++++++++++++++++++++++++ base/output_file_requirements.h | 14 +++++++ caching/cache_repair.cc | 6 ++- caching/cache_restore.cc | 6 ++- era/era_restore.cc | 6 ++- features/step_definitions/thin_steps.rb | 5 +++ features/thin_restore.feature | 12 ++++++ man8/cache_repair.8 | 3 +- man8/cache_restore.8 | 3 +- man8/thin_repair.8 | 3 +- man8/thin_restore.8 | 3 +- persistent-data/file_utils.cc | 7 +++- thin-provisioning/thin_repair.cc | 6 ++- thin-provisioning/thin_restore.cc | 4 +- 16 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 base/output_file_requirements.cc create mode 100644 base/output_file_requirements.h diff --git a/Makefile.in b/Makefile.in index 14da10f..4264191 100644 --- a/Makefile.in +++ b/Makefile.in @@ -25,6 +25,7 @@ PROGRAMS=\ all: $(PROGRAMS) SOURCE=\ + base/output_file_requirements.cc \ base/application.cc \ base/base64.cc \ base/disk_units.cc \ diff --git a/base/application.cc b/base/application.cc index 0ec1749..3c5202c 100644 --- a/base/application.cc +++ b/base/application.cc @@ -61,8 +61,14 @@ application::run(int argc, char **argv) std::list::const_iterator it; for (it = cmds_.begin(); it != cmds_.end(); ++it) { - if (cmd == (*it)->get_name()) - return (*it)->run(argc, argv); + if (cmd == (*it)->get_name()) { + try { + return (*it)->run(argc, argv); + } catch (std::exception const &e) { + cerr << e.what() << "\n"; + return 1; + } + } } std::cerr << "Unknown command '" << cmd << "'\n"; diff --git a/base/output_file_requirements.cc b/base/output_file_requirements.cc new file mode 100644 index 0000000..001e127 --- /dev/null +++ b/base/output_file_requirements.cc @@ -0,0 +1,51 @@ +#include "base/output_file_requirements.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace base; +using namespace std; + +//---------------------------------------------------------------- + +namespace { + void explain_output_file_requirements() { + ostringstream out; + out << "The output file should either be a block device,\n" + << "or an existing file. The file needs to be large\n" + << "enough to hold the metadata."; + + throw runtime_error(out.str()); + } + + unsigned const MIN_SIZE = 32 * 1024; +} + +void +base::check_output_file_requirements(string const &path) +{ + struct stat info; + int r = ::stat(path.c_str(), &info); + if (r) { + cerr << "Output file does not exist.\n\n"; + explain_output_file_requirements(); + } + + if (!info.st_size) { + cerr << "Zero size output file.\n\n"; + explain_output_file_requirements(); + } + + if (info.st_size < MIN_SIZE) { + cerr << "Output file too small.\n\n"; + explain_output_file_requirements(); + } +} + +//---------------------------------------------------------------- diff --git a/base/output_file_requirements.h b/base/output_file_requirements.h new file mode 100644 index 0000000..5d1f85f --- /dev/null +++ b/base/output_file_requirements.h @@ -0,0 +1,14 @@ +#ifndef BASE_OUTPUT_FILE_REQUIREMENTS_H +#define BASE_OUTPUT_FILE_REQUIREMENTS_H + +#include + +//---------------------------------------------------------------- + +namespace base { + void check_output_file_requirements(std::string const &path); +} + +//---------------------------------------------------------------- + +#endif diff --git a/caching/cache_repair.cc b/caching/cache_repair.cc index dfe644b..bfc92c8 100644 --- a/caching/cache_repair.cc +++ b/caching/cache_repair.cc @@ -2,6 +2,7 @@ #include #include +#include "base/output_file_requirements.h" #include "caching/commands.h" #include "caching/metadata.h" #include "caching/metadata_dump.h" @@ -105,7 +106,10 @@ cache_repair_cmd::run(int argc, char **argv) return 1; } - if (!output_path) { + if (output_path) + check_output_file_requirements(*output_path); + + else { cerr << "no output file provided" << endl; usage(cerr); return 1; diff --git a/caching/cache_restore.cc b/caching/cache_restore.cc index a357eed..efa8e98 100644 --- a/caching/cache_restore.cc +++ b/caching/cache_restore.cc @@ -1,5 +1,6 @@ #include "version.h" +#include "base/output_file_requirements.h" #include "caching/commands.h" #include "caching/metadata.h" #include "caching/restore_emitter.h" @@ -169,7 +170,10 @@ cache_restore_cmd::run(int argc, char **argv) return 1; } - if (!fs.output) { + if (fs.output) + check_output_file_requirements(*fs.output); + + else { cerr << "No output file provided." << endl << endl; usage(cerr); return 1; diff --git a/era/era_restore.cc b/era/era_restore.cc index 0f41a96..97dff35 100644 --- a/era/era_restore.cc +++ b/era/era_restore.cc @@ -1,5 +1,6 @@ #include "version.h" +#include "base/output_file_requirements.h" #include "era/commands.h" #include "era/metadata.h" #include "era/restore_emitter.h" @@ -121,7 +122,10 @@ era_restore_cmd::run(int argc, char **argv) return 1; } - if (!fs.output) { + if (fs.output) + check_output_file_requirements(*fs.output); + + else { cerr << "No output file provided." << endl << endl; usage(cerr); return 1; diff --git a/features/step_definitions/thin_steps.rb b/features/step_definitions/thin_steps.rb index 1900293..39527f4 100644 --- a/features/step_definitions/thin_steps.rb +++ b/features/step_definitions/thin_steps.rb @@ -24,6 +24,11 @@ Given(/^a corrupt superblock$/) do end end +Given(/^a tiny file$/) do + run_simple("rm -f tiny") + run_simple("fallocate -l 123 tiny") +end + When(/^I run thin_check with (.*?)$/) do |opts| run_simple("thin_check #{opts} #{dev_file}", false) end diff --git a/features/thin_restore.feature b/features/thin_restore.feature index e5bdb04..1c1a7f4 100644 --- a/features/thin_restore.feature +++ b/features/thin_restore.feature @@ -55,6 +55,18 @@ Feature: thin_restore No output file provided. """ + Scenario: tiny output file + Given a tiny file + When I run thin_restore with -i metadata.xml -o tiny + Then it should fail with: + """ + Output file too small. + + The output file should either be a block device, + or an existing file. The file needs to be large + enough to hold the metadata. + """ + Scenario: --quiet is accepted Given valid thin metadata When I run thin_restore with -i metadata.xml -o metadata.bin --quiet diff --git a/man8/cache_repair.8 b/man8/cache_repair.8 index b702707..f701e77 100644 --- a/man8/cache_repair.8 +++ b/man8/cache_repair.8 @@ -32,7 +32,8 @@ This tool cannot be run on live metadata. Input file or device with binary metadata. .IP "\fB\-o, \-\-output\fP \fI{device|file}\fP" -Output file or device for repaired binary metadata. +Output file or device for repaired binary metadata. If a file is used +then it must be preallocated, and large enough to hold the metadata. .IP "\fB\-h, \-\-help\fP" Print help and exit. diff --git a/man8/cache_restore.8 b/man8/cache_restore.8 index d9fd580..9a1de2f 100644 --- a/man8/cache_restore.8 +++ b/man8/cache_restore.8 @@ -30,7 +30,8 @@ This tool cannot be run on live metadata. Input file or device with metadata. .IP "\fB\-o, \-\-output\fP \fI{device|file}\fP" -Output file or device. +Output file or device for repaired binary metadata. If a file is used +then it must be preallocated, and large enough to hold the metadata. .IP "\fB{\-\-debug-override-metadata-version}\fP \fI\fP" ONLY FOR DEBUGGING PURPOSES: diff --git a/man8/thin_repair.8 b/man8/thin_repair.8 index a8bd529..8e063fc 100644 --- a/man8/thin_repair.8 +++ b/man8/thin_repair.8 @@ -32,7 +32,8 @@ This tool cannot be run on live metadata. Input file or device with binary metadata. .IP "\fB\-o, \-\-output\fP \fI{device|file}\fP" -Output file or device for repaired binary metadata. +Output file or device for repaired binary metadata. If a file is used +then it must be preallocated, and large enough to hold the metadata. .IP "\fB\-h, \-\-help\fP" Print help and exit. diff --git a/man8/thin_restore.8 b/man8/thin_restore.8 index f4d590d..02dd33c 100644 --- a/man8/thin_restore.8 +++ b/man8/thin_restore.8 @@ -33,7 +33,8 @@ Suppress output messages, return only exit code. Input file or device with metadata. .IP "\fB\-o, \-\-output\fP \fI{device|file}\fP" -Output file or device. +Output file or device for repaired binary metadata. If a file is used +then it must be preallocated, and large enough to hold the metadata. .IP "\fB\-h, \-\-help\fP" Print help and exit. diff --git a/persistent-data/file_utils.cc b/persistent-data/file_utils.cc index 4ed190e..4fd513d 100644 --- a/persistent-data/file_utils.cc +++ b/persistent-data/file_utils.cc @@ -22,8 +22,11 @@ persistent_data::get_nr_blocks(string const &path, block_address block_size) block_address nr_blocks; int r = ::stat(path.c_str(), &info); - if (r) - throw runtime_error("Couldn't stat dev path"); + if (r) { + ostringstream out; + out << "Couldn't stat dev path '" << path << "'"; + throw runtime_error(out.str()); + } if (S_ISREG(info.st_mode) && info.st_size) nr_blocks = div_down(info.st_size, block_size); diff --git a/thin-provisioning/thin_repair.cc b/thin-provisioning/thin_repair.cc index 2741b78..716970e 100644 --- a/thin-provisioning/thin_repair.cc +++ b/thin-provisioning/thin_repair.cc @@ -2,6 +2,7 @@ #include #include +#include "base/output_file_requirements.h" #include "persistent-data/file_utils.h" #include "thin-provisioning/commands.h" #include "human_readable_format.h" @@ -99,7 +100,10 @@ thin_repair_cmd::run(int argc, char **argv) return 1; } - if (!output_path) { + if (output_path) + check_output_file_requirements(*output_path); + + else { cerr << "no output file provided" << endl; usage(cerr); return 1; diff --git a/thin-provisioning/thin_restore.cc b/thin-provisioning/thin_restore.cc index b779b94..6c23c04 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 "base/output_file_requirements.h" #include "persistent-data/file_utils.h" #include "thin-provisioning/commands.h" #include "thin-provisioning/emitter.h" @@ -138,7 +139,8 @@ thin_restore_cmd::run(int argc, char **argv) cerr << "No output file provided." << endl << endl; usage(cerr); return 1; - } + } else + check_output_file_requirements(output); return restore(input, output, quiet); }