From 9c9d943e3a3773335ab38c20c778adf23dc886f5 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Sun, 28 Jun 2015 20:26:17 +0300 Subject: [PATCH 1/5] file: add close() method Closing a file can both expose latent errors that did not have the opportunity to be reported earlier, and also may block. While both are unlikely in the case of O_DIRECT files, better not to risk it. --- core/file.hh | 14 ++++++++++++++ core/reactor.cc | 10 ++++++++++ 2 files changed, 24 insertions(+) diff --git a/core/file.hh b/core/file.hh index 991b47ef4c..519db7c5a9 100644 --- a/core/file.hh +++ b/core/file.hh @@ -64,6 +64,7 @@ public: virtual future<> discard(uint64_t offset, uint64_t length) = 0; virtual future<> allocate(uint64_t position, uint64_t length) = 0; virtual future size(void) = 0; + virtual future<> close() = 0; virtual subscription list_directory(std::function (directory_entry de)> next) = 0; friend class reactor; @@ -89,6 +90,7 @@ public: future<> discard(uint64_t offset, uint64_t length); virtual future<> allocate(uint64_t position, uint64_t length) override; future size(void); + virtual future<> close() override; virtual subscription list_directory(std::function (directory_entry de)> next) override; }; @@ -254,6 +256,18 @@ public: return _file_impl->size(); } + /// Closes the file. + /// + /// Flushes any pending operations and release any resources associated with + /// the file (except for stable storage). + /// + /// \note + /// to ensure file data reaches stable storage, you must call \ref flush() + /// before calling \c close(). + future<> close() { + return _file_impl->close(); + } + subscription list_directory(std::function (directory_entry de)> next) { return _file_impl->list_directory(std::move(next)); } diff --git a/core/reactor.cc b/core/reactor.cc index e8cfc25076..03cc3f88d8 100644 --- a/core/reactor.cc +++ b/core/reactor.cc @@ -696,6 +696,16 @@ posix_file_impl::size(void) { }); } +future<> +posix_file_impl::close() { + return engine()._thread_pool.submit>([fd = _fd] { + return wrap_syscall(::close(fd)); + }).then([this] (syscall_result sr) { + _fd = -1; + sr.throw_if_error(); + }); +} + future blockdev_file_impl::size(void) { return engine()._thread_pool.submit>([this] { From c05c7c09cf6c4634eb4b69b0d6af46ed4805a6c8 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Sun, 28 Jun 2015 20:36:23 +0300 Subject: [PATCH 2/5] fstream: close file on stream close --- core/fstream.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/fstream.cc b/core/fstream.cc index 729dfa77f9..8957cb07a8 100644 --- a/core/fstream.cc +++ b/core/fstream.cc @@ -128,7 +128,11 @@ public: }); }); } - future<> close() { return _file->flush(); } + future<> close() { + return _file->flush().then([this] { + return _file->close(); + }); + } }; class file_data_sink : public data_sink { From 803954480621b99402e65cec46dce84fffa2996c Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Sun, 28 Jun 2015 20:39:06 +0300 Subject: [PATCH 3/5] file: warn if file::close() was not called in time --- core/file.hh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/file.hh b/core/file.hh index 519db7c5a9..5ebcee01df 100644 --- a/core/file.hh +++ b/core/file.hh @@ -76,6 +76,11 @@ public: posix_file_impl(int fd) : _fd(fd) {} ~posix_file_impl() { if (_fd != -1) { + if (std::uncaught_exception()) { + std::cerr << "WARNING: closing file in reactor thread during exception recovery\n"; + } else { + std::cerr << "WARNING: closing file in reactor thread\n"; + } ::close(_fd); } } From 7e66cc02e1497d8352507182f76c2c713cb1f81d Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Sun, 28 Jun 2015 20:40:37 +0300 Subject: [PATCH 4/5] tests: close file in fileiotest --- tests/fileiotest.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/fileiotest.cc b/tests/fileiotest.cc index 08b2ecfd2a..64638867f0 100644 --- a/tests/fileiotest.cc +++ b/tests/fileiotest.cc @@ -59,6 +59,8 @@ SEASTAR_TEST_CASE(test1) { } return ft->sem.wait(max).then([ft] () mutable { return ft->f.flush(); + }).then([ft] { + return ft->f.close(); }).then([ft] () mutable { std::cout << "done\n"; delete ft; From ae38e95bb2fd9f2388a6c1af91190cfb6bf46d45 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Sun, 28 Jun 2015 20:42:30 +0300 Subject: [PATCH 5/5] file: uninline posix_file_impl destructor --- core/file.hh | 12 +----------- core/reactor.cc | 11 +++++++++++ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/core/file.hh b/core/file.hh index 5ebcee01df..fca85dee4c 100644 --- a/core/file.hh +++ b/core/file.hh @@ -74,17 +74,7 @@ class posix_file_impl : public file_impl { public: int _fd; posix_file_impl(int fd) : _fd(fd) {} - ~posix_file_impl() { - if (_fd != -1) { - if (std::uncaught_exception()) { - std::cerr << "WARNING: closing file in reactor thread during exception recovery\n"; - } else { - std::cerr << "WARNING: closing file in reactor thread\n"; - } - ::close(_fd); - } - } - + virtual ~posix_file_impl() override; future write_dma(uint64_t pos, const void* buffer, size_t len); future write_dma(uint64_t pos, std::vector iov); future read_dma(uint64_t pos, void* buffer, size_t len); diff --git a/core/reactor.cc b/core/reactor.cc index 03cc3f88d8..afe55311c5 100644 --- a/core/reactor.cc +++ b/core/reactor.cc @@ -471,6 +471,17 @@ bool reactor::process_io() return n; } +posix_file_impl::~posix_file_impl() { + if (_fd != -1) { + if (std::uncaught_exception()) { + std::cerr << "WARNING: closing file in reactor thread during exception recovery\n"; + } else { + std::cerr << "WARNING: closing file in reactor thread\n"; + } + ::close(_fd); + } +} + future posix_file_impl::write_dma(uint64_t pos, const void* buffer, size_t len) { return engine().submit_io_write([this, pos, buffer, len] (iocb& io) {