From c6a2620f5de7aeea4e9c20ff4e6d1befb4b2e935 Mon Sep 17 00:00:00 2001 From: Thanos Makatos Date: Wed, 9 Mar 2016 13:26:13 +0300 Subject: [PATCH 1/9] add configure option to enable static linking --- Makefile.in | 4 ++++ configure.ac | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/Makefile.in b/Makefile.in index 30b0c1a..add456e 100644 --- a/Makefile.in +++ b/Makefile.in @@ -119,6 +119,10 @@ else CXXLIB+=-lstdc++ endif +ifeq ("@STATIC@", "yes") +LDFLAGS+=-static +endif + INSTALL:=@INSTALL@ PREFIX:=@prefix@ BINDIR:=$(DESTDIR)$(PREFIX)/sbin diff --git a/configure.ac b/configure.ac index 1161145..0eef6ab 100644 --- a/configure.ac +++ b/configure.ac @@ -145,6 +145,14 @@ AC_ARG_ENABLE(static_cxx, STATIC_CXX=$enableval, STATIC_CXX=no) AC_MSG_RESULT($STATIC_CXX) +################################################################################ +dnl -- Enable static linking. +AC_MSG_CHECKING(whether to statically link) +AC_ARG_ENABLE(static, + AC_HELP_STRING(--enable-static, [enable static link]), + STATIC=$enableval, STATIC=no) +AC_MSG_RESULT($STATIC) + ################################################################################ dnl -- Check for getopt AC_CHECK_HEADERS(getopt.h, AC_DEFINE([HAVE_GETOPTLONG], 1, [Define to 1 if getopt_long is available.])) @@ -173,6 +181,7 @@ AC_SUBST(RELEASE_DATE) AC_SUBST(TESTING) AC_SUBST(THIN_PROVISIONING_TOOLS_VERSION) AC_SUBST(STATIC_CXX) +AC_SUBST(STATIC) ################################################################################ dnl -- First and last lines should not contain files to generate in order to From 3fb41776561c1ecaaa56eae6b93aee6089de26b6 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 8 Apr 2016 17:07:04 +0100 Subject: [PATCH 2/9] [thin_trim] more bug fixing --- thin-provisioning/thin_trim.cc | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/thin-provisioning/thin_trim.cc b/thin-provisioning/thin_trim.cc index 0a8a3b0..2cee686 100644 --- a/thin-provisioning/thin_trim.cc +++ b/thin-provisioning/thin_trim.cc @@ -39,8 +39,6 @@ namespace { range[0] = block_to_byte(b); range[1] = block_to_byte(e) - range[0]; - cerr << "emitting discard for blocks (" << b << ", " << e << "]\n"; - if (ioctl(fd_, BLKDISCARD, &range)) throw runtime_error("discard ioctl failed"); } @@ -99,8 +97,13 @@ namespace { highest_ = b; if (count) { - if (last_referenced_ && (b > *last_referenced_ + 1)) - emitter_.emit(*last_referenced_ + 1, b); + if (last_referenced_) { + if (b > *last_referenced_ + 1) + emitter_.emit(*last_referenced_ + 1, b); + + } else if (b > 0) { + emitter_.emit(0, b); + } last_referenced_ = b; } @@ -109,7 +112,7 @@ namespace { void complete() { if (last_referenced_) { if (*last_referenced_ != *highest_) - emitter_.emit(*last_referenced_, *highest_ + 1ull); + emitter_.emit(*last_referenced_ + 1ull, *highest_ + 1ull); } else if (highest_) emitter_.emit(0ull, *highest_ + 1); @@ -122,17 +125,13 @@ namespace { }; int trim(string const &metadata_dev, string const &data_dev) { - cerr << "in trim\n"; - // We can trim any block that has zero count in the data // space map. block_manager<>::ptr bm = open_bm(metadata_dev, block_manager<>::READ_ONLY); metadata md(bm); - if (!md.data_sm_->get_nr_free()) { - cerr << "All data blocks allocated, nothing to discard\n"; + if (!md.data_sm_->get_nr_free()) return 0; - } discard_emitter de(data_dev, md.sb_.data_block_size_, md.data_sm_->get_nr_blocks()); From 7dbc5d1221044723fd8a652b663fe215ae3e5aad Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Fri, 8 Apr 2016 17:07:40 +0100 Subject: [PATCH 3/9] bump version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 1f969e9..1e685eb 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.6.2-rc7 +0.6.2-rc8 From cd725901756eedada1d13a11fe328e068ca1b83f Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Sat, 7 May 2016 11:56:21 +0100 Subject: [PATCH 4/9] bump version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 1e685eb..b4edc50 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.6.2-rc8 +0.6.2-rc9 From b46676575c807acf1ef7dbfdaad7f2d5bb96c54d Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Sun, 8 May 2016 23:35:19 -0400 Subject: [PATCH 5/9] fix up test targets - PHONY is misspelled - fix the pdata_tools target dep - add a "check" alias to match standard automake behavior - mark test & check targets as phony --- Makefile.in | 5 +++-- unit-tests/Makefile.in | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Makefile.in b/Makefile.in index add456e..b8fbb81 100644 --- a/Makefile.in +++ b/Makefile.in @@ -229,12 +229,13 @@ install: bin/pdata_tools ifeq ("@TESTING@", "yes") include unit-tests/Makefile -.PHONEY: features +.PHONY: features test check -features: pdata_tools +features: bin/pdata_tools cucumber --no-color --format progress test: features unit-test +check: unit-test endif -include $(DEPEND_FILES) diff --git a/unit-tests/Makefile.in b/unit-tests/Makefile.in index 9307e5f..738552b 100644 --- a/unit-tests/Makefile.in +++ b/unit-tests/Makefile.in @@ -84,7 +84,7 @@ unit-tests/unit_tests: $(TEST_OBJECTS) lib/libgmock.a lib/libpdata.a @echo " [LD] $<" $(V)g++ $(CXXFLAGS) $(LDFLAGS) -o $@ $(TEST_OBJECTS) $(LIBS) $(GMOCK_LIBS) $(LIBEXPAT) -.PHONEY: unit-test +.PHONY: unit-test unit-test: unit-tests/unit_tests unit-tests/unit_tests From 96e0e92afd6ae57c55b75b42c4c6156ae5b3f341 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Sun, 8 May 2016 23:47:21 -0400 Subject: [PATCH 6/9] get default CFLAGS/CXXFLAGS from configure Since autoconf already sets up default compiler flags for us, use those. These also come from the user's build settings. --- Makefile.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Makefile.in b/Makefile.in index b8fbb81..2d677d7 100644 --- a/Makefile.in +++ b/Makefile.in @@ -103,9 +103,11 @@ STRIP:=@STRIP@ OBJECTS:=$(subst .cc,.o,$(SOURCE)) TOP_DIR:=@top_srcdir@ TOP_BUILDDIR:=@top_builddir@ -CFLAGS+=-g -Wall -O3 +CFLAGS?=@CFLAGS@ +CFLAGS+=-Wall CFLAGS+=@LFS_FLAGS@ -CXXFLAGS+=-g -Wall -fno-strict-aliasing -std=gnu++98 +CXXFLAGS?=@CXXFLAGS@ +CXXFLAGS+=-Wall -fno-strict-aliasing -std=gnu++98 CXXFLAGS+=@CXXOPTIMISE_FLAG@ CXXFLAGS+=@CXXDEBUG_FLAG@ CXXFLAGS+=@CXX_STRERROR_FLAG@ From 3e24cff8a140a6e7f82a5c01944d93d6dcbf7966 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Sun, 8 May 2016 23:51:22 -0400 Subject: [PATCH 7/9] respect & use CPPFLAGS properly The common preprocessor variable is named CPPFLAGS. configure sets this up for us, including sourcing values from the user. Rename INCLUDES to match, and pull the default from configure. --- Makefile.in | 7 ++++--- unit-tests/Makefile.in | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Makefile.in b/Makefile.in index 2d677d7..14da10f 100644 --- a/Makefile.in +++ b/Makefile.in @@ -112,7 +112,8 @@ CXXFLAGS+=@CXXOPTIMISE_FLAG@ CXXFLAGS+=@CXXDEBUG_FLAG@ CXXFLAGS+=@CXX_STRERROR_FLAG@ CXXFLAGS+=@LFS_FLAGS@ -INCLUDES+=-I$(TOP_BUILDDIR) -I$(TOP_DIR) -I$(TOP_DIR)/thin-provisioning +CPPFLAGS?=@CPPFLAGS@ +CPPFLAGS+=-I$(TOP_BUILDDIR) -I$(TOP_DIR) -I$(TOP_DIR)/thin-provisioning LIBS:=-laio -lexpat ifeq ("@STATIC_CXX@", "yes") @@ -149,9 +150,9 @@ endif %.o: %.cc @echo " [CXX] $<" - $(V) $(CXX) -c $(INCLUDES) $(CXXFLAGS) -o $@ $< + $(V) $(CXX) -c $(CPPFLAGS) $(CXXFLAGS) -o $@ $< @echo " [DEP] $<" - $(V) $(CXX) -MM -MT $(subst .cc,.o,$<) $(INCLUDES) $(TEST_INCLUDES) $(CXXFLAGS) $< > $*.$$$$; \ + $(V) $(CXX) -MM -MT $(subst .cc,.o,$<) $(CPPFLAGS) $(TEST_INCLUDES) $(CXXFLAGS) $< > $*.$$$$; \ sed 's,\([^ :]*\)\.o[ :]*,\1.o \1.gmo $* : Makefile ,g' < $*.$$$$ > $*.d; \ $(RM) $*.$$$$ diff --git a/unit-tests/Makefile.in b/unit-tests/Makefile.in index 738552b..0ff70dd 100644 --- a/unit-tests/Makefile.in +++ b/unit-tests/Makefile.in @@ -74,9 +74,9 @@ TEST_OBJECTS=$(subst .cc,.gmo,$(TEST_SOURCE)) %.gmo: %.cc @echo " [CXX] $<" - $(V) $(CXX) -c $(INCLUDES) $(GMOCK_INCLUDES) $(CXXFLAGS) $(GMOCK_FLAGS) -o $@ $< + $(V) $(CXX) -c $(CPPFLAGS) $(GMOCK_INCLUDES) $(CXXFLAGS) $(GMOCK_FLAGS) -o $@ $< @echo " [DEP] $<" - $(V) $(CXX) -MM -MT $(subst .cc,.o,$<) $(INCLUDES) $(GMOCK_INCLUDES) $(CXXFLAGS) $(GMOCK_FLAGS) $< > $*.$$$$; \ + $(V) $(CXX) -MM -MT $(subst .cc,.o,$<) $(CPPFLAGS) $(GMOCK_INCLUDES) $(CXXFLAGS) $(GMOCK_FLAGS) $< > $*.$$$$; \ sed 's,\([^ :]*\)\.o[ :]*,\1.o \1.gmo $* : Makefile ,g' < $*.$$$$ > $*.d; \ $(RM) $*.$$$$ From ceffa5f5c44198c577b3a3777662fa5f4fad1854 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 11 Jul 2016 11:30:55 +0100 Subject: [PATCH 8/9] Bump version to 0.6.2 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index b4edc50..b616048 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.6.2-rc9 +0.6.2 From 4779fb9b80a4348f680acba039520aaa8beac4c4 Mon Sep 17 00:00:00 2001 From: Joe Thornber Date: Mon, 11 Jul 2016 14:53:03 +0100 Subject: [PATCH 9/9] [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); }