Merge pull request ceph#54609 from xxhdx1985126/wip-crimson-rollback-fixes

crimson/osd/pg_backend: fix rollback on whiteouts

Reviewed-by: Matan Breizman <[email protected]>

Author: Matan-B

src/crimson/osd/ops_executer.cc

@@ -638,8 +638,10 @@ OpsExecuter::do_execute_op(OSDOp& osd_op)
   case CEPH_OSD_OP_ROLLBACK:
     return do_write_op([this, &head=obc,
                         &osd_op](auto& backend, auto& os, auto& txn) {
-      return backend.rollback(os, osd_op, txn, *osd_op_params, delta_stats,
-                              head, pg->obc_loader);
+      ceph_assert(obc->ssc);
+      return backend.rollback(os, obc->ssc->snapset,
+			      osd_op, txn, *osd_op_params, delta_stats,
+                              head, pg->obc_loader, snapc);
     });
   case CEPH_OSD_OP_APPEND:
     return do_write_op([this, &osd_op](auto& backend, auto& os, auto& txn) {
@@ -666,9 +668,7 @@ OpsExecuter::do_execute_op(OSDOp& osd_op)
   case CEPH_OSD_OP_DELETE:
   {
     bool whiteout = false;
-    if (!obc->ssc->snapset.clones.empty() ||
-        (snapc.snaps.size() &&                      // there are snaps
-        snapc.snaps[0] > obc->ssc->snapset.seq)) {  // existing obj is old
+    if (should_whiteout(obc->ssc->snapset, snapc)) {  // existing obj is old
       logger().debug("{} has or will have clones, will whiteout {}",
                      __func__, obc->obs.oi.soid);
       whiteout = true;

src/crimson/osd/pg_backend.cc

@@ -770,12 +770,14 @@ PGBackend::write_iertr::future<> PGBackend::writefull(
 
 PGBackend::rollback_iertr::future<> PGBackend::rollback(
   ObjectState& os,
+  const SnapSet &ss,
   const OSDOp& osd_op,
   ceph::os::Transaction& txn,
   osd_op_params_t& osd_op_params,
   object_stat_sum_t& delta_stats,
   crimson::osd::ObjectContextRef head,
-  crimson::osd::ObjectContextLoader& obc_loader)
+  crimson::osd::ObjectContextLoader& obc_loader,
+  const SnapContext &snapc)
 {
   const ceph_osd_op& op = osd_op.op;
   snapid_t snapid = (uint64_t)op.snap.snapid;
@@ -833,12 +835,12 @@ PGBackend::rollback_iertr::future<> PGBackend::rollback(
     // if there's no snapshot, we delete the object;
     // otherwise, do nothing.
     crimson::ct_error::enoent::handle(
-    [this, &os, &snapid, &txn, &delta_stats] {
+    [this, &os, &snapid, &txn, &delta_stats, &snapc, &ss] {
       logger().debug("PGBackend::rollback: deleting head on {}"
                      " with snap_id of {}"
                      " because got ENOENT|whiteout on obc lookup",
                      os.oi.soid, snapid);
-      return remove(os, txn, delta_stats, false);
+      return remove(os, txn, delta_stats, should_whiteout(ss, snapc));
     }),
     rollback_ertr::pass_further{},
     crimson::ct_error::assert_all{"unexpected error in rollback"}

src/crimson/osd/pg_backend.h

@@ -197,12 +197,14 @@ class PGBackend
       rollback_ertr>;
   rollback_iertr::future<> rollback(
     ObjectState& os,
+    const SnapSet &ss,
     const OSDOp& osd_op,
     ceph::os::Transaction& txn,
     osd_op_params_t& osd_op_params,
     object_stat_sum_t& delta_stats,
     crimson::osd::ObjectContextRef head,
-    crimson::osd::ObjectContextLoader& obc_loader);
+    crimson::osd::ObjectContextLoader& obc_loader,
+    const SnapContext &snapc);
   write_iertr::future<> truncate(
     ObjectState& os,
     const OSDOp& osd_op,

src/osd/osd_types.h

@@ -5572,7 +5572,12 @@ WRITE_CLASS_ENCODER(SnapSet)
 
 std::ostream& operator<<(std::ostream& out, const SnapSet& cs);
 
-
+inline static const bool should_whiteout(
+  const SnapSet &ss,
+  const SnapContext &client_snapc) {
+  return !ss.clones.empty() ||
+    (!client_snapc.snaps.empty() && client_snapc.snaps[0] > ss.seq);
+}
 
 #define OI_ATTR "_"
 #define SS_ATTR "snapset"