os/FileStore: refactor backend instantiation

Use a factory method for creating the backend so that the logic is not
duplicated in mkfs() and _detect_fs().  Make the other fs-dependent logic
(xattr limits) key off of the fs_type magic.

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

diff --git a/src/os/BtrfsFileStoreBackend.h b/src/os/BtrfsFileStoreBackend.h
index cec976180d6e792f5ab4b633dc6d7c50cb8708cb..9bc878f77676e0a30568e1794712e1d56493572f 100644 (file)
--- a/src/os/BtrfsFileStoreBackend.h
+++ b/src/os/BtrfsFileStoreBackend.h
@@ -31,6 +31,9 @@ private:
 public:
   BtrfsFileStoreBackend(FileStore *fs);
   ~BtrfsFileStoreBackend() {}
+  const char *get_name() {
+    return "btrfs";
+  }
   int detect_features();
   bool can_checkpoint();
   int create_current();

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index cd895199f2f4bdb99bd342c7dcccf14d6033a535..64de42b5a3f38a91e7a156eae784aaf5686dc996 100644 (file)
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -418,7 +418,7 @@ FileStore::FileStore(const std::string &base, const std::string &jdev, const cha
   blk_size(0),
   fsid_fd(-1), op_fd(-1),
   basedir_fd(-1), current_fd(-1),
-  generic_backend(NULL), backend(NULL),
+  backend(NULL),
   index_manager(do_update),
   ondisk_finisher(g_ceph_context),
   lock("FileStore::lock"),
@@ -462,7 +462,7 @@ FileStore::FileStore(const std::string &base, const std::string &jdev, const cha
   m_filestore_sloppy_crc(g_conf->filestore_sloppy_crc),
   m_filestore_sloppy_crc_block_size(g_conf->filestore_sloppy_crc_block_size),
   m_filestore_max_alloc_hint_size(g_conf->filestore_max_alloc_hint_size),
-  m_fs_type(FS_TYPE_NONE),
+  m_fs_type(0),
   m_filestore_max_inline_xattr_size(0),
   m_filestore_max_inline_xattrs(0)
 {
@@ -512,9 +512,6 @@ FileStore::FileStore(const std::string &base, const std::string &jdev, const cha
   g_ceph_context->get_perfcounters_collection()->add(logger);
   g_ceph_context->_conf->add_observer(this);
 
-  generic_backend = new GenericFileStoreBackend(this);
-  backend = generic_backend;
-
   superblock.compat_features = get_fs_initial_compat_set();
 }
 
@@ -523,8 +520,6 @@ FileStore::~FileStore()
   g_ceph_context->_conf->remove_observer(this);
   g_ceph_context->get_perfcounters_collection()->remove(logger);
 
-  delete generic_backend;
-
   if (journal)
     journal->logger = NULL;
   delete logger;
@@ -584,6 +579,56 @@ int FileStore::dump_journal(ostream& out)
   return r;
 }
 
+FileStoreBackend *FileStoreBackend::create(long f_type, FileStore *fs)
+{
+  switch (f_type) {
+#if defined(__linux__)
+  case BTRFS_SUPER_MAGIC:
+    return new BtrfsFileStoreBackend(fs);
+# ifdef HAVE_LIBXFS
+  case XFS_SUPER_MAGIC:
+    return new XfsFileStoreBackend(fs);
+# endif
+#endif
+#ifdef HAVE_LIBZFS
+  case ZFS_SUPER_MAGIC:
+    return new ZFSFileStoreBackend(fs);
+#endif
+  default:
+    return new GenericFileStoreBackend(fs);
+  }
+}
+
+void FileStore::create_backend(long f_type)
+{
+  m_fs_type = f_type;
+
+  assert(backend == NULL);
+  backend = FileStoreBackend::create(f_type, this);
+
+  dout(0) << "backend " << backend->get_name()
+         << " (magic 0x" << std::hex << f_type << std::dec << ")"
+         << dendl;
+
+  switch (f_type) {
+  case BTRFS_SUPER_MAGIC:
+    wbthrottle.set_fs(WBThrottle::BTRFS);
+    break;
+
+  case XFS_SUPER_MAGIC:
+    // wbthrottle is constructed with fs(WBThrottle::XFS)
+    if (m_filestore_replica_fadvise) {
+      dout(1) << " disabling 'filestore replica fadvise' due to known issues with fadvise(DONTNEED) on xfs" << dendl;
+      g_conf->set_val("filestore_replica_fadvise", "false");
+      g_conf->apply_changes(NULL);
+      assert(m_filestore_replica_fadvise == false);
+    }
+    break;
+  }
+
+  set_xattr_limits_via_conf();
+}
+
 int FileStore::mkfs()
 {
   int ret = 0;
@@ -677,19 +722,7 @@ int FileStore::mkfs()
     goto close_fsid_fd;
   }
 
-  if (basefs.f_type == BTRFS_SUPER_MAGIC) {
-#if defined(__linux__)
-    backend = new BtrfsFileStoreBackend(this);
-#endif
-  } else if (basefs.f_type == XFS_SUPER_MAGIC) {
-#ifdef HAVE_LIBXFS
-    backend = new XfsFileStoreBackend(this);
-#endif
-  } else if (basefs.f_type == ZFS_SUPER_MAGIC) {
-#ifdef HAVE_LIBZFS
-    backend = new ZFSFileStoreBackend(this);
-#endif
-  }
+  create_backend(basefs.f_type);
 
   ret = backend->create_current();
   if (ret < 0) {
@@ -761,10 +794,8 @@ int FileStore::mkfs()
   fsid_fd = -1;
  close_basedir_fd:
   VOID_TEMP_FAILURE_RETRY(::close(basedir_fd));
-  if (backend != generic_backend) {
-    delete backend;
-    backend = generic_backend;
-  }
+  delete backend;
+  backend = NULL;
   return ret;
 }
 
@@ -871,40 +902,7 @@ int FileStore::_detect_fs()
 
   blk_size = st.f_bsize;
 
-  m_fs_type = FS_TYPE_OTHER;
-  if (st.f_type == BTRFS_SUPER_MAGIC) {
-#if defined(__linux__)
-    dout(0) << "mount detected btrfs" << dendl;
-    backend = new BtrfsFileStoreBackend(this);
-    m_fs_type = FS_TYPE_BTRFS;
-
-    wbthrottle.set_fs(WBThrottle::BTRFS);
-#endif
-  } else if (st.f_type == XFS_SUPER_MAGIC) {
-#ifdef HAVE_LIBXFS
-    dout(0) << "mount detected xfs (libxfs)" << dendl;
-    backend = new XfsFileStoreBackend(this);
-#else
-    dout(0) << "mount detected xfs" << dendl;
-#endif
-    m_fs_type = FS_TYPE_XFS;
-
-    // wbthrottle is constructed with fs(WBThrottle::XFS)
-    if (m_filestore_replica_fadvise) {
-      dout(1) << " disabling 'filestore replica fadvise' due to known issues with fadvise(DONTNEED) on xfs" << dendl;
-      g_conf->set_val("filestore_replica_fadvise", "false");
-      g_conf->apply_changes(NULL);
-      assert(m_filestore_replica_fadvise == false);
-    }
-  } else if (st.f_type == ZFS_SUPER_MAGIC) {
-#ifdef HAVE_LIBZFS
-    dout(0) << "mount detected zfs (libzfs)" << dendl;
-    backend = new ZFSFileStoreBackend(this);
-    m_fs_type = FS_TYPE_ZFS;
-#endif
-  }
-
-  set_xattr_limits_via_conf();
+  create_backend(st.f_type);
 
   r = backend->detect_features();
   if (r < 0) {
@@ -1547,10 +1545,8 @@ int FileStore::umount()
     basedir_fd = -1;
   }
 
-  if (backend != generic_backend) {
-    delete backend;
-    backend = generic_backend;
-  }
+  delete backend;
+  backend = NULL;
 
   object_map.reset();
 
@@ -5011,33 +5007,28 @@ void FileStore::set_xattr_limits_via_conf()
   uint32_t fs_xattr_size;
   uint32_t fs_xattrs;
 
-  assert(m_fs_type != FS_TYPE_NONE);
-
-  switch(m_fs_type) {
-    case FS_TYPE_XFS:
-      fs_xattr_size = g_conf->filestore_max_inline_xattr_size_xfs;
-      fs_xattrs = g_conf->filestore_max_inline_xattrs_xfs;
-      break;
-    case FS_TYPE_BTRFS:
-      fs_xattr_size = g_conf->filestore_max_inline_xattr_size_btrfs;
-      fs_xattrs = g_conf->filestore_max_inline_xattrs_btrfs;
-      break;
-    case FS_TYPE_ZFS:
-    case FS_TYPE_OTHER:
-      fs_xattr_size = g_conf->filestore_max_inline_xattr_size_other;
-      fs_xattrs = g_conf->filestore_max_inline_xattrs_other;
-      break;
-    default:
-      assert(!"Unknown fs type");
-  }
-
-  //Use override value if set
+  switch (m_fs_type) {
+  case XFS_SUPER_MAGIC:
+    fs_xattr_size = g_conf->filestore_max_inline_xattr_size_xfs;
+    fs_xattrs = g_conf->filestore_max_inline_xattrs_xfs;
+    break;
+  case BTRFS_SUPER_MAGIC:
+    fs_xattr_size = g_conf->filestore_max_inline_xattr_size_btrfs;
+    fs_xattrs = g_conf->filestore_max_inline_xattrs_btrfs;
+    break;
+  default:
+    fs_xattr_size = g_conf->filestore_max_inline_xattr_size_other;
+    fs_xattrs = g_conf->filestore_max_inline_xattrs_other;
+    break;
+  }
+
+  // Use override value if set
   if (g_conf->filestore_max_inline_xattr_size)
     m_filestore_max_inline_xattr_size = g_conf->filestore_max_inline_xattr_size;
   else
     m_filestore_max_inline_xattr_size = fs_xattr_size;
 
-  //Use override value if set
+  // Use override value if set
   if (g_conf->filestore_max_inline_xattrs)
     m_filestore_max_inline_xattrs = g_conf->filestore_max_inline_xattrs;
   else

diff --git a/src/os/FileStore.h b/src/os/FileStore.h
index c9907295777e6a0aebe75c7947f1f1e1347362b8..347e1a2d598edf51031b24fe9f9e39a6e721d362 100644 (file)
--- a/src/os/FileStore.h
+++ b/src/os/FileStore.h
@@ -64,14 +64,6 @@ static const __SWORD_TYPE ZFS_SUPER_MAGIC(0x2fc12fc1);
 #endif
 
 
-enum fs_types {
-  FS_TYPE_NONE = 0,
-  FS_TYPE_XFS,
-  FS_TYPE_BTRFS,
-  FS_TYPE_ZFS,
-  FS_TYPE_OTHER
-};
-
 class FileStoreBackend;
 
 #define CEPH_FS_FEATURE_INCOMPAT_SHARDS CompatSet::Feature(1, "sharded objects")
@@ -135,9 +127,10 @@ private:
 
   int fsid_fd, op_fd, basedir_fd, current_fd;
 
-  FileStoreBackend *generic_backend;
   FileStoreBackend *backend;
 
+  void create_backend(long f_type);
+
   deque<uint64_t> snaps;
 
   // Indexed Collections
@@ -614,7 +607,7 @@ private:
   bool m_filestore_sloppy_crc;
   int m_filestore_sloppy_crc_block_size;
   uint64_t m_filestore_max_alloc_hint_size;
-  enum fs_types m_fs_type;
+  long m_fs_type;
 
   //Determined xattr handling based on fs type
   void set_xattr_limits_via_conf();
@@ -680,9 +673,14 @@ protected:
   int get_crc_block_size() {
     return filestore->m_filestore_sloppy_crc_block_size;
   }
+
 public:
   FileStoreBackend(FileStore *fs) : filestore(fs) {}
   virtual ~FileStoreBackend() {}
+
+  static FileStoreBackend *create(long f_type, FileStore *fs);
+
+  virtual const char *get_name() = 0;
   virtual int detect_features() = 0;
   virtual int create_current() = 0;
   virtual bool can_checkpoint() = 0;

diff --git a/src/os/GenericFileStoreBackend.h b/src/os/GenericFileStoreBackend.h
index 8302ed1c1be27346eede9bc13097a1b5cf73b40f..fec56ceb45745ce7937116ae2d030110cac9cc49 100644 (file)
--- a/src/os/GenericFileStoreBackend.h
+++ b/src/os/GenericFileStoreBackend.h
@@ -28,6 +28,9 @@ public:
   GenericFileStoreBackend(FileStore *fs);
   virtual ~GenericFileStoreBackend() {}
 
+  virtual const char *get_name() {
+    return "generic";
+  }
   virtual int detect_features();
   virtual int create_current();
   virtual bool can_checkpoint() { return false; }

diff --git a/src/os/XfsFileStoreBackend.h b/src/os/XfsFileStoreBackend.h
index aaada403ccdb95df1480bbcef680f1c173693cb4..282fc1c9ba12355f882a02b0e68c7579fac6437b 100644 (file)
--- a/src/os/XfsFileStoreBackend.h
+++ b/src/os/XfsFileStoreBackend.h
@@ -26,6 +26,9 @@ private:
 public:
   XfsFileStoreBackend(FileStore *fs);
   ~XfsFileStoreBackend() {}
+  const char *get_name() {
+    return "xfs";
+  }
   int detect_features();
   int set_alloc_hint(int fd, uint64_t hint);
 };