From: Igor Fedotov Date: Wed, 22 Apr 2020 11:41:16 +0000 (+0300) Subject: os/bluestore: fix freelist's meta usage when bdev label is unsupported X-Git-Tag: v16.1.0~2497^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a1c8d38c4aaad778610c7a07c893ca9466f0fca7;p=ceph.git os/bluestore: fix freelist's meta usage when bdev label is unsupported This primarily applies to SPDK/NVMEDevice backend. Signed-off-by: Igor Fedotov --- diff --git a/src/os/bluestore/BitmapFreelistManager.cc b/src/os/bluestore/BitmapFreelistManager.cc index 3f75c30dea28..f48e3943d6dc 100644 --- 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 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 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 09465da611b1..8c08daa94e17 100644 --- 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 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 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 de18468d4745..4d3f7807dd64 100644 --- 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> 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 c9e831c7a4b1..82fbd72b56af 100644 --- 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 d1c4976f58cf..911a07aa2724 100644 --- 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 cfg_reader) = 0; virtual void sync(KeyValueDB* kvdb) = 0; virtual void shutdown() = 0;