From 84539d19b0ed480e8a05b4ca583f02860bd5deab Mon Sep 17 00:00:00 2001
From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Date: Thu, 26 Jun 2025 16:48:00 +0530
Subject: [PATCH] librbd/api: fix mirror group disable

Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
---
 src/librbd/api/Mirror.cc | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/librbd/api/Mirror.cc b/src/librbd/api/Mirror.cc
index 8f236747d22..58d0d8e0caa 100644
--- a/src/librbd/api/Mirror.cc
+++ b/src/librbd/api/Mirror.cc
@@ -3012,8 +3012,8 @@ int Mirror<I>::group_disable(IoCtx& group_ioctx, const char *group_name,
   request->send();
   r = ctx.wait();
   if (r == -ENOENT) {
-    ldout(cct, 10) << "mirroring for group " << group_name
-                   << " already disabled" << dendl;
+    ldout(cct, 10) << "ignoring disable command: mirroring is not enabled for "
+                   << "this group: " << group_name << dendl;
     return 0;
   } else if (r < 0) {
     lderr(cct) << "failed to get mirror group info: "
@@ -3044,13 +3044,13 @@ int Mirror<I>::group_disable(IoCtx& group_ioctx, const char *group_name,
   }
 
   int ret_code = 0;
-  for (auto image_ctx : image_ctxs) {
-    ldout(cct, 10) << "attempting to disable image with id " << image_ctx->id
-                   << ": " << cpp_strerror(r) << dendl;
-    r = image_disable(image_ctx, force, true);
+  for (size_t i = 0; i < image_ctxs.size(); i++) {
+    ldout(cct, 10) << "attempting to disable image with id "
+                   << image_ctxs[i]->id << ": " << cpp_strerror(r) << dendl;
+    r = image_disable(image_ctxs[i], force, true);
     if (r < 0) {
-      lderr(cct) << "failed to disable mirroring on image: " << image_ctx->name
-                 << cpp_strerror(r) << dendl;
+      lderr(cct) << "failed to disable mirroring on image: "
+                 << image_ctxs[i]->name << cpp_strerror(r) << dendl;
       if (ret_code == 0) {
         ret_code = r;
       }
@@ -3063,10 +3063,10 @@ int Mirror<I>::group_disable(IoCtx& group_ioctx, const char *group_name,
 
   // undo an image disable might not be of our interest. If needed, user must
   // issue the same command again.
-  if (r < 0) {
+  if (ret_code < 0) {
     lderr(cct) << "failed to disable one or more images: "
-               << cpp_strerror(r) << dendl;
-    return r;
+               << cpp_strerror(ret_code) << dendl;
+    return ret_code;
   }
 
   std::vector<cls::rbd::GroupSnapshot> snaps;
@@ -3077,22 +3077,23 @@ int Mirror<I>::group_disable(IoCtx& group_ioctx, const char *group_name,
   req->send();
   r = cond.wait();
   if (r < 0) {
-    lderr(cct) << "failed to list group snapshots: "
+    lderr(cct) << "failed to list group snapshots, retry later: "
                << cpp_strerror(r) << dendl;
-    // ignore
+    return r;
   }
 
+  std::string group_header_oid = librbd::util::group_header_name(group_id);
   for (auto &snap : snaps) {
     auto ns = std::get_if<cls::rbd::GroupSnapshotNamespaceMirror>(
         &snap.snapshot_namespace);
     if (ns == nullptr) {
       continue;
     }
-    r = util::group_snap_remove(group_ioctx, group_id, snap);
+    r = cls_client::group_snap_remove(&group_ioctx, group_header_oid, snap.id);
     if (r < 0) {
       lderr(cct) << "failed to remove group snapshot metadata: "
                  << cpp_strerror(r) << dendl;
-      // ignore
+      return r;
     }
   }
 
-- 
2.39.5