From: Adam Kupczyk <akupczyk@ibm.com>
Date: Mon, 1 Jul 2024 13:44:41 +0000 (+0000)
Subject: os/bluestore: fix fsck fixing multiple bdev label (superblock)
X-Git-Tag: v19.2.1~271^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cf4d671836b9ba799b738d2c74d25775a2a2f8bd;p=ceph.git

os/bluestore: fix fsck fixing multiple bdev label (superblock)

The problem was that after fixing label location,
location was not marked as "good label".
In result the region was not excluded when saving allocator state.
After reading allocator state, label was treated as corrupted.

Fixed fixing bdev label when freelist is bitmap (HDD).

Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>
(cherry picked from commit 721482adc0de1371cf92cd4e73255ba170a2a5ac)
---

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 93de73fb4838..74f0f5845ec6 100644
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -10477,10 +10477,8 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
         dout(1) << "fsck bdev label at 0x" << std::hex << position << std::dec
                 <<  "taken by bluefs, cannot be fixed" << dendl;
       } else {
-        if (repair) {
-          // Mark blocks so we could move offending objects away.
-          bdev_labels_in_repair.push_back(position);
-        }
+        // Mark blocks so we could move offending objects away.
+        bdev_labels_in_repair.push_back(position);
       }
     }
     // Mark locations of those bdev labels that are not taken by bluefs.
@@ -11064,7 +11062,10 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
         ceph_assert(bdev_label_valid_locations.empty());
       }
       //unmark extra bdev copies, will collide with the check
-      for (uint64_t location : bdev_label_valid_locations) {
+
+      std::vector<uint64_t> sum = bdev_label_valid_locations;
+      sum.insert(sum.end(), bdev_labels_in_repair.begin(), bdev_labels_in_repair.end());
+      for (uint64_t location : sum) {
         uint64_t length = std::max<uint64_t>(BDEV_LABEL_BLOCK_SIZE, alloc_size);
         if (location != BDEV_FIRST_LABEL_POSITION) {
           apply_for_bitset_range(location, length, alloc_size, used_blocks,
@@ -11163,6 +11164,11 @@ int BlueStore::_fsck_on_open(BlueStore::FSCKDepth depth, bool repair)
     // Now fix bdev_labels that were detected to be broken & repairable.
     string p = path + "/block";
     _write_bdev_label(cct, bdev, p, bdev_label, bdev_labels_in_repair);
+    for (uint64_t pos : bdev_labels_in_repair) {
+      if (pos != BDEV_FIRST_LABEL_POSITION) {
+        bdev_label_valid_locations.push_back(pos);
+      }
+    }
     repaired += bdev_labels_in_repair.size();
   }
 
diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc
index f1bd146eaeae..486aa6be529c 100644
--- a/src/test/objectstore/store_test.cc
+++ b/src/test/objectstore/store_test.cc
@@ -10693,6 +10693,10 @@ TEST_P(MultiLabelTest, UpgradeToMultiLabelCollisionWithObjects) {
     }
   }
   umount();
+  // Need to do 2 passes of repair:
+  // - the first one moves offending objects away
+  // - the second can then fix bdev labels
+  store->repair(false);
   ASSERT_EQ(store->repair(false), 0);
   ASSERT_EQ(store->fsck(false), 0);
   bluestore_bdev_label_t label;