From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Date: Tue, 16 Dec 2025 12:53:18 +0000 (+0530)
Subject: cleanup: simplify group image snapshot validation
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=56e17170f286ba5f9bf4eb10162fb777c9255352;p=ceph-ci.git

cleanup: simplify group image snapshot validation

validation of an image snapshot association with a group snapshot requires
checking either a valid group_spec or the presence of a group_snap_id
in cls::rbd::MirrorSnapshotNamespace.

this commit tries to remove redundant validation of checking for both and rely
on this minimal condition of checking for a valid group_spec.

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
---

diff --git a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
index 42940f3aaa8..3af262898ef 100644
--- a/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
+++ b/src/librbd/mirror/snapshot/CreatePrimaryRequest.cc
@@ -209,7 +209,7 @@ void CreatePrimaryRequest<I>::unlink_peer() {
         if (info->mirror_peer_uuids.empty() ||
             (info->mirror_peer_uuids.count(peer) != 0 &&
              info->is_primary() && !info->complete)) {
-          if (info->group_spec.is_valid() || !info->group_snap_id.empty()) {
+          if (info->group_spec.is_valid()) {
             // snap is part of a group snap
             continue;
           }
@@ -239,7 +239,7 @@ void CreatePrimaryRequest<I>::unlink_peer() {
           // snapshot is not linked with this peer
           continue;
         }
-        if (info->group_spec.is_valid() || !info->group_snap_id.empty()) {
+        if (info->group_spec.is_valid()) {
           // snap is part of a group snap
           continue;
         }
diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc
index a3c4444a3ca..f951f88b56e 100644
--- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc
+++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc
@@ -504,8 +504,7 @@ void Replayer<I>::scan_local_mirror_snapshots(
         ceph_assert(m_local_snap_id_end == CEPH_NOSNAP);
 
         if (mirror_ns->mirror_peer_uuids.empty() &&
-            (!mirror_ns->group_spec.is_valid() &&
-             mirror_ns->group_snap_id.empty())) {
+            !mirror_ns->group_spec.is_valid()) {
           // no other peer will attempt to sync to this snapshot so store as
           // a candidate for removal
           m_prune_snap_ids.insert(local_snap_id);
@@ -648,8 +647,7 @@ void Replayer<I>::scan_remote_mirror_snapshots(
                       m_state_builder->remote_mirror_uuid);
 
         if (m_remote_snap_id_end == CEPH_NOSNAP &&
-            (!mirror_ns->group_spec.is_valid() &&
-             mirror_ns->group_snap_id.empty())) {
+            !mirror_ns->group_spec.is_valid()) {
           // haven't found the end snap so treat this as a candidate for unlink
           unlink_snap_ids.insert(remote_snap_id);
         }
@@ -751,8 +749,7 @@ void Replayer<I>::scan_remote_mirror_snapshots(
           get_local_image_state();
         } else {
           // Standalone image mirroring
-          if (!m_remote_mirror_snap_ns.group_spec.is_valid() &&
-              m_remote_mirror_snap_ns.group_snap_id.empty()) {
+          if (!m_remote_mirror_snap_ns.group_spec.is_valid()) {
             copy_snapshots();
             return;
           }
@@ -800,8 +797,7 @@ void Replayer<I>::scan_remote_mirror_snapshots(
     handle_replay_complete(locker, -EEXIST, "split-brain");
     return;
   } else if (remote_demoted &&
-      (!m_remote_mirror_snap_ns.group_spec.is_valid() &&
-       m_remote_mirror_snap_ns.group_snap_id.empty())) {
+             !m_remote_mirror_snap_ns.group_spec.is_valid()) {
     dout(10) << "remote image demoted" << dendl;
     handle_replay_complete(locker, -EREMOTEIO, "remote image demoted");
     return;
@@ -1343,8 +1339,7 @@ void Replayer<I>::handle_notify_image_update(int r) {
         cls::rbd::MirrorSnapshotNamespace>(&snap_ns);
       if (mirror_ns == nullptr || !mirror_ns->complete) {
         continue;
-      } else if (mirror_ns->group_spec.is_valid() ||
-          !mirror_ns->group_snap_id.empty()) {
+      } else if (mirror_ns->group_spec.is_valid()) {
         unlink = false;
       }
       break;