Merge branch '2016-07-07-improve-thin-repair-error-message' into v0.7-devel

Conflicts:
	Makefile.in
	VERSION
	configure.ac
	persistent-data/file_utils.cc

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 \

base/application.cc

@@ -61,8 +61,14 @@ application::run(int argc, char **argv)
 
 	std::list<command::ptr>::const_iterator it;
 	for (it = cmds_.begin(); it != cmds_.end(); ++it) {
-		if (cmd == (*it)->get_name())
+		if (cmd == (*it)->get_name()) {
-			return (*it)->run(argc, argv);
+			try {
+				return (*it)->run(argc, argv);
+			} catch (std::exception const &e) {
+				cerr << e.what() << "\n";
+				return 1;
+			}
+		}
 	}
 
 	std::cerr << "Unknown command '" << cmd << "'\n";

base/output_file_requirements.cc

@@ -0,0 +1,51 @@
+#include "base/output_file_requirements.h"
+
+#include <iostream>
+#include <linux/fs.h>
+#include <sstream>
+#include <stdexcept>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+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();
+	}
+}
+
+//----------------------------------------------------------------

base/output_file_requirements.h

@@ -0,0 +1,14 @@
+#ifndef BASE_OUTPUT_FILE_REQUIREMENTS_H
+#define BASE_OUTPUT_FILE_REQUIREMENTS_H
+
+#include <string>
+
+//----------------------------------------------------------------
+
+namespace base {
+	void check_output_file_requirements(std::string const &path);
+}
+
+//----------------------------------------------------------------
+
+#endif

caching/cache_repair.cc

@@ -2,6 +2,7 @@
 #include <getopt.h>
 #include <libgen.h>
 
+#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;

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;

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;

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

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

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.

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<integer>\fP"
 ONLY FOR DEBUGGING PURPOSES:

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.

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.

persistent-data/file_utils.cc

@@ -22,9 +22,12 @@ persistent_data::get_nr_blocks(string const &path, sector_t block_size)
 	block_address nr_blocks;
 
 	int r = ::stat(path.c_str(), &info);
-	if (r)
+	if (r) {
-		throw runtime_error("Couldn't stat dev path " + path + ": " +
+		ostringstream out;
-			strerror(errno));
+		out << "Couldn't stat dev path '" << path << "': "
+		    << strerror(errno);
+		throw runtime_error(out.str());
+	}
 
 	if (S_ISREG(info.st_mode) && info.st_size)
 		nr_blocks = div_down<block_address>(info.st_size, block_size);

thin-provisioning/thin_repair.cc

@@ -2,6 +2,7 @@
 #include <getopt.h>
 #include <libgen.h>
 
+#include "base/output_file_requirements.h"
 #include "persistent-data/file_utils.h"
 #include "thin-provisioning/commands.h"
 #include "human_readable_format.h"
@@ -102,7 +103,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;

thin-provisioning/thin_restore.cc

@@ -16,6 +16,7 @@
 // with thin-provisioning-tools.  If not, see
 // <http://www.gnu.org/licenses/>.
 
+#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);
 }

unit-tests/Makefile.in

@@ -78,9 +78,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) $*.$$$$
 
@@ -88,7 +88,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