From 9b7b3edcaa80abb6e1918fc8edaae4041c6e257e Mon Sep 17 00:00:00 2001
From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Date: Wed, 14 Feb 2024 20:31:21 +0530
Subject: [PATCH] rbd-mirror: address partial group snapshots case

Make sure group snapshots doesn't get copied to secondary if the group snapshot
is incomplete on primary. On creation time of a group snapshots on primary,
make sure to delete the previous snapshot in case it is incomplete.

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
---
 .../mirror/snapshot/CreatePrimaryRequest.cc   | 45 ++++++++++++++++++-
 .../mirror/snapshot/UnlinkPeerRequest.cc      |  2 +-
 src/tools/rbd_mirror/GroupReplayer.cc         |  5 +++
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
index 9db6143462367..41c9dae321eab 100644
--- a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
+++ b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
@@ -190,6 +190,7 @@ void CreatePrimaryRequest<I>::unlink_peer() {
 
   std::string peer_uuid;
   uint64_t snap_id = CEPH_NOSNAP;
+  CephContext *cct = m_image_ctx->cct;
 
   {
     std::shared_lock image_locker{m_image_ctx->image_lock};
@@ -212,6 +213,8 @@ void CreatePrimaryRequest<I>::unlink_peer() {
     for (const auto& peer : m_mirror_peer_uuids) {
       size_t count = 0;
       uint64_t unlink_snap_id = 0;
+      uint64_t prev_snap_id = 0;
+      std::string prev_group_snap_id;
       for (const auto& snap_info_pair : m_image_ctx->snap_info) {
         auto info = std::get_if<cls::rbd::MirrorSnapshotNamespace>(
           &snap_info_pair.second.snap_namespace);
@@ -223,12 +226,48 @@ void CreatePrimaryRequest<I>::unlink_peer() {
           // promotion
           count = 0;
           unlink_snap_id = 0;
+          prev_snap_id = 0;
           continue;
         }
         if (info->mirror_peer_uuids.count(peer) == 0) {
           // snapshot is not linked with this peer
           continue;
         }
+        if (prev_snap_id) {
+          librados::IoCtx m_group_io_ctx;
+          int r = librbd::util::create_ioctx(m_image_ctx->md_ctx,
+                                             "group", info->group_spec.pool_id,
+                                             {}, &m_group_io_ctx);
+          if (r < 0) {
+            return;
+          }
+          cls::rbd::GroupSnapshot prev_group_snap;
+          std::string group_header_oid = librbd::util::group_header_name(
+                                                 info->group_spec.group_id);
+          r = cls_client::group_snap_get_by_id(&m_group_io_ctx,
+                                               group_header_oid,
+                                               prev_group_snap_id,
+                                               &prev_group_snap);
+          if (r < 0) {
+            lderr(cct) << "failed to retrieve group snapshot: "
+                       << cpp_strerror(r) << dendl;
+            prev_snap_id = snap_info_pair.first;
+            prev_group_snap_id = info->group_snap_id;
+            continue;
+          }
+          if (prev_group_snap.state != cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE) {
+            peer_uuid = peer;
+            snap_id = prev_snap_id;
+            r = cls_client::group_snap_remove(&m_group_io_ctx,
+                                              group_header_oid,
+                                              prev_group_snap.id);
+            if (r < 0) {
+              lderr(cct) << "failed to remove group snapshot metadata: "
+                         << cpp_strerror(r) << dendl;
+            }
+            goto do_unlink;
+          }
+        }
         count++;
         if (count == max_snapshots) {
           unlink_snap_id = snap_info_pair.first;
@@ -238,6 +277,11 @@ void CreatePrimaryRequest<I>::unlink_peer() {
           snap_id = unlink_snap_id;
           goto do_unlink;
         }
+        prev_snap_id = 0;
+        if (info->group_spec.is_valid() && !info->group_snap_id.empty()) {
+          prev_snap_id = snap_info_pair.first;
+          prev_group_snap_id = info->group_snap_id;
+        }
       }
     }
   }
@@ -246,7 +290,6 @@ void CreatePrimaryRequest<I>::unlink_peer() {
   return;
 
 do_unlink:
-  CephContext *cct = m_image_ctx->cct;
   ldout(cct, 15) << "peer=" << peer_uuid << ", snap_id=" << snap_id << dendl;
 
   auto ctx = create_context_callback<
diff --git a/src/librbd/mirror/snapshot/UnlinkPeerRequest.cc b/src/librbd/mirror/snapshot/UnlinkPeerRequest.cc
index dc596e5175ce9..adb431f0f1320 100644
--- a/src/librbd/mirror/snapshot/UnlinkPeerRequest.cc
+++ b/src/librbd/mirror/snapshot/UnlinkPeerRequest.cc
@@ -192,7 +192,7 @@ void UnlinkPeerRequest<I>::unlink_group_snapshot(
     const cls::rbd::SnapshotNamespace& snap_namespace,
     const std::string& snap_name) {
   CephContext *cct = m_image_ctx->cct;
-  ldout(cct, 15) << dendl;
+  ldout(cct, 15) << "snap_name: " << snap_name << dendl;
 
   auto info = std::get_if<cls::rbd::MirrorSnapshotNamespace>(&snap_namespace);
   if (!info->group_spec.is_valid()) {
diff --git a/src/tools/rbd_mirror/GroupReplayer.cc b/src/tools/rbd_mirror/GroupReplayer.cc
index 59d474f3c2d3a..a8857e7e3f201 100644
--- a/src/tools/rbd_mirror/GroupReplayer.cc
+++ b/src/tools/rbd_mirror/GroupReplayer.cc
@@ -973,8 +973,13 @@ void GroupReplayer<I>::handle_get_remote_group_snapshot(
         &iter, &remote_group_snap);
   }
 
+  bool complete = (remote_group_snap.state == cls::rbd::GROUP_SNAPSHOT_STATE_COMPLETE);
   if (r < 0) {
     derr << "failed to get remote group snapshot: " << cpp_strerror(r) << dendl;
+  } else if (!complete) {
+    derr << "incomplete remote group snapshot: " << remote_group_snap_id
+         << dendl;
+    r = -EAGAIN;
   } else {
     m_local_group_snaps[remote_group_snap_id].name = remote_group_snap.name;
   }
-- 
2.39.5