os/bluestore: fix freelist's meta usage when bdev label is unsupported

This primarily applies to SPDK/NVMEDevice backend.

Signed-off-by: Igor Fedotov <ifedotov@suse.com>

diff --git a/src/os/bluestore/BitmapFreelistManager.cc b/src/os/bluestore/BitmapFreelistManager.cc
index 3f75c30dea289a13a5d9b83aa1369332d85aecc3..f48e3943d6dc1313e67da18c9f5214a8b84e1551 100644 (file)
--- a/src/os/bluestore/BitmapFreelistManager.cc
+++ b/src/os/bluestore/BitmapFreelistManager.cc
@@ -82,7 +82,7 @@ int BitmapFreelistManager::create(uint64_t new_size, uint64_t granularity,
     // set past-eof blocks as allocated
     _xor(size, blocks * bytes_per_block - size, txn);
   }
-  dout(10) << __func__
+  dout(0) << __func__
           << " size 0x" << std::hex << size
           << " bytes_per_block 0x" << bytes_per_block
           << " blocks 0x" << blocks
@@ -167,7 +167,7 @@ int BitmapFreelistManager::read_size_meta_from_db(KeyValueDB* kvdb,
   int r = kvdb->get(meta_prefix, "size", &v);
   if (r < 0) {
     derr << __func__ << " missing size meta in DB" << dendl;
-    return ENOENT;
+    return -ENOENT;
   } else {
     auto p = v.cbegin();
     decode(*res, p);
@@ -216,12 +216,11 @@ void BitmapFreelistManager::_load_from_db(KeyValueDB* kvdb)
 }
 
 
-int BitmapFreelistManager::init(const bluestore_bdev_label_t& label,
-  KeyValueDB *kvdb,
-  bool db_in_read_only)
+int BitmapFreelistManager::init(KeyValueDB *kvdb, bool db_in_read_only,
+  std::function<int(const std::string&, std::string*)> cfg_reader)
 {
   dout(1) << __func__ << dendl;
-  int r = _init_from_label(label);
+  int r = _init_from_external_cfg(cfg_reader);
   if (r != 0) {
     dout(1) << __func__ << " fall back to legacy meta repo" << dendl;
     _load_from_db(kvdb);
@@ -238,70 +237,43 @@ int BitmapFreelistManager::init(const bluestore_bdev_label_t& label,
   return 0;
 }
 
-int BitmapFreelistManager::_init_from_label(const bluestore_bdev_label_t& label)
+int BitmapFreelistManager::_init_from_external_cfg(
+  std::function<int(const std::string&, std::string*)> cfg_reader)
 {
   dout(1) << __func__ << dendl;
 
-  int r = ENOENT;
   string err;
 
-  auto it = label.meta.find("bfm_size");
-  auto end = label.meta.end();
-  if (it != end) {
-    size = strict_iecstrtoll(it->second.c_str(), &err);
-    if (!err.empty()) {
-      derr << __func__ << " Failed to parse - "
-        << it->first << ":" << it->second
-        << ", error: " << err << dendl;
-      return r;
-    }
-  } else {
-    // this is expected for legacy deployed OSDs
-    dout(0) << __func__ << " bfm_size not found in bdev meta" << dendl;
-    return r;
-  }
-
-  it = label.meta.find("bfm_blocks");
-  if (it != end) {
-    blocks = strict_iecstrtoll(it->second.c_str(), &err);
-    if (!err.empty()) {
-      derr << __func__ << " Failed to parse - "
-        << it->first << ":" << it->second
-        << ", error: " << err << dendl;
+  const size_t key_count = 4;
+  string keys[key_count] = {
+    "bfm_size",
+    "bfm_blocks",
+    "bfm_bytes_per_block",
+    "bfm_blocks_per_key"};
+  uint64_t* vals[key_count] = {
+    &size,
+    &blocks,
+    &bytes_per_block,
+    &blocks_per_key};
+
+  for (size_t i = 0; i < key_count; i++) {
+    string val;
+    int r = cfg_reader(keys[i], &val);
+    if (r == 0) {
+      *(vals[i]) = strict_iecstrtoll(val.c_str(), &err);
+      if (!err.empty()) {
+        derr << __func__ << " Failed to parse - "
+          << keys[i] << ":" << val
+          << ", error: " << err << dendl;
+        return r;
+      }
+    } else {
+      // this is expected for legacy deployed OSDs
+      dout(0) << __func__ << " " << keys[i] << " not found in bdev meta" << dendl;
       return r;
     }
-  } else {
-    derr << __func__ << " bfm_blocks not found in bdev meta" << dendl;
-    return r;
   }
 
-  it = label.meta.find("bfm_bytes_per_block");
-  if (it != end) {
-    bytes_per_block = strict_iecstrtoll(it->second.c_str(), &err);
-    if (!err.empty()) {
-      derr << __func__ << " Failed to parse - "
-        << it->first << ":" << it->second
-        << ", error: " << err << dendl;
-      return r;
-    }
-  } else {
-    derr << __func__ << " bfm_bytes_per_block not found in bdev meta" << dendl;
-    return r;
-  }
-  it = label.meta.find("bfm_blocks_per_key");
-  if (it != end) {
-    blocks_per_key = strict_iecstrtoll(it->second.c_str(), &err);
-    if (!err.empty()) {
-      derr << __func__ << " Failed to parse - "
-        << it->first << ":" << it->second
-        << ", error: " << err << dendl;
-      return r;
-    }
-  } else {
-    derr << __func__ << " bfm_blocks_per_key not found in bdev meta" << dendl;
-    return r;
-  }
-  r = 0;
   return 0;
 }
 

diff --git a/src/os/bluestore/BitmapFreelistManager.h b/src/os/bluestore/BitmapFreelistManager.h
index 09465da611b17b331931f7e3a51fe88e105f1a0d..8c08daa94e17ed92bfc873fb5b7a779bf1f19269 100644 (file)
--- a/src/os/bluestore/BitmapFreelistManager.h
+++ b/src/os/bluestore/BitmapFreelistManager.h
@@ -46,7 +46,8 @@ class BitmapFreelistManager : public FreelistManager {
     uint64_t offset, uint64_t length,
     KeyValueDB::Transaction txn);
 
-  int _init_from_label(const bluestore_bdev_label_t& label);
+  int _init_from_external_cfg(
+    std::function<int(const std::string&, std::string*)> cfg_reader);
 
   int _expand(uint64_t new_size, KeyValueDB* db);
 
@@ -66,9 +67,8 @@ public:
   int create(uint64_t size, uint64_t granularity,
             KeyValueDB::Transaction txn) override;
 
-  int init(const bluestore_bdev_label_t& l,
-    KeyValueDB *kvdb,
-    bool db_in_read_only) override;
+  int init(KeyValueDB *kvdb, bool db_in_read_only,
+    std::function<int(const std::string&, std::string*)> cfg_reader) override;
 
   void shutdown() override;
   void sync(KeyValueDB* kvdb) override;

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index de18468d474526ca3855ecabb398ba3aa57b42fc..4d3f7807dd64c566a0610dab484605073cc40e27 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -4974,7 +4974,6 @@ void BlueStore::_close_bdev()
 int BlueStore::_open_fm(KeyValueDB::Transaction t, bool read_only)
 {
   int r;
-  bluestore_bdev_label_t label;
 
   ceph_assert(fm == NULL);
   fm = FreelistManager::create(cct, freelist_type, PREFIX_ALLOC);
@@ -5047,25 +5046,20 @@ int BlueStore::_open_fm(KeyValueDB::Transaction t, bool read_only)
        start += l + u;
       }
     }
-    r = _write_out_fm_meta(0, false, &label);
+    r = _write_out_fm_meta(0);
     ceph_assert(r == 0);
   } else {
-    string p = path + "/block";
-    r = _read_bdev_label(cct, p, &label);
+    r = fm->init(db, read_only,
+      [&](const std::string& key, std::string* result) {
+        return read_meta(key, result);
+    });
     if (r < 0) {
-      derr << __func__ << " freelist init failed, error reading bdev label: " << cpp_strerror(r) << dendl;
+      derr << __func__ << " freelist init failed: " << cpp_strerror(r) << dendl;
       delete fm;
       fm = NULL;
       return r;
     }
   }
-  r = fm->init(label, db, read_only);
-  if (r < 0) {
-    derr << __func__ << " freelist init failed: " << cpp_strerror(r) << dendl;
-    delete fm;
-    fm = NULL;
-    return r;
-  }
   // if space size tracked by free list manager is that higher than actual
   // dev size one can hit out-of-space allocation which will result
   // in data loss and/or assertions
@@ -5091,31 +5085,18 @@ void BlueStore::_close_fm()
   fm = NULL;
 }
 
-int BlueStore::_write_out_fm_meta(uint64_t target_size,
-  bool update_root_size,
-  bluestore_bdev_label_t* res_label)
+int BlueStore::_write_out_fm_meta(uint64_t target_size)
 {
+  int r = 0;
   string p = path + "/block";
 
   std::vector<std::pair<string, string>> fm_meta;
   fm->get_meta(target_size, &fm_meta);
 
-  bluestore_bdev_label_t label;
-  int r = _read_bdev_label(cct, p, &label);
-  if (r < 0)
-    return r;
-
   for (auto& m : fm_meta) {
-    label.meta[m.first] = m.second;
-  }
-  if (update_root_size) {
-    label.size = target_size;
-  }
-  r = _write_bdev_label(cct, p, label);
-  if (res_label) {
-    *res_label = label;
+    r = write_meta(m.first, m.second);
+    ceph_assert(r == 0);
   }
-
   return r;
 }
 
@@ -6916,7 +6897,7 @@ int BlueStore::expand_devices(ostream& out)
     out << bluefs_layout.shared_bdev
       << " : expanding " << " from 0x" << std::hex
       << size0 << " to 0x" << size << std::dec << std::endl;
-    _write_out_fm_meta(size, true);
+    _write_out_fm_meta(size);
     cold_close();
 
     // mount in read/write to sync expansion changes

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index c9e831c7a4b173540a0985089cafa117610e5842..82fbd72b56af3237d329fb68560ebcac957e58cc 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -2294,9 +2294,7 @@ private:
   void _close_db(bool read_only);
   int _open_fm(KeyValueDB::Transaction t, bool read_only);
   void _close_fm();
-  int _write_out_fm_meta(uint64_t target_size,
-    bool update_root_size = false,
-    bluestore_bdev_label_t* res_label = nullptr);
+  int _write_out_fm_meta(uint64_t target_size);
   int _open_alloc();
   void _close_alloc();
   int _open_collections();

diff --git a/src/os/bluestore/FreelistManager.h b/src/os/bluestore/FreelistManager.h
index d1c4976f58cfac43180cdae31def6b64f1e0cec2..911a07aa272473833635c44be3445d53ddd3d165 100644 (file)
--- a/src/os/bluestore/FreelistManager.h
+++ b/src/os/bluestore/FreelistManager.h
@@ -27,9 +27,8 @@ public:
   virtual int create(uint64_t size, uint64_t granularity,
                     KeyValueDB::Transaction txn) = 0;
 
-  virtual int init(const bluestore_bdev_label_t& l,
-    KeyValueDB *kvdb,
-    bool db_in_read_only) = 0;
+  virtual int init(KeyValueDB *kvdb, bool db_in_read_only,
+    std::function<int(const std::string&, std::string*)> cfg_reader) = 0;
   virtual void sync(KeyValueDB* kvdb) = 0;
   virtual void shutdown() = 0;