From: Sage Weil <sage@inktank.com>
Date: Thu, 21 Jun 2012 18:17:34 +0000 (-0700)
Subject: filestore: remove btrfs start/end transaction ioctl code
X-Git-Tag: v0.49~83^2~8
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7044192da5afcc9e5036dce07a0e450fe34ca3bc;p=ceph.git

filestore: remove btrfs start/end transaction ioctl code

This was an ill-conceived approach to getting atomic transactions out of
btrfs.  It doesn't offer rollback, which means that any error means we need
to wedge the file system and reboot in order to avoid corrupting the
data set.  And that's silly!

Snapshots are more robust and only marginally slower (because we have to
quiesce our writes while waiting for the snap to start, and btrfs resume
work in-kernel slightly faster...maybe).

Fixes: #2623
Signed-off-by: Sage Weil <sage@inktank.com>
---

diff --git a/src/common/config_opts.h b/src/common/config_opts.h
index bd6c39aedc62..b671807d27ba 100644
--- a/src/common/config_opts.h
+++ b/src/common/config_opts.h
@@ -347,7 +347,6 @@ OPTION(filestore_max_inline_xattrs, OPT_U32, 2)
 
 OPTION(filestore_max_sync_interval, OPT_DOUBLE, 5)    // seconds
 OPTION(filestore_min_sync_interval, OPT_DOUBLE, .01)  // seconds
-OPTION(filestore_btrfs_trans, OPT_BOOL, false)
 OPTION(filestore_btrfs_snap, OPT_BOOL, true)
 OPTION(filestore_btrfs_clone_range, OPT_BOOL, true)
 OPTION(filestore_fsync_flushes_journal_data, OPT_BOOL, false)
diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index 41433e768397..2f88157461cf 100644
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -690,7 +690,6 @@ FileStore::FileStore(const std::string &base, const std::string &jdev, const cha
   logger(NULL),
   m_filestore_btrfs_clone_range(g_conf->filestore_btrfs_clone_range),
   m_filestore_btrfs_snap (g_conf->filestore_btrfs_snap ),
-  m_filestore_btrfs_trans(g_conf->filestore_btrfs_trans),
   m_filestore_commit_timeout(g_conf->filestore_commit_timeout),
   m_filestore_fiemap(g_conf->filestore_fiemap),
   m_filestore_flusher (g_conf->filestore_flusher ),
@@ -2366,11 +2365,6 @@ int FileStore::do_transactions(list<Transaction*> &tls, uint64_t op_seq)
     ops += (*p)->get_num_ops();
   }
 
-  int id = _transaction_start(bytes, ops);
-  if (id < 0) {
-    return id;
-  }
-    
   int trans_num = 0;
   for (list<Transaction*>::iterator p = tls.begin();
        p != tls.end();
@@ -2380,7 +2374,6 @@ int FileStore::do_transactions(list<Transaction*> &tls, uint64_t op_seq)
       break;
   }
   
-  _transaction_finish(id);
   return r;
 }
 
@@ -2414,52 +2407,6 @@ unsigned FileStore::apply_transactions(list<Transaction*> &tls,
 }
 
 
-// btrfs transaction start/end interface
-
-int FileStore::_transaction_start(uint64_t bytes, uint64_t ops)
-{
-  if (!btrfs || !btrfs_trans_start_end ||
-      !m_filestore_btrfs_trans)
-    return 0;
-
-  int fd = ::open(basedir.c_str(), O_RDONLY);
-  if (fd < 0) {
-    int err = errno;
-    dout(0) << "transaction_start got " << cpp_strerror(err) << " from btrfs open" << dendl;
-    assert(0 == "couldn't open basedir");
-  }
-
-  int r = ::ioctl(fd, BTRFS_IOC_TRANS_START);
-  if (r < 0) {
-    int err = errno;
-    dout(0) << "transaction_start got " << cpp_strerror(err) << " from btrfs ioctl" << dendl;    
-    TEMP_FAILURE_RETRY(::close(fd));
-    return -err;
-  }
-  dout(10) << "transaction_start " << fd << dendl;
-
-  char fn[PATH_MAX];
-  snprintf(fn, sizeof(fn), "%s/current/trans.%d", basedir.c_str(), fd);
-  ::mknod(fn, 0644, 0);
-
-  return fd;
-}
-
-void FileStore::_transaction_finish(int fd)
-{
-  if (!btrfs || !btrfs_trans_start_end ||
-      !m_filestore_btrfs_trans)
-    return;
-
-  char fn[PATH_MAX];
-  snprintf(fn, sizeof(fn), "%s/current/trans.%d", basedir.c_str(), fd);
-  ::unlink(fn);
-  
-  dout(10) << "transaction_finish " << fd << dendl;
-  ::ioctl(fd, BTRFS_IOC_TRANS_END);
-  TEMP_FAILURE_RETRY(::close(fd));
-}
-
 void FileStore::_set_replay_guard(int fd,
 				  const SequencerPosition& spos,
 				  const hobject_t *hoid,
diff --git a/src/os/FileStore.h b/src/os/FileStore.h
index 236430dde3e8..e98301c67bba 100644
--- a/src/os/FileStore.h
+++ b/src/os/FileStore.h
@@ -314,8 +314,6 @@ public:
   int do_transactions(list<Transaction*> &tls, uint64_t op_seq);
   unsigned apply_transaction(Transaction& t, Context *ondisk=0);
   unsigned apply_transactions(list<Transaction*>& tls, Context *ondisk=0);
-  int _transaction_start(uint64_t bytes, uint64_t ops);
-  void _transaction_finish(int id);
   unsigned _do_transaction(Transaction& t, uint64_t op_seq, int trans_num);
 
   int queue_transaction(Sequencer *osr, Transaction* t);
@@ -477,7 +475,6 @@ private:
 			  const std::set <std::string> &changed);
   bool m_filestore_btrfs_clone_range;
   bool m_filestore_btrfs_snap;
-  bool m_filestore_btrfs_trans;
   float m_filestore_commit_timeout;
   bool m_filestore_fiemap;
   bool m_filestore_flusher;