From f34a60b1db13c34913e443f7726f95acb53f3214 Mon Sep 17 00:00:00 2001 From: Adam Kupczyk Date: Wed, 24 Apr 2024 17:13:22 +0000 Subject: [PATCH] ceph-bluestore-tool: Fix set-label-key and rm-label-key Functions were not adapted to multiple superblocks. Now for superblock labels that handle multiple copies, all are updated. Signed-off-by: Adam Kupczyk (cherry picked from commit 4927876f7a426d1712e1a3ec8cfd045c582abce1) --- src/os/bluestore/BlueStore.cc | 25 +++++++------ src/os/bluestore/BlueStore.h | 12 ++++--- src/os/bluestore/bluestore_tool.cc | 56 +++++++++++++++++++++++------- src/test/objectstore/store_test.cc | 16 +++++++-- 4 files changed, 81 insertions(+), 28 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index edb839df769..f09613a078a 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -6012,7 +6012,7 @@ int BlueStore::write_meta(const std::string& key, const std::string& value) } string p = path + "/block"; if (bdev_label_valid_locations.empty()) { - _read_main_bdev_label(cct, bdev, p, fsid, &bdev_label, &bdev_label_valid_locations, + _read_multi_bdev_label(cct, bdev, p, fsid, &bdev_label, &bdev_label_valid_locations, &bdev_label_multi, &bdev_label_epoch); } if (!bdev_label_valid_locations.empty()) { @@ -6035,7 +6035,7 @@ int BlueStore::read_meta(const std::string& key, std::string *value) } string p = path + "/block"; if (bdev_label_valid_locations.empty()) { - _read_main_bdev_label(cct, bdev, p, fsid, &bdev_label, &bdev_label_valid_locations, + _read_multi_bdev_label(cct, bdev, p, fsid, &bdev_label, &bdev_label_valid_locations, &bdev_label_multi, &bdev_label_epoch); } if (!bdev_label_valid_locations.empty()) { @@ -6470,7 +6470,7 @@ int BlueStore::get_block_device_fsid(CephContext* cct, const string& path, unique_ptr bdev(BlockDevice::create(cct, bdev_path, nullptr, nullptr, nullptr, nullptr)); int r = bdev->open(bdev_path); if (r == 0) { - r = _read_main_bdev_label(cct, bdev.get(), bdev_path, uuid_d(), &label); + r = _read_multi_bdev_label(cct, bdev.get(), bdev_path, uuid_d(), &label); } if (r == 0) { *fsid = label.osd_uuid; @@ -6620,7 +6620,7 @@ int BlueStore::_read_bdev_label( 1 When some, but not all labels are read -ENOENT Otherwise */ -int BlueStore::_read_main_bdev_label( +int BlueStore::_read_multi_bdev_label( CephContext* cct, BlockDevice* bdev, const string& path, @@ -6680,7 +6680,7 @@ int BlueStore::_read_main_bdev_label( } } else { derr << __func__ << " label at 0x" << std::hex << position << std::dec - << " is multi=yes but no epoch=" << dendl; + << " is multi=yes but no epoch" << dendl; } } else if (r == 0) { derr << __func__ << " label at 0x" << std::hex << position << std::dec @@ -6826,7 +6826,7 @@ int BlueStore::_set_main_bdev_label() int BlueStore::_check_main_bdev_label() { string block_path = path + "/block"; - int r = _read_main_bdev_label(cct, bdev, block_path, fsid, &bdev_label, + int r = _read_multi_bdev_label(cct, bdev, block_path, fsid, &bdev_label, &bdev_label_valid_locations, &bdev_label_multi, &bdev_label_epoch); if (r < 0) return r; @@ -6846,16 +6846,21 @@ int BlueStore::_check_main_bdev_label() } int BlueStore::read_bdev_label( - CephContext* cct, const std::string &path, - bluestore_bdev_label_t *label, uint64_t disk_position) + CephContext* cct, + const std::string &path, + bluestore_bdev_label_t* out_label, + std::vector* out_valid_positions, + bool* out_is_multi, + int64_t* out_epoch) { unique_ptr bdev(BlockDevice::create( cct, path, nullptr, nullptr, nullptr, nullptr)); int r = bdev->open(path); if (r < 0) return r; - r = BlueStore::_read_bdev_label( - cct, bdev.get(), path, label, disk_position); + uuid_d fsid; + r = BlueStore::_read_multi_bdev_label( + cct, bdev.get(), path, fsid, out_label, out_valid_positions, out_is_multi, out_epoch); bdev->close(); return r; } diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index fa855a9b41b..a84fa8adf8f 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -2780,14 +2780,14 @@ private: const std::string& desc, bool create); int _set_main_bdev_label(); int _check_main_bdev_label(); - static int _read_main_bdev_label( + static int _read_multi_bdev_label( CephContext* cct, BlockDevice* bdev, const std::string& path, uuid_d fsid, bluestore_bdev_label_t *out_label, std::vector* out_valid_positions = nullptr, - bool* out_is_cloned = nullptr, + bool* out_is_multi = nullptr, int64_t* out_epoch = nullptr); int _set_bdev_label_size(const std::string& path, uint64_t size); void _main_bdev_label_try_reserve(); @@ -3428,8 +3428,12 @@ public: std::vector({disk_position})); } static int read_bdev_label( - CephContext* cct, const std::string &path, - bluestore_bdev_label_t *label, uint64_t disk_position = 0); + CephContext* cct, + const std::string &path, + bluestore_bdev_label_t *out_label, + std::vector* out_valid_positions = nullptr, + bool* out_is_cloned = nullptr, + int64_t* out_epoch = nullptr); static int write_bdev_label( CephContext* cct, const std::string &path, const bluestore_bdev_label_t& label, uint64_t disk_position = 0); diff --git a/src/os/bluestore/bluestore_tool.cc b/src/os/bluestore/bluestore_tool.cc index 0d2b3e8f442..b1cc5807dfd 100644 --- a/src/os/bluestore/bluestore_tool.cc +++ b/src/os/bluestore/bluestore_tool.cc @@ -694,7 +694,11 @@ int main(int argc, char **argv) } else if (action == "set-label-key") { bluestore_bdev_label_t label; - int r = BlueStore::read_bdev_label(cct.get(), devs.front(), &label); + std::vector valid_positions; + bool is_multi = false; + int64_t epoch = -1; + int r = BlueStore::read_bdev_label(cct.get(), devs.front(), &label, + &valid_positions, &is_multi, &epoch); if (r < 0) { cerr << "unable to read label for " << devs.front() << ": " << cpp_strerror(r) << std::endl; @@ -716,16 +720,32 @@ int main(int argc, char **argv) } else { label.meta[key] = value; } - r = BlueStore::write_bdev_label(cct.get(), devs.front(), label); - if (r < 0) { - cerr << "unable to write label for " << devs.front() << ": " - << cpp_strerror(r) << std::endl; - exit(EXIT_FAILURE); + if (is_multi) { + epoch++; + label.meta["epoch"] = std::to_string(epoch); + } + bool wrote_at_least_one = false; + for (uint64_t position : valid_positions) { + r = BlueStore::write_bdev_label(cct.get(), devs.front(), label, position); + if (r < 0) { + cerr << "unable to write label for " << devs.front() + << " at 0x" << std::hex << position << std::dec + << ": " << cpp_strerror(r) << std::endl; + } else { + wrote_at_least_one = true; + } + } + if (!wrote_at_least_one) { + exit(EXIT_FAILURE); } } else if (action == "rm-label-key") { bluestore_bdev_label_t label; - int r = BlueStore::read_bdev_label(cct.get(), devs.front(), &label); + std::vector valid_positions; + bool is_multi = false; + int64_t epoch = -1; + int r = BlueStore::read_bdev_label(cct.get(), devs.front(), &label, + &valid_positions, &is_multi, &epoch); if (r < 0) { cerr << "unable to read label for " << devs.front() << ": " << cpp_strerror(r) << std::endl; @@ -736,11 +756,23 @@ int main(int argc, char **argv) exit(EXIT_FAILURE); } label.meta.erase(key); - r = BlueStore::write_bdev_label(cct.get(), devs.front(), label); - if (r < 0) { - cerr << "unable to write label for " << devs.front() << ": " - << cpp_strerror(r) << std::endl; - exit(EXIT_FAILURE); + if (is_multi) { + epoch++; + label.meta["epoch"] = std::to_string(epoch); + } + bool wrote_at_least_one = false; + for (uint64_t position : valid_positions) { + r = BlueStore::write_bdev_label(cct.get(), devs.front(), label, position); + if (r < 0) { + cerr << "unable to write label for " << devs.front() + << " at 0x" << std::hex << position << std::dec + << ": " << cpp_strerror(r) << std::endl; + } else { + wrote_at_least_one = true; + } + } + if (!wrote_at_least_one) { + exit(EXIT_FAILURE); } } else if (action == "bluefs-bdev-sizes") { diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc index f3d09755327..556b00b920d 100644 --- a/src/test/objectstore/store_test.cc +++ b/src/test/objectstore/store_test.cc @@ -221,12 +221,24 @@ class MultiLabelTest : public StoreTestDeferredSetup { } bool read_bdev_label(bluestore_bdev_label_t* label, uint64_t position) { string bdev_path = get_data_dir() + "/block"; - int r = BlueStore::read_bdev_label(g_ceph_context, bdev_path, label, position); + unique_ptr bdev(BlockDevice::create( + g_ceph_context, bdev_path, nullptr, nullptr, nullptr, nullptr)); + int r = bdev->open(bdev_path); + if (r < 0) + return r; + r = BlueStore::debug_read_bdev_label(g_ceph_context, bdev.get(), bdev_path, label, position); + bdev->close(); return r; } bool write_bdev_label(const bluestore_bdev_label_t& label, uint64_t position) { string bdev_path = get_data_dir() + "/block"; - int r = BlueStore::write_bdev_label(g_ceph_context, bdev_path, label, position); + unique_ptr bdev(BlockDevice::create( + g_ceph_context, bdev_path, nullptr, nullptr, nullptr, nullptr)); + int r = bdev->open(bdev_path); + if (r < 0) + return r; + r = BlueStore::debug_write_bdev_label(g_ceph_context, bdev.get(), bdev_path, label, position); + bdev->close(); return r; } protected: -- 2.39.5