refactor: cleanup read/readv APIs

This commit is contained in:
Marcus Holland-Moritz 2024-07-28 18:53:52 +02:00
parent 30c8ac2566
commit 3ae80f564e
8 changed files with 197 additions and 101 deletions

View File

@ -159,12 +159,32 @@ class filesystem_v2 {
int open(inode_view entry) const { return impl_->open(entry); }
ssize_t
size_t
read(uint32_t inode, char* buf, size_t size, file_off_t offset = 0) const {
return impl_->read(inode, buf, size, offset);
}
ssize_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
size_t read(uint32_t inode, char* buf, size_t size, file_off_t offset,
std::error_code& ec) const {
return impl_->read(inode, buf, size, offset, ec);
}
size_t
read(uint32_t inode, char* buf, size_t size, std::error_code& ec) const {
return impl_->read(inode, buf, size, 0, ec);
}
size_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
file_off_t offset, std::error_code& ec) const {
return impl_->readv(inode, buf, size, offset, ec);
}
size_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
std::error_code& ec) const {
return impl_->readv(inode, buf, size, 0, ec);
}
size_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
file_off_t offset = 0) const {
return impl_->readv(inode, buf, size, offset);
}
@ -257,9 +277,13 @@ class filesystem_v2 {
readlink(inode_view entry, readlink_mode mode) const = 0;
virtual int statvfs(vfs_stat* stbuf) const = 0;
virtual int open(inode_view entry) const = 0;
virtual ssize_t
virtual size_t
read(uint32_t inode, char* buf, size_t size, file_off_t offset) const = 0;
virtual ssize_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
virtual size_t read(uint32_t inode, char* buf, size_t size,
file_off_t offset, std::error_code& ec) const = 0;
virtual size_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
file_off_t offset, std::error_code& ec) const = 0;
virtual size_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
file_off_t offset) const = 0;
virtual std::vector<std::future<block_range>>
readv(uint32_t inode, size_t size, file_off_t offset) const = 0;

View File

