From b7d418131d0bbfb97ae15b5e886fae56c521e445 Mon Sep 17 00:00:00 2001 From: Ming-Hung Tsai Date: Sun, 30 Apr 2017 01:51:52 +0800 Subject: [PATCH] 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 | 1 + base/file_utils.cc | 143 ++++++++++++++++++ base/file_utils.h | 21 +++ base/xml_utils.cc | 20 +-- base/xml_utils.h | 1 - caching/cache_restore.cc | 13 +- era/superblock.cc | 3 + persistent-data/block.tcc | 95 +----------- persistent-data/block_counter.h | 2 +- .../data-structures/bloom_filter.cc | 7 +- .../data-structures/bloom_filter.h | 6 +- persistent-data/data-structures/btree.tcc | 1 + persistent-data/file_utils.cc | 55 +------ persistent-data/file_utils.h | 6 +- persistent-data/space-maps/careful_alloc.cc | 2 + persistent-data/space-maps/recursive.cc | 1 + persistent-data/validators.cc | 2 + thin-provisioning/cache_stream.cc | 17 +-- thin-provisioning/superblock.cc | 3 + thin-provisioning/thin_trim.cc | 2 + 20 files changed, 210 insertions(+), 191 deletions(-) create mode 100644 base/file_utils.cc create mode 100644 base/file_utils.h diff --git a/Makefile.in b/Makefile.in index 7e8c144..237939b 100644 --- a/Makefile.in +++ b/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 \ diff --git a/base/file_utils.cc b/base/file_utils.cc new file mode 100644 index 0000000..d35784e --- /dev/null +++ b/base/file_utils.cc @@ -0,0 +1,143 @@ +#include "base/file_utils.h" + +#include "base/error_string.h" + +#include +#include +#include +#include +#include +#include +#include + +//---------------------------------------------------------------- + +// 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(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; +} + +//---------------------------------------------------------------- diff --git a/base/file_utils.h b/base/file_utils.h new file mode 100644 index 0000000..d50c273 --- /dev/null +++ b/base/file_utils.h @@ -0,0 +1,21 @@ +#ifndef BASE_FILE_UTILS_H +#define BASE_FILE_UTILS_H + +#include +#include +#include + +//---------------------------------------------------------------- + +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 diff --git a/base/xml_utils.cc b/base/xml_utils.cc index 8dd897c..970f3a2 100644 --- a/base/xml_utils.cc +++ b/base/xml_utils.cc @@ -1,8 +1,9 @@ #include "xml_utils.h" -#include "persistent-data/file_utils.h" +#include "base/file_utils.h" #include #include +#include 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 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 xml_parser::create_monitor(bool quiet) { diff --git a/base/xml_utils.h b/base/xml_utils.h index fbfdd2c..5f39126 100644 --- a/base/xml_utils.h +++ b/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 create_monitor(bool quiet); XML_Parser parser_; diff --git a/caching/cache_restore.cc b/caching/cache_restore.cc index f62306d..770b0bc 100644 --- a/caching/cache_restore.cc +++ b/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 create_monitor(bool quiet) { if (!quiet && isatty(fileno(stdout))) return create_progress_bar("Restoring"); diff --git a/era/superblock.cc b/era/superblock.cc index f61a542..8ab0bec 100644 --- a/era/superblock.cc +++ b/era/superblock.cc @@ -3,10 +3,13 @@ #include "persistent-data/checksum.h" #include "persistent-data/errors.h" +#include + using namespace base; using namespace era; using namespace superblock_damage; using namespace persistent_data; +using namespace std; //---------------------------------------------------------------- diff --git a/persistent-data/block.tcc b/persistent-data/block.tcc index bf50c02..34b605a 100644 --- a/persistent-data/block.tcc +++ b/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 -#include -#include -#include -#include -#include - #include #include -#include //---------------------------------------------------------------- -// 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 block_manager::read_ref::read_ref(block_cache::block &b) @@ -233,17 +148,17 @@ namespace persistent_data { template int - block_manager::open_or_create_block_file(string const &path, off_t file_size, mode m, bool excl) + block_manager::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"); diff --git a/persistent-data/block_counter.h b/persistent-data/block_counter.h index ea70f93..0b7ba5b 100644 --- a/persistent-data/block_counter.h +++ b/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++; } diff --git a/persistent-data/data-structures/bloom_filter.cc b/persistent-data/data-structures/bloom_filter.cc index 08516e1..62211f9 100644 --- a/persistent-data/data-structures/bloom_filter.cc +++ b/persistent-data/data-structures/bloom_filter.cc @@ -3,6 +3,7 @@ #include using namespace persistent_data; +using namespace std; //---------------------------------------------------------------- @@ -91,7 +92,7 @@ bloom_filter::flush() } void -bloom_filter::fill_probes(block_address b, vector &probes) const +bloom_filter::fill_probes(block_address b, std::vector &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 &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++) diff --git a/persistent-data/data-structures/bloom_filter.h b/persistent-data/data-structures/bloom_filter.h index 6407878..09357e3 100644 --- a/persistent-data/data-structures/bloom_filter.h +++ b/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 &probes) const; + void fill_probes(block_address b, std::vector &probes) const; transaction_manager &tm_; persistent_data::bitset bits_; diff --git a/persistent-data/data-structures/btree.tcc b/persistent-data/data-structures/btree.tcc index ce49a30..35de301 100644 --- a/persistent-data/data-structures/btree.tcc +++ b/persistent-data/data-structures/btree.tcc @@ -24,6 +24,7 @@ #include "persistent-data/validators.h" #include +#include //---------------------------------------------------------------- diff --git a/persistent-data/file_utils.cc b/persistent-data/file_utils.cc index a57e92e..337ac69 100644 --- a/persistent-data/file_utils.cc +++ b/persistent-data/file_utils.cc @@ -5,56 +5,26 @@ #include #include #include +#include +#include #include 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; - - 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(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(nr_blocks, block_size); - } else - // FIXME: needs a better message - throw runtime_error("bad path"); - - return nr_blocks; + return div_down(file_utils::get_file_length(path), + block_size); } 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"); -} - //---------------------------------------------------------------- diff --git a/persistent-data/file_utils.h b/persistent-data/file_utils.h index 1dbfce2..c3449a7 100644 --- a/persistent-data/file_utils.h +++ b/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); - block_address get_nr_metadata_blocks(string const &path); + persistent_data::block_address get_nr_blocks(std::string const &path, sector_t block_size = MD_BLOCK_SIZE); + 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); } //---------------------------------------------------------------- diff --git a/persistent-data/space-maps/careful_alloc.cc b/persistent-data/space-maps/careful_alloc.cc index 5bca0ca..4e8560c 100644 --- a/persistent-data/space-maps/careful_alloc.cc +++ b/persistent-data/space-maps/careful_alloc.cc @@ -21,6 +21,8 @@ #include +using namespace std; + //---------------------------------------------------------------- namespace { diff --git a/persistent-data/space-maps/recursive.cc b/persistent-data/space-maps/recursive.cc index 3d6f000..205f0cd 100644 --- a/persistent-data/space-maps/recursive.cc +++ b/persistent-data/space-maps/recursive.cc @@ -22,6 +22,7 @@ #include using namespace persistent_data; +using namespace std; //---------------------------------------------------------------- diff --git a/persistent-data/validators.cc b/persistent-data/validators.cc index a50947d..eb85abb 100644 --- a/persistent-data/validators.cc +++ b/persistent-data/validators.cc @@ -4,6 +4,8 @@ #include "persistent-data/errors.h" #include "persistent-data/validators.h" +#include + using namespace bcache; using namespace persistent_data; diff --git a/thin-provisioning/cache_stream.cc b/thin-provisioning/cache_stream.cc index a21ed64..7ebcd1c 100644 --- a/thin-provisioning/cache_stream.cc +++ b/thin-provisioning/cache_stream.cc @@ -2,25 +2,14 @@ #include "thin-provisioning/cache_stream.h" #include "persistent-data/file_utils.h" +#include + 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) { diff --git a/thin-provisioning/superblock.cc b/thin-provisioning/superblock.cc index fe6ef06..3e9d609 100644 --- a/thin-provisioning/superblock.cc +++ b/thin-provisioning/superblock.cc @@ -3,8 +3,11 @@ #include "thin-provisioning/superblock.h" #include "persistent-data/file_utils.h" +#include + using namespace thin_provisioning; using namespace superblock_detail; +using namespace std; //---------------------------------------------------------------- diff --git a/thin-provisioning/thin_trim.cc b/thin-provisioning/thin_trim.cc index 0a8a3b0..82498bd 100644 --- a/thin-provisioning/thin_trim.cc +++ b/thin-provisioning/thin_trim.cc @@ -1,8 +1,10 @@ #include #include #include +#include #include #include +#include #undef BLOCK_SIZE