crimson/os/seastore/transaction_manager: Move CRC_NULL checks

check_full_extent_integrity would allow for CRC_NULL (0) checksums
when `full_extent_integrity_check` was false. Instead, move this
check as an assertiion into TransactionManager::read_pin.

This would allow us to reuse CRC_NULL concept for more purposes e.g
skipping integrity checks when no CRC is passed (next commits).

Note: With this change check_full_extent_integrity could be called
      only on non CRC_NULL, the check would be moved in next commits.

Signed-off-by: Matan Breizman <mbreizma@redhat.com>

diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc
index d1d4fc0d61d99fe453142f9679ea48b5d42b265b..9da688d6652fcd058ebb5139165f700e0e9e4eb3 100644 (file)
--- a/src/crimson/os/seastore/lba/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc
@@ -1281,8 +1281,8 @@ BtreeLBAManager::remap_mappings(
            val.pladdr = paddr + remap.offset;
          }
          val.refcount = EXTENT_DEFAULT_REF_COUNT;
-         val.checksum = 0; // the checksum should be updated later when
-                           // committing the transaction
+         // Checksum will be updated when the committing the transaction
+         val.checksum = CRC_NULL;
          return btree.insert(c, iter, new_key, std::move(val)
          ).si_then([c, &remap, &mapping, &ret, &iter](auto p) {
            auto &[it, inserted] = p;

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index f62c8a774ba13524ecae4776d53b16660e84884a..8c0b5f0d3823470ef2c4d1e3948b497c3ce9d72e 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -634,14 +634,7 @@ void TransactionManager::check_full_extent_integrity(
     t,
     pin_crc,
     ref_crc);
-  bool inconsistent = false;
-  if (full_extent_integrity_check) {
-    inconsistent = (pin_crc != ref_crc);
-  } else { // !full_extent_integrity_check: remapped extent may be skipped
-    inconsistent = !(pin_crc == 0 ||
-                     pin_crc == ref_crc);
-  }
-  if (unlikely(inconsistent)) {
+  if (unlikely(pin_crc != ref_crc)) {
     SUBERRORT(seastore_tm,
       "extent checksum inconsistent, recorded: 0x{:x}, actual: 0x{:x}",
       t,

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 884b8fe3f0c2beed2ade8a209c624ce0f56d196f..487bebe8939e27a7f9d34d2d99eb8493ee5d5f7c 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -1332,9 +1332,12 @@ private:
          }
        }).si_then([this, &t, &remaps, original_paddr,
                    original_laddr, original_len, FNAME](auto ext) mutable {
-         ceph_assert(full_extent_integrity_check
-             ? (ext && ext->is_fully_loaded())
-             : true);
+         if (full_extent_integrity_check) {
+           ceph_assert(ext && ext->is_fully_loaded());
+           // CRC_NULL shouldn't be possible when full extent
+           // integrity checks are enabled.
+           assert(ext->calc_crc32c() != CRC_NULL);
+         }
          std::optional<ceph::bufferptr> original_bptr;
          // TODO: preserve the bufferspace if partially loaded
          if (ext && ext->is_fully_loaded()) {
@@ -1485,7 +1488,7 @@ private:
     });
   }
 
-  void check_full_extent_integrity(
+  static void check_full_extent_integrity(
     Transaction &t, uint32_t ref_crc, uint32_t pin_crc);
 
   /**