@ -25,8 +25,8 @@
#include <iosfwd>
#include <memory>
#include <string>
#include <folly/Expected.h>
#include <system_error>
#include <vector>
#include <dwarfs/block_range.h>
#include <dwarfs/types.h>
@ -54,20 +54,21 @@ class inode_reader_v2 {
inode_reader_v2& operator=(inode_reader_v2&&) = default;
ssize_t read(char* buf, uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks) const {
return impl_->read(buf, inode, size, offset, chunks);
size_t read(char* buf, uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks, std::error_code& ec) const {
return impl_->read(buf, inode, size, offset, chunks, ec);
}
ssize_t readv(iovec_read_buf& buf, uint32_t inode, size_t size,
file_off_t offset, chunk_range chunks) const {
return impl_->readv(buf, inode, size, offset, chunks);
size_t
readv(iovec_read_buf& buf, uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks, std::error_code& ec) const {
return impl_->readv(buf, inode, size, offset, chunks, ec);
}
folly::Expected<std::vector<std::future<block_range>>, int>
readv(uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks) const {
return impl_->readv(inode, size, offset, chunks);
std::vector<std::future<block_range>>
readv(uint32_t inode, size_t size, file_off_t offset, chunk_range chunks,
std::error_code& ec) const {
return impl_->readv(inode, size, offset, chunks, ec);
}
void
@ -87,13 +88,15 @@ class inode_reader_v2 {
public:
virtual ~impl() = default;
virtual ssize_t read(char* buf, uint32_t inode, size_t size,
file_off_t offset, chunk_range chunks) const = 0;
virtual ssize_t readv(iovec_read_buf& buf, uint32_t inode, size_t size,
file_off_t offset, chunk_range chunks) const = 0;
virtual folly::Expected<std::vector<std::future<block_range>>, int>
readv(uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks) const = 0;
virtual size_t
read(char* buf, uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks, std::error_code& ec) const = 0;
virtual size_t
readv(iovec_read_buf& buf, uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks, std::error_code& ec) const = 0;
virtual std::vector<std::future<block_range>>
readv(uint32_t inode, size_t size, file_off_t offset, chunk_range chunks,
std::error_code& ec) const = 0;
virtual void dump(std::ostream& os, const std::string& indent,
chunk_range chunks) const = 0;
virtual void set_num_workers(size_t num) = 0;

View File

@ -432,9 +432,13 @@ class filesystem_ final : public filesystem_v2::impl {
std::string readlink(inode_view entry, readlink_mode mode) const override;
int statvfs(vfs_stat* stbuf) const override;
int open(inode_view entry) const override;
ssize_t read(uint32_t inode, char* buf, size_t size,
size_t read(uint32_t inode, char* buf, size_t size,
file_off_t offset) const override;
ssize_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
size_t read(uint32_t inode, char* buf, size_t size, file_off_t offset,
std::error_code& ec) const override;
size_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
file_off_t offset, std::error_code& ec) const override;
size_t readv(uint32_t inode, iovec_read_buf& buf, size_t size,
file_off_t offset) const override;
std::vector<std::future<block_range>>
readv(uint32_t inode, size_t size, file_off_t offset) const override;
@ -471,6 +475,8 @@ class filesystem_ final : public filesystem_v2::impl {
std::vector<std::future<block_range>>
readv_ec(uint32_t inode, size_t size, file_off_t offset,
std::error_code& ec) const;
size_t readv_ec(uint32_t inode, iovec_read_buf& buf, size_t size,
file_off_t offset, std::error_code& ec) const;
LOG_PROXY_DECL(LoggerPolicy);
os_access const& os_;
@ -499,9 +505,11 @@ class filesystem_ final : public filesystem_v2::impl {
PERFMON_CLS_TIMER_DECL(statvfs)
PERFMON_CLS_TIMER_DECL(open)
PERFMON_CLS_TIMER_DECL(read)
PERFMON_CLS_TIMER_DECL(read_ec)
PERFMON_CLS_TIMER_DECL(readv_iovec)
PERFMON_CLS_TIMER_DECL(readv_iovec_ec)
PERFMON_CLS_TIMER_DECL(readv_future)
PERFMON_CLS_TIMER_DECL(readv_future_throw)
PERFMON_CLS_TIMER_DECL(readv_future_ec)
};
template <typename LoggerPolicy>
@ -580,9 +588,11 @@ filesystem_<LoggerPolicy>::filesystem_(
PERFMON_CLS_TIMER_INIT(statvfs)
PERFMON_CLS_TIMER_INIT(open)
PERFMON_CLS_TIMER_INIT(read)
PERFMON_CLS_TIMER_INIT(read_ec)
PERFMON_CLS_TIMER_INIT(readv_iovec)
PERFMON_CLS_TIMER_INIT(readv_iovec_ec)
PERFMON_CLS_TIMER_INIT(readv_future)
PERFMON_CLS_TIMER_INIT(readv_future_throw) // clang-format on
PERFMON_CLS_TIMER_INIT(readv_future_ec) // clang-format on
{
block_cache cache(lgr, os_, mm_, options.block_cache, perfmon);
filesystem_parser parser(mm_, image_offset_);
@ -1120,23 +1130,55 @@ int filesystem_<LoggerPolicy>::open(inode_view entry) const {
}
template <typename LoggerPolicy>
ssize_t filesystem_<LoggerPolicy>::read(uint32_t inode, char* buf, size_t size,
size_t filesystem_<LoggerPolicy>::read(uint32_t inode, char* buf, size_t size,
file_off_t offset) const {
PERFMON_CLS_SCOPED_SECTION(read)
if (auto chunks = meta_.get_chunks(inode)) {
return ir_.read(buf, inode, size, offset, *chunks);
return call_ec_throw([&](std::error_code& ec) {
return ir_.read(buf, inode, size, offset, *chunks, ec);
});
}
return -EBADF;
throw std::system_error(std::make_error_code(std::errc::bad_file_descriptor));
}
template <typename LoggerPolicy>
ssize_t filesystem_<LoggerPolicy>::readv(uint32_t inode, iovec_read_buf& buf,
size_t
filesystem_<LoggerPolicy>::read(uint32_t inode, char* buf, size_t size,
file_off_t offset, std::error_code& ec) const {
PERFMON_CLS_SCOPED_SECTION(read_ec)
if (auto chunks = meta_.get_chunks(inode)) {
return ir_.read(buf, inode, size, offset, *chunks, ec);
}
ec = std::make_error_code(std::errc::bad_file_descriptor);
return 0;
}
template <typename LoggerPolicy>
size_t filesystem_<LoggerPolicy>::readv_ec(uint32_t inode, iovec_read_buf& buf,
size_t size, file_off_t offset,
std::error_code& ec) const {
if (auto chunks = meta_.get_chunks(inode)) {
return ir_.readv(buf, inode, size, offset, *chunks, ec);
}
ec = std::make_error_code(std::errc::bad_file_descriptor);
return 0;
}
template <typename LoggerPolicy>
size_t filesystem_<LoggerPolicy>::readv(uint32_t inode, iovec_read_buf& buf,
size_t size, file_off_t offset,
std::error_code& ec) const {
PERFMON_CLS_SCOPED_SECTION(readv_iovec_ec)
return readv_ec(inode, buf, size, offset, ec);
}
template <typename LoggerPolicy>
size_t filesystem_<LoggerPolicy>::readv(uint32_t inode, iovec_read_buf& buf,
size_t size, file_off_t offset) const {
PERFMON_CLS_SCOPED_SECTION(readv_iovec)
if (auto chunks = meta_.get_chunks(inode)) {
return ir_.readv(buf, inode, size, offset, *chunks);
}
return -EBADF;
return call_ec_throw([&](std::error_code& ec) {
return readv_ec(inode, buf, size, offset, ec);
});
}
template <typename LoggerPolicy>
@ -1145,12 +1187,7 @@ filesystem_<LoggerPolicy>::readv_ec(uint32_t inode, size_t size,
file_off_t offset,
std::error_code& ec) const {
if (auto chunks = meta_.get_chunks(inode)) {
if (auto ranges = ir_.readv(inode, size, offset, *chunks)) {
ec.clear();
return std::move(ranges).value();
} else {
ec = std::error_code(-ranges.error(), std::system_category());
}
return ir_.readv(inode, size, offset, *chunks, ec);
}
ec = std::make_error_code(std::errc::bad_file_descriptor);
return {};
@ -1160,7 +1197,7 @@ template <typename LoggerPolicy>
std::vector<std::future<block_range>>
filesystem_<LoggerPolicy>::readv(uint32_t inode, size_t size, file_off_t offset,
std::error_code& ec) const {
PERFMON_CLS_SCOPED_SECTION(readv_future)
PERFMON_CLS_SCOPED_SECTION(readv_future_ec)
return readv_ec(inode, size, offset, ec);
}
@ -1168,7 +1205,7 @@ template <typename LoggerPolicy>
std::vector<std::future<block_range>>
filesystem_<LoggerPolicy>::readv(uint32_t inode, size_t size,
file_off_t offset) const {
PERFMON_CLS_SCOPED_SECTION(readv_future_throw)
PERFMON_CLS_SCOPED_SECTION(readv_future)
return call_ec_throw(
[&](std::error_code& ec) { return readv_ec(inode, size, offset, ec); });
}

View File

@ -120,13 +120,14 @@ class inode_reader_ final : public inode_reader_v2::impl {
}
}
ssize_t read(char* buf, uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks) const override;
ssize_t readv(iovec_read_buf& buf, uint32_t inode, size_t size,
file_off_t offset, chunk_range chunks) const override;
folly::Expected<std::vector<std::future<block_range>>, int>
readv(uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks) const override;
size_t read(char* buf, uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks, std::error_code& ec) const override;
size_t
readv(iovec_read_buf& buf, uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks, std::error_code& ec) const override;
std::vector<std::future<block_range>>
readv(uint32_t inode, size_t size, file_off_t offset, chunk_range chunks,
std::error_code& ec) const override;
void dump(std::ostream& os, const std::string& indent,
chunk_range chunks) const override;
void set_num_workers(size_t num) override { cache_.set_num_workers(num); }
@ -143,13 +144,14 @@ class inode_reader_ final : public inode_reader_v2::impl {
using readahead_cache_type = folly::EvictingCacheMap<uint32_t, file_off_t>;
folly::Expected<std::vector<std::future<block_range>>, int>
std::vector<std::future<block_range>>
read_internal(uint32_t inode, size_t size, file_off_t offset,
chunk_range chunks) const;
chunk_range chunks, std::error_code& ec) const;
template <typename StoreFunc>
ssize_t read_internal(uint32_t inode, size_t size, file_off_t read_offset,
chunk_range chunks, const StoreFunc& store) const;
size_t read_internal(uint32_t inode, size_t size, file_off_t read_offset,
chunk_range chunks, std::error_code& ec,
const StoreFunc& store) const;
void do_readahead(uint32_t inode, chunk_range::iterator it,
chunk_range::iterator end, file_off_t read_offset,
@ -225,20 +227,24 @@ void inode_reader_<LoggerPolicy>::do_readahead(uint32_t inode,
}
template <typename LoggerPolicy>
folly::Expected<std::vector<std::future<block_range>>, int>
std::vector<std::future<block_range>>
inode_reader_<LoggerPolicy>::read_internal(uint32_t inode, size_t const size,
file_off_t const read_offset,
chunk_range chunks) const {
chunk_range chunks,
std::error_code& ec) const {
std::vector<std::future<block_range>> ranges;
auto offset = read_offset;
if (offset < 0) {
return folly::makeUnexpected(-EINVAL);
ec = std::make_error_code(std::errc::invalid_argument);
return ranges;
}
// request ranges from block cache
std::vector<std::future<block_range>> ranges;
if (size == 0 || chunks.empty()) {
ec.clear();
return ranges;
}
@ -278,6 +284,7 @@ inode_reader_<LoggerPolicy>::read_internal(uint32_t inode, size_t const size,
if (it == end) {
// offset beyond EOF; TODO: check if this should rather be -EINVAL
ec.clear();
return ranges;
}
@ -290,7 +297,9 @@ inode_reader_<LoggerPolicy>::read_internal(uint32_t inode, size_t const size,
if (copysize == 0) {
LOG_ERROR << "invalid zero-sized chunk";
return folly::makeUnexpected(-EIO);
ec = std::make_error_code(std::errc::invalid_argument);
ranges.clear();
break;
}
if (num_read + copysize > size) {
@ -326,21 +335,22 @@ inode_reader_<LoggerPolicy>::read_internal(uint32_t inode, size_t const size,
template <typename LoggerPolicy>
template <typename StoreFunc>
ssize_t
size_t
inode_reader_<LoggerPolicy>::read_internal(uint32_t inode, size_t size,
file_off_t offset,
chunk_range chunks,
std::error_code& ec,
const StoreFunc& store) const {
auto ranges = read_internal(inode, size, offset, chunks);
auto ranges = read_internal(inode, size, offset, chunks, ec);
if (!ranges) {
return ranges.error();
if (ec) {
return 0;
}
try {
// now fill the buffer
size_t num_read = 0;
for (auto& r : ranges.value()) {
for (auto& r : ranges) {
auto br = r.get();
store(num_read, br);
num_read += br.size();
@ -350,51 +360,56 @@ inode_reader_<LoggerPolicy>::read_internal(uint32_t inode, size_t size,
LOG_ERROR << exception_str(std::current_exception());
}
return -EIO;
ec = std::make_error_code(std::errc::io_error);
return 0;
}
template <typename LoggerPolicy>
folly::Expected<std::vector<std::future<block_range>>, int>
std::vector<std::future<block_range>>
inode_reader_<LoggerPolicy>::readv(uint32_t inode, size_t const size,
file_off_t offset,
chunk_range chunks) const {
file_off_t offset, chunk_range chunks,
std::error_code& ec) const {
PERFMON_CLS_SCOPED_SECTION(readv_future)
PERFMON_SET_CONTEXT(static_cast<uint64_t>(offset), size);
return read_internal(inode, size, offset, chunks);
return read_internal(inode, size, offset, chunks, ec);
}
template <typename LoggerPolicy>
ssize_t
inode_reader_<LoggerPolicy>::read(char* buf, uint32_t inode, size_t size,
file_off_t offset, chunk_range chunks) const {
size_t inode_reader_<LoggerPolicy>::read(char* buf, uint32_t inode, size_t size,
file_off_t offset, chunk_range chunks,
std::error_code& ec) const {
PERFMON_CLS_SCOPED_SECTION(read)
PERFMON_SET_CONTEXT(static_cast<uint64_t>(offset), size);
return read_internal(inode, size, offset, chunks,
return read_internal(inode, size, offset, chunks, ec,
[&](size_t num_read, const block_range& br) {
::memcpy(buf + num_read, br.data(), br.size());
});
}
template <typename LoggerPolicy>
ssize_t inode_reader_<LoggerPolicy>::readv(iovec_read_buf& buf, uint32_t inode,
size_t inode_reader_<LoggerPolicy>::readv(iovec_read_buf& buf, uint32_t inode,
size_t size, file_off_t offset,
chunk_range chunks) const {
chunk_range chunks,
std::error_code& ec) const {
PERFMON_CLS_SCOPED_SECTION(readv_iovec)
PERFMON_SET_CONTEXT(static_cast<uint64_t>(offset), size);
auto rv = read_internal(
inode, size, offset, chunks, [&](size_t, const block_range& br) {
buf.buf.resize(buf.buf.size() + 1);
buf.buf.back().iov_base = const_cast<uint8_t*>(br.data());
buf.buf.back().iov_len = br.size();
auto rv = read_internal(inode, size, offset, chunks, ec,
[&](size_t, const block_range& br) {
auto& iov = buf.buf.emplace_back();
iov.iov_base = const_cast<uint8_t*>(br.data());
iov.iov_len = br.size();
buf.ranges.emplace_back(br);
});
{
std::lock_guard lock(iovec_sizes_mutex_);
iovec_sizes_.addValue(buf.buf.size());
}
return rv;
}

View File

@ -659,14 +659,15 @@ void op_read(fuse_req_t req, fuse_ino_t ino, size_t size, file_off_t off,
return EIO;
}
std::error_code ec;
iovec_read_buf buf;
ssize_t rv = userdata.fs.readv(ino, buf, size, off);
auto num = userdata.fs.readv(ino, buf, size, off, ec);
LOG_DEBUG << "readv(" << ino << ", " << size << ", " << off << ") -> " << rv
<< " [size = " << buf.buf.size() << "]";
LOG_DEBUG << "readv(" << ino << ", " << size << ", " << off << ") -> "
<< num << " [size = " << buf.buf.size() << "]: " << ec.message();
if (rv < 0) {
return -rv;
if (ec) {
return -ec.value();
}
return -fuse_reply_iov(req, buf.buf.empty() ? nullptr : &buf.buf[0],

View File

@ -854,8 +854,10 @@ void check_compat(logger& lgr, filesystem_v2 const& fs,
auto inode = fs.open(*entry);
EXPECT_GE(inode, 0);
std::error_code ec;
std::vector<char> buf(st.size);
auto rv = fs.read(inode, &buf[0], st.size, 0);
auto rv = fs.read(inode, &buf[0], st.size, ec);
EXPECT_FALSE(ec);
EXPECT_EQ(rv, st.size);
EXPECT_EQ(format_sh, std::string(buf.begin(), buf.end()));

View File

@ -259,8 +259,10 @@ void basic_end_to_end_test(std::string const& compressor,
int inode = fs.open(*entry);
EXPECT_GE(inode, 0);
std::error_code ec;
std::vector<char> buf(st.size);
ssize_t rv = fs.read(inode, &buf[0], st.size, 0);
auto rv = fs.read(inode, &buf[0], st.size, ec);
EXPECT_FALSE(ec);
EXPECT_EQ(rv, st.size);
EXPECT_EQ(std::string(buf.begin(), buf.end()), test::loremipsum(st.size));
@ -422,7 +424,6 @@ void basic_end_to_end_test(std::string const& compressor,
EXPECT_TRUE(fs.access(*entry, R_OK, 1000, 100));
entry = fs.find(0, "baz.pl");
ASSERT_TRUE(entry);
std::error_code ec;
fs.access(*entry, R_OK, 1337, 0, ec);
EXPECT_EQ(set_uid ? EACCES : 0, ec.value());
EXPECT_EQ(set_uid, !fs.access(*entry, R_OK, 1337, 0));
@ -799,7 +800,7 @@ TEST_P(compression_regression, github45) {
EXPECT_GE(inode, 0);
std::vector<char> buf(st.size);
ssize_t rv = fs.read(inode, &buf[0], st.size, 0);
auto rv = fs.read(inode, &buf[0], st.size);
EXPECT_EQ(rv, st.size);
EXPECT_EQ(std::string(buf.begin(), buf.end()), contents);
};
@ -1202,10 +1203,13 @@ TEST(section_index_regression, github183) {
EXPECT_NO_THROW(inode = fs.open(*entry));
std::error_code ec;
std::vector<char> buf(st.size);
auto rv = fs.read(inode, &buf[0], st.size, 0);
auto rv = fs.read(inode, &buf[0], st.size, ec);
EXPECT_EQ(rv, -EIO);
EXPECT_TRUE(ec);
EXPECT_EQ(rv, 0);
EXPECT_EQ(ec.value(), EIO);
std::stringstream idss;
EXPECT_THROW(filesystem_v2::identify(lgr, *input, mm, idss, 3),

View File

@ -532,7 +532,7 @@ TEST(dwarfsextract_test, perfmon) {
auto errs = t.err();
EXPECT_GT(outs.size(), 100);
EXPECT_FALSE(errs.empty());
EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.readv_future]"));
EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.readv_future_ec]"));
EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.getattr]"));
EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.open]"));
EXPECT_THAT(errs, ::testing::HasSubstr("[filesystem_v2.readlink_ec]"));
@ -2379,11 +2379,21 @@ TEST(mkdwarfs_test, filesystem_read_error) {
EXPECT_EQ(-1, fs.open(*iv));
{
char buf[1];
EXPECT_EQ(-EBADF, fs.read(iv->inode_num(), buf, sizeof(buf), 0));
std::error_code ec;
auto num = fs.read(iv->inode_num(), buf, sizeof(buf), ec);
EXPECT_TRUE(ec);
EXPECT_EQ(0, num);
EXPECT_EQ(EBADF, ec.value());
EXPECT_THAT([&] { fs.read(iv->inode_num(), buf, sizeof(buf)); },
::testing::Throws<std::system_error>());
}
{
iovec_read_buf buf;
EXPECT_EQ(-EBADF, fs.readv(iv->inode_num(), buf, 42, 0));
std::error_code ec;
auto num = fs.readv(iv->inode_num(), buf, 42, ec);
EXPECT_EQ(0, num);
EXPECT_TRUE(ec);
EXPECT_EQ(EBADF, ec.value());
}
{
std::error_code ec;