From 21580723aa6a6ed18cc232571311511437f9dfaf Mon Sep 17 00:00:00 2001
From: Yingxin Cheng <yingxin.cheng@intel.com>
Date: Thu, 15 Jul 2021 10:17:38 +0800
Subject: [PATCH] crimson/os/seastore: assert the committing delta is not empty

It makes no sense to commit an empty delta. It is mostly an issue that
user forget to generate delta during mutation, or there are futile
copy-on-write operations.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
---
 src/crimson/os/seastore/cache.cc                 |  1 +
 .../staged-fltree/node_delta_recorder.h          |  1 -
 src/test/crimson/seastore/test_block.cc          | 16 ++++++++++++++++
 src/test/crimson/seastore/test_block.h           |  7 ++++---
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 10c38d066f5..e1727e0aac9 100644
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -573,6 +573,7 @@ record_t Cache::prepare_record(Transaction &t)
 	});
       i->last_committed_crc = final_crc;
     }
+    assert(record.deltas.back().bl.length());
   }
 
   // Transaction is now a go, set up in-memory cache state
diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_delta_recorder.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_delta_recorder.h
index 27df8e23a19..ea26195de71 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_delta_recorder.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_delta_recorder.h
@@ -29,7 +29,6 @@ class DeltaRecorder {
   }
 
   ceph::bufferlist get_delta() {
-    assert(!is_empty());
     return std::move(encoded);
   }
 
diff --git a/src/test/crimson/seastore/test_block.cc b/src/test/crimson/seastore/test_block.cc
index f3d6531bd92..f7a39b0ef59 100644
--- a/src/test/crimson/seastore/test_block.cc
+++ b/src/test/crimson/seastore/test_block.cc
@@ -22,4 +22,20 @@ void TestBlock::apply_delta(const ceph::bufferlist &bl) {
   }
 }
 
+ceph::bufferlist TestBlockPhysical::get_delta() {
+  ceph::bufferlist bl;
+  encode(delta, bl);
+  return bl;
+}
+
+void TestBlockPhysical::apply_delta_and_adjust_crc(
+    paddr_t, const ceph::bufferlist &bl) {
+  auto biter = bl.begin();
+  decltype(delta) deltas;
+  decode(deltas, biter);
+  for (auto &&d : deltas) {
+    set_contents(d.val, d.offset, d.len);
+  }
+}
+
 }
diff --git a/src/test/crimson/seastore/test_block.h b/src/test/crimson/seastore/test_block.h
index 44ec65a2375..4df99784ad4 100644
--- a/src/test/crimson/seastore/test_block.h
+++ b/src/test/crimson/seastore/test_block.h
@@ -90,7 +90,7 @@ struct TestBlockPhysical : crimson::os::seastore::CachedExtent{
 
   TestBlockPhysical(ceph::bufferptr &&ptr)
     : CachedExtent(std::move(ptr)) {}
-  TestBlockPhysical(const TestBlock &other)
+  TestBlockPhysical(const TestBlockPhysical &other)
     : CachedExtent(other) {}
 
   CachedExtentRef duplicate_for_write() final {
@@ -104,15 +104,16 @@ struct TestBlockPhysical : crimson::os::seastore::CachedExtent{
 
   void set_contents(char c, uint16_t offset, uint16_t len) {
     ::memset(get_bptr().c_str() + offset, c, len);
+    delta.push_back({c, offset, len});
   }
 
   void set_contents(char c) {
     set_contents(c, 0, get_length());
   }
 
-  ceph::bufferlist get_delta() final { return ceph::bufferlist(); }
+  ceph::bufferlist get_delta() final;
 
-  void apply_delta_and_adjust_crc(paddr_t, const ceph::bufferlist &bl) final {}
+  void apply_delta_and_adjust_crc(paddr_t, const ceph::bufferlist &bl) final;
 };
 using TestBlockPhysicalRef = TCachedExtentRef<TestBlockPhysical>;
 
-- 
2.39.5