Spin-off syscall-related file operations (#78)

* [file_utils] spin-off syscall-related file operations

1. Eliminate the potential circular dependency between
   persistent-data/block.h and persistent-data/file_utils.h,
   if the former one wants to include the latter.
2. Avoid namespace pollution by removing the "using namespace std"
   declaration in block.tcc.
3. Correct the header hierarchy: base/xml_utils.h now no longer
   depends on the higher-level persistent-data/file_utils.h

* [file_utils] support block files in get_file_length()

Makefile.in

@@ -33,6 +33,7 @@ SOURCE=\
 	base/error_state.cc \
 	base/error_string.cc \
 	base/grid_layout.cc \
+	base/file_utils.cc \
 	base/progress_monitor.cc \
 	base/rolling_hash.cc \
 	base/xml_utils.cc \

base/file_utils.cc

@@ -0,0 +1,143 @@
+#include "base/file_utils.h"
+
+#include "base/error_string.h"
+
+#include <sstream>
+#include <stdexcept>
+#include <linux/fs.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+//----------------------------------------------------------------
+
+// FIXME: give this namespace a name
+namespace {
+	using namespace std;
+
+	int const DEFAULT_MODE = 0666;
+	int const OPEN_FLAGS = O_DIRECT;
+
+	// FIXME: introduce a new exception for this, or at least lift this
+	// to exception.h
+	void syscall_failed(char const *call) {
+		ostringstream out;
+		out << "syscall '" << call << "' failed: " << base::error_string(errno);
+		throw runtime_error(out.str());
+	}
+
+	void syscall_failed(string const &call, string const &message)
+	{
+		ostringstream out;
+		out << "syscall '" << call << "' failed: " << base::error_string(errno) << "\n"
+		    << message;
+		throw runtime_error(out.str());
+	}
+}
+
+//----------------------------------------------------------------
+
+int
+file_utils::open_file(string const &path, int flags) {
+	int fd = ::open(path.c_str(), OPEN_FLAGS | flags, DEFAULT_MODE);
+	if (fd < 0)
+		syscall_failed("open",
+			       "Note: you cannot run this tool with these options on live metadata.");
+
+	return fd;
+}
+
+bool
+file_utils::file_exists(string const &path) {
+	struct ::stat info;
+
+	int r = ::stat(path.c_str(), &info);
+	if (r) {
+		if (errno == ENOENT)
+			return false;
+
+		syscall_failed("stat");
+		return false; // never get here
+
+	} else
+		return S_ISREG(info.st_mode) || S_ISBLK(info.st_mode);
+}
+
+void
+file_utils::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
+file_utils::create_block_file(string const &path, off_t file_size) {
+	if (file_exists(path)) {
+		ostringstream out;
+		out << __FUNCTION__ << ": file '" << path << "' already exists";
+		throw runtime_error(out.str());
+	}
+
+	int fd = open_file(path, O_CREAT | O_EXCL | O_RDWR);
+
+	int r = ::ftruncate(fd, file_size);
+	if (r < 0)
+		syscall_failed("ftruncate");
+
+	return fd;
+}
+
+int
+file_utils::open_block_file(string const &path, off_t min_size, bool writeable, bool excl) {
+	if (!file_exists(path)) {
+		ostringstream out;
+		out << __FUNCTION__ << ": file '" << path << "' doesn't exist";
+		throw runtime_error(out.str());
+	}
+
+	int flags = writeable ? O_RDWR : O_RDONLY;
+	if (excl)
+		flags |= O_EXCL;
+
+	return open_file(path, flags);
+}
+
+uint64_t
+file_utils::get_file_length(string const &file) {
+	struct stat info;
+	uint64_t nr_bytes;
+
+	int r = ::stat(file.c_str(), &info);
+	if (r)
+		throw runtime_error("Couldn't stat path");
+
+	if (S_ISREG(info.st_mode))
+		// It's okay to cast st_size to a uint64_t value.
+		// If LFS is enabled, st_size should not be negative for regular files.
+		nr_bytes = static_cast<uint64_t>(info.st_size);
+	else if (S_ISBLK(info.st_mode)) {
+		// To get the size of a block device we need to
+		// open it, and then make an ioctl call.
+		int fd = ::open(file.c_str(), O_RDONLY);
+		if (fd < 0)
+			throw runtime_error("couldn't open block device to ascertain size");
+
+		r = ::ioctl(fd, BLKGETSIZE64, &nr_bytes);
+		if (r) {
+			::close(fd);
+			throw runtime_error("ioctl BLKGETSIZE64 failed");
+		}
+		::close(fd);
+	} else
+		// FIXME: needs a better message
+		throw runtime_error("bad path");
+
+	return nr_bytes;
+}
+
+//----------------------------------------------------------------

base/file_utils.h

@@ -0,0 +1,21 @@
+#ifndef BASE_FILE_UTILS_H
+#define BASE_FILE_UTILS_H
+
+#include <string>
+#include <sys/types.h>
+#include <stdint.h>
+
+//----------------------------------------------------------------
+
+namespace file_utils {
+	int open_file(std::string const &path, int flags);
+	bool file_exists(std::string const &path);
+	void check_file_exists(std::string const &file);
+	int create_block_file(std::string const &path, off_t file_size);
+	int open_block_file(std::string const &path, off_t min_size, bool writeable, bool excl = true);
+	uint64_t get_file_length(std::string const &file);
+}
+
+//----------------------------------------------------------------
+
+#endif

base/xml_utils.cc

@@ -1,8 +1,9 @@
 #include "xml_utils.h"
 
-#include "persistent-data/file_utils.h"
+#include "base/file_utils.h"
 #include <fstream>
 #include <iostream>
+#include <sys/stat.h>
 
 using namespace xml_utils;
 
@@ -11,13 +12,13 @@ using namespace xml_utils;
 void
 xml_parser::parse(std::string const &backup_file, bool quiet)
 {
-	persistent_data::check_file_exists(backup_file);
+	file_utils::check_file_exists(backup_file);
 	ifstream in(backup_file.c_str(), ifstream::in);
 
 	std::unique_ptr<base::progress_monitor> monitor = create_monitor(quiet);
 
 	size_t total = 0;
-	size_t input_length = get_file_length(backup_file);
+	size_t input_length = file_utils::get_file_length(backup_file);
 
 	XML_Error error_code = XML_ERROR_NONE;
 	while (!in.eof() && error_code == XML_ERROR_NONE) {
@@ -43,19 +44,6 @@ xml_parser::parse(std::string const &backup_file, bool quiet)
 	}
 }
 
-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;
-}
-
 unique_ptr<base::progress_monitor>
 xml_parser::create_monitor(bool quiet)
 {

base/xml_utils.h

@@ -36,7 +36,6 @@ namespace xml_utils {
 		void parse(std::string const &backup_file, bool quiet);
 
 	private:
-		size_t get_file_length(string const &file) const;
 		unique_ptr<base::progress_monitor> create_monitor(bool quiet);
 
 		XML_Parser parser_;

caching/cache_restore.cc

@@ -1,5 +1,6 @@
 #include "version.h"
 
+#include "base/file_utils.h"
 #include "base/output_file_requirements.h"
 #include "caching/commands.h"
 #include "caching/metadata.h"
@@ -17,22 +18,12 @@
 using namespace boost;
 using namespace caching;
 using namespace persistent_data;
+using namespace file_utils;
 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;
-	}
-
 	unique_ptr<progress_monitor> create_monitor(bool quiet) {
 		if (!quiet && isatty(fileno(stdout)))
 			return create_progress_bar("Restoring");

era/superblock.cc

@@ -3,10 +3,13 @@
 #include "persistent-data/checksum.h"
 #include "persistent-data/errors.h"
 
+#include <sstream>
+
 using namespace base;
 using namespace era;
 using namespace superblock_damage;
 using namespace persistent_data;
+using namespace std;
 
 //----------------------------------------------------------------
 

persistent-data/block.tcc

@@ -19,99 +19,14 @@
 #include "block.h"
 
 #include "base/error_string.h"
+#include "base/file_utils.h"
 #include "block-cache/io_engine.h"
 
-#include <errno.h>
-#include <fcntl.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <unistd.h>
-#include <fcntl.h>
-
 #include <boost/bind.hpp>
 #include <stdexcept>
-#include <sstream>
 
 //----------------------------------------------------------------
 
-// FIXME: give this namesace a name
-namespace {
-	using namespace std;
-
-	int const DEFAULT_MODE = 0666;
-	int const OPEN_FLAGS = O_DIRECT;
-
-	// FIXME: introduce a new exception for this, or at least lift this
-	// to exception.h
-	void syscall_failed(char const *call) {
-		ostringstream out;
-		out << "syscall '" << call << "' failed: " << base::error_string(errno);
-		throw runtime_error(out.str());
-	}
-
-	void syscall_failed(string const &call, string const &message)
-	{
-		ostringstream out;
-		out << "syscall '" << call << "' failed: " << base::error_string(errno) << "\n"
-		    << message;
-		throw runtime_error(out.str());
-	}
-
-	int open_file(string const &path, int flags) {
-		int fd = ::open(path.c_str(), OPEN_FLAGS | flags, DEFAULT_MODE);
-		if (fd < 0)
-			syscall_failed("open",
-				 "Note: you cannot run this tool with these options on live metadata.");
-
-		return fd;
-	}
-
-	bool file_exists(string const &path) {
-		struct ::stat info;
-
-		int r = ::stat(path.c_str(), &info);
-		if (r) {
-			if (errno == ENOENT)
-				return false;
-
-			syscall_failed("stat");
-			return false; // never get here
-
-		} else
-			return S_ISREG(info.st_mode) || S_ISBLK(info.st_mode);
-	}
-
-	int create_block_file(string const &path, off_t file_size) {
-		if (file_exists(path)) {
-			ostringstream out;
-			out << __FUNCTION__ << ": file '" << path << "' already exists";
-			throw runtime_error(out.str());
-		}
-
-		int fd = open_file(path, O_CREAT | O_EXCL | O_RDWR);
-
-		int r = ::ftruncate(fd, file_size);
-		if (r < 0)
-			syscall_failed("ftruncate");
-
-		return fd;
-	}
-
-	int open_block_file(string const &path, off_t min_size, bool writeable, bool excl = true) {
-		if (!file_exists(path)) {
-			ostringstream out;
-			out << __FUNCTION__ << ": file '" << path << "' doesn't exist";
-			throw runtime_error(out.str());
-		}
-
-		int flags = writeable ? O_RDWR : O_RDONLY;
-		if (excl)
-			flags |= O_EXCL;
-
-		return open_file(path, flags);
-	}
-};
-
 namespace persistent_data {
 	template <uint32_t BlockSize>
 	block_manager<BlockSize>::read_ref::read_ref(block_cache::block &b)
@@ -233,17 +148,17 @@ namespace persistent_data {
 
 	template <uint32_t BlockSize>
 	int
-	block_manager<BlockSize>::open_or_create_block_file(string const &path, off_t file_size, mode m, bool excl)
+	block_manager<BlockSize>::open_or_create_block_file(std::string const &path, off_t file_size, mode m, bool excl)
 	{
 		switch (m) {
 		case READ_ONLY:
-			return open_block_file(path, file_size, false, excl);
+			return file_utils::open_block_file(path, file_size, false, excl);
 
 		case READ_WRITE:
-			return open_block_file(path, file_size, true, excl);
+			return file_utils::open_block_file(path, file_size, true, excl);
 
 		case CREATE:
-			return create_block_file(path, file_size);
+			return file_utils::create_block_file(path, file_size);
 
 		default:
 			throw std::runtime_error("unsupported mode");

persistent-data/block_counter.h

@@ -38,7 +38,7 @@ namespace persistent_data {
 		virtual void inc(block_address b) {
 			count_map::iterator it = counts_.find(b);
 			if (it == counts_.end())
-				counts_.insert(make_pair(b, 1));
+				counts_.insert(std::make_pair(b, 1));
 			else
 				it->second++;
 		}

persistent-data/data-structures/bloom_filter.cc

@@ -3,6 +3,7 @@
 #include <stdexcept>
 
 using namespace persistent_data;
+using namespace std;
 
 //----------------------------------------------------------------
 
@@ -91,7 +92,7 @@ bloom_filter::flush()
 }
 
 void
-bloom_filter::fill_probes(block_address b, vector<unsigned> &probes) const
+bloom_filter::fill_probes(block_address b, std::vector<unsigned> &probes) const
 {
 	uint32_t h1 = hash1(b) & mask_;
 	uint32_t h2 = hash2(b) & mask_;
@@ -105,7 +106,7 @@ bloom_filter::fill_probes(block_address b, vector<unsigned> &probes) const
 }
 
 void
-bloom_filter::print_debug(ostream &out)
+bloom_filter::print_debug(std::ostream &out)
 {
 	print_residency(out);
 
@@ -133,7 +134,7 @@ bloom_filter::print_debug(ostream &out)
 }
 
 void
-bloom_filter::print_residency(ostream &out)
+bloom_filter::print_residency(std::ostream &out)
 {
 	unsigned count = 0;
 	for (unsigned i = 0; i < bits_.get_nr_bits(); i++)

persistent-data/data-structures/bloom_filter.h

@@ -26,12 +26,12 @@ namespace persistent_data {
 		void set(uint64_t b);
 		void flush();
 
-		void print_debug(ostream &out);
+		void print_debug(std::ostream &out);
 
 	private:
-		void print_residency(ostream &out);
+		void print_residency(std::ostream &out);
 
-		void fill_probes(block_address b, vector<unsigned> &probes) const;
+		void fill_probes(block_address b, std::vector<unsigned> &probes) const;
 
 		transaction_manager &tm_;
 		persistent_data::bitset bits_;

persistent-data/data-structures/btree.tcc

@@ -24,6 +24,7 @@
 #include "persistent-data/validators.h"
 
 #include <iostream>
+#include <sstream>
 
 //----------------------------------------------------------------
 

persistent-data/file_utils.cc

@@ -5,56 +5,26 @@
 #include <sys/ioctl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <fcntl.h>
+#include <sstream>
 #include <unistd.h>
 
 using namespace base;
 using namespace bcache;
 using namespace persistent_data;
+using namespace std;
 
 //----------------------------------------------------------------
 
 persistent_data::block_address
-persistent_data::get_nr_blocks(string const &path, sector_t block_size)
+persistent_data::get_nr_blocks(std::string const &path, sector_t block_size)
 {
-	using namespace persistent_data;
+	return div_down<block_address>(file_utils::get_file_length(path),
-
+				       block_size);
-	struct stat info;
-	block_address nr_blocks;
-
-	int r = ::stat(path.c_str(), &info);
-	if (r) {
-		ostringstream out;
-		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);
-
-	else if (S_ISBLK(info.st_mode)) {
-		// To get the size of a block device we need to
-		// open it, and then make an ioctl call.
-		int fd = ::open(path.c_str(), O_RDONLY);
-		if (fd < 0)
-			throw runtime_error("couldn't open block device to ascertain size");
-
-		r = ::ioctl(fd, BLKGETSIZE64, &nr_blocks);
-		if (r) {
-			::close(fd);
-			throw runtime_error("ioctl BLKGETSIZE64 failed");
-		}
-		::close(fd);
-		nr_blocks = div_down<block_address>(nr_blocks, block_size);
-	} else
-		// FIXME: needs a better message
-		throw runtime_error("bad path");
-
-	return nr_blocks;
 }
 
 block_address
-persistent_data::get_nr_metadata_blocks(string const &path)
+persistent_data::get_nr_metadata_blocks(std::string const &path)
 {
 	return get_nr_blocks(path, MD_BLOCK_SIZE);
 }
@@ -66,15 +36,4 @@ persistent_data::open_bm(std::string const &dev_path, block_manager<>::mode m, b
 	return block_manager<>::ptr(new block_manager<>(dev_path, nr_blocks, 1, m, excl));
 }
 
-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");
-}
-
 //----------------------------------------------------------------

persistent-data/file_utils.h

@@ -9,13 +9,11 @@
 
 // FIXME: move to a different unit
 namespace persistent_data {
-	persistent_data::block_address get_nr_blocks(string const &path, sector_t block_size = MD_BLOCK_SIZE);
+	persistent_data::block_address get_nr_blocks(std::string const &path, sector_t block_size = MD_BLOCK_SIZE);
-	block_address get_nr_metadata_blocks(string const &path);
+	block_address get_nr_metadata_blocks(std::string const &path);
 
 	block_manager<>::ptr open_bm(std::string const &dev_path,
 				     block_manager<>::mode m, bool excl = true);
-
-	void check_file_exists(std::string const &file);
 }
 
 //----------------------------------------------------------------

persistent-data/space-maps/careful_alloc.cc

@@ -21,6 +21,8 @@
 
 #include <boost/shared_ptr.hpp>
 
+using namespace std;
+
 //----------------------------------------------------------------
 
 namespace {

persistent-data/space-maps/recursive.cc

@@ -22,6 +22,7 @@
 #include <list>
 
 using namespace persistent_data;
+using namespace std;
 
 //----------------------------------------------------------------
 

persistent-data/validators.cc

@@ -4,6 +4,8 @@
 #include "persistent-data/errors.h"
 #include "persistent-data/validators.h"
 
+#include <sstream>
+
 using namespace bcache;
 using namespace persistent_data;
 

thin-provisioning/cache_stream.cc

@@ -2,25 +2,14 @@
 #include "thin-provisioning/cache_stream.h"
 #include "persistent-data/file_utils.h"
 
+#include <fcntl.h>
+
 using namespace thin_provisioning;
 using namespace std;
 using namespace persistent_data;
 
 //----------------------------------------------------------------
 
-namespace {
-	int open_file(string const &path) {
-		int fd = ::open(path.c_str(), O_RDONLY | O_DIRECT | O_EXCL, 0666);
-		if (fd < 0)
-			syscall_failed("open",
-				       "Note: you cannot run this tool with these options on live metadata.");
-
-		return fd;
-	}
-}
-
-//----------------------------------------------------------------
-
 cache_stream::cache_stream(string const &path,
 			   block_address block_size,
 			   size_t cache_mem)
@@ -29,7 +18,7 @@ cache_stream::cache_stream(string const &path,
 
 	  // hack because cache uses LRU rather than MRU
 	  cache_blocks_((cache_mem / block_size) / 2u),
-	  fd_(open_file(path)),
+	  fd_(file_utils::open_file(path, O_RDONLY | O_EXCL)),
 	  v_(new bcache::noop_validator()),
 	  cache_(new block_cache(fd_, block_size / 512, nr_blocks_, cache_mem)),
 	  current_index_(0) {

thin-provisioning/superblock.cc

@@ -3,8 +3,11 @@
 #include "thin-provisioning/superblock.h"
 #include "persistent-data/file_utils.h"
 
+#include <sstream>
+
 using namespace thin_provisioning;
 using namespace superblock_detail;
+using namespace std;
 
 //----------------------------------------------------------------
 

thin-provisioning/thin_trim.cc

@@ -1,8 +1,10 @@
 #include <iostream>
 #include <getopt.h>
 #include <sys/ioctl.h>
+#include <sys/stat.h>
 #include <linux/fs.h>
 #include <libgen.h>
+#include <fcntl.h>
 
 #undef BLOCK_SIZE