From 400975b09ec4cc46b86cb2216727324e869a3a9b Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 11 Mar 2016 11:02:58 -0500 Subject: [PATCH 1/2] os/filestore: fix punch hole usage in _zero If we punch a hole that extends past EOF, ObjectStore semantics are that the file size is also extended. Do that. Note that this bug was hidden before because we weren't passing KEEP_SIZE to fallocate until 7bd95b595fddb8a4e618a2c7df1ba04eccf0829d and the fallocate was *always* failing with EOPNOTSUPP (making us fall back to writing actual zeros). Signed-off-by: Sage Weil --- src/os/filestore/FileStore.cc | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc index d6d97728de4..fea8618b851 100644 --- a/src/os/filestore/FileStore.cc +++ b/src/os/filestore/FileStore.cc @@ -3249,10 +3249,29 @@ int FileStore::_zero(const coll_t& cid, const ghobject_t& oid, uint64_t offset, goto out; } + struct stat st; + ret = ::fstat(**fd, &st); + if (ret < 0) { + ret = -errno; + lfn_close(fd); + goto out; + } + // first try fallocate ret = fallocate(**fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE, offset, len); - if (ret < 0) + if (ret < 0) { ret = -errno; + } else { + // ensure we extent file size, if needed + if (offset + len > st.st_size) { + ret = ::ftruncate(**fd, offset + len); + if (ret < 0) { + ret = -errno; + lfn_close(fd); + goto out; + } + } + } lfn_close(fd); if (ret >= 0 && m_filestore_sloppy_crc) { From 55a66975ef7e8e354b554a5682d36c980aa883ac Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 11 Mar 2016 11:13:02 -0500 Subject: [PATCH 2/2] os/filestore: add filestore_punch_hole = false option Make punch hole usage optional. Default to off, since it's relatively untested. Signed-off-by: Sage Weil --- src/common/config_opts.h | 1 + src/os/filestore/FileStore.cc | 75 ++++++++++++++++++----------------- 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/src/common/config_opts.h b/src/common/config_opts.h index 79837bec6f6..a6f7428f226 100644 --- a/src/common/config_opts.h +++ b/src/common/config_opts.h @@ -1016,6 +1016,7 @@ OPTION(filestore_btrfs_clone_range, OPT_BOOL, true) OPTION(filestore_zfs_snap, OPT_BOOL, false) // zfsonlinux is still unstable OPTION(filestore_fsync_flushes_journal_data, OPT_BOOL, false) OPTION(filestore_fiemap, OPT_BOOL, false) // (try to) use fiemap +OPTION(filestore_punch_hole, OPT_BOOL, false) OPTION(filestore_seek_data_hole, OPT_BOOL, false) // (try to) use seek_data/hole OPTION(filestore_fadvise, OPT_BOOL, true) //collect device partition information for management application to use diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc index fea8618b851..26d56c9713d 100644 --- a/src/os/filestore/FileStore.cc +++ b/src/os/filestore/FileStore.cc @@ -3239,57 +3239,60 @@ int FileStore::_zero(const coll_t& cid, const ghobject_t& oid, uint64_t offset, dout(15) << "zero " << cid << "/" << oid << " " << offset << "~" << len << dendl; int ret = 0; + if (g_conf->filestore_punch_hole) { #ifdef CEPH_HAVE_FALLOCATE # if !defined(DARWIN) && !defined(__FreeBSD__) # ifdef FALLOC_FL_KEEP_SIZE - // first try to punch a hole. - FDRef fd; - ret = lfn_open(cid, oid, false, &fd); - if (ret < 0) { - goto out; - } + // first try to punch a hole. + FDRef fd; + ret = lfn_open(cid, oid, false, &fd); + if (ret < 0) { + goto out; + } - struct stat st; - ret = ::fstat(**fd, &st); - if (ret < 0) { - ret = -errno; - lfn_close(fd); - goto out; - } + struct stat st; + ret = ::fstat(**fd, &st); + if (ret < 0) { + ret = -errno; + lfn_close(fd); + goto out; + } - // first try fallocate - ret = fallocate(**fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE, offset, len); - if (ret < 0) { - ret = -errno; - } else { - // ensure we extent file size, if needed - if (offset + len > st.st_size) { - ret = ::ftruncate(**fd, offset + len); - if (ret < 0) { - ret = -errno; - lfn_close(fd); - goto out; + // first try fallocate + ret = fallocate(**fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE, + offset, len); + if (ret < 0) { + ret = -errno; + } else { + // ensure we extent file size, if needed + if (offset + len > st.st_size) { + ret = ::ftruncate(**fd, offset + len); + if (ret < 0) { + ret = -errno; + lfn_close(fd); + goto out; + } } } - } - lfn_close(fd); + lfn_close(fd); - if (ret >= 0 && m_filestore_sloppy_crc) { - int rc = backend->_crc_update_zero(**fd, offset, len); - assert(rc >= 0); - } + if (ret >= 0 && m_filestore_sloppy_crc) { + int rc = backend->_crc_update_zero(**fd, offset, len); + assert(rc >= 0); + } - if (ret == 0) - goto out; // yay! - if (ret != -EOPNOTSUPP) - goto out; // some other error + if (ret == 0) + goto out; // yay! + if (ret != -EOPNOTSUPP) + goto out; // some other error # endif # endif #endif + } // lame, kernel is old and doesn't support it. // write zeros.. yuck! - dout(20) << "zero FALLOC_FL_PUNCH_HOLE not supported, falling back to writing zeros" << dendl; + dout(20) << "zero falling back to writing zeros" << dendl; { bufferlist bl; bl.append_zero(len);