From: VinayBhaskar-V <vvarada@redhat.com>
Date: Thu, 15 May 2025 14:18:30 +0000 (+0530)
Subject: librbd: retry list_snap_orders() once instead of failing sort_snaps()
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=433df92ff66bd0e7ebf3b8f66ddd9df43a8687d5;p=ceph.git

librbd: retry list_snap_orders() once instead of failing sort_snaps()

If snapshot listing races with snapshot creation, rbd group snap ls
may spuriously return an unsorted listing and internal fail_if_not_sorted=true
consumers may generate a spurious EINVAL error. This is because snap orders
are listed before snaps themselves and the "missing order for snap" check in
sort_snaps() is driven by the list of snaps.
This can be improved by grabbing snap order keys one more time, adding the newly
discovered snap orders to m_snap_orders and retrying instead of immediately failing the sort.

Fixes: https://tracker.ceph.com/issues/67984
Signed-off-by: VinayBhaskar-V <vvarada@redhat.com>
(cherry picked from commit 3c21aec85e986f5206e83fcdcf5b1e0441b35f56)
---

diff --git a/src/librbd/group/ListSnapshotsRequest.cc b/src/librbd/group/ListSnapshotsRequest.cc
index bbd3759ff2b4c..4765f373881fe 100644
--- a/src/librbd/group/ListSnapshotsRequest.cc
+++ b/src/librbd/group/ListSnapshotsRequest.cc
@@ -92,7 +92,11 @@ void ListSnapshotsRequest<I>::handle_list_snap_orders(int r) {
 
   m_snap_orders.insert(snap_orders.begin(), snap_orders.end());
   if (snap_orders.size() < MAX_RETURN) {
-    list_snaps();
+    if (m_retried_snap_orders) {
+      sort_snaps();
+    } else {
+      list_snaps();
+    }
     return;
   }
 
@@ -158,7 +162,18 @@ void ListSnapshotsRequest<I>::sort_snaps() {
   for (const auto& snap : *m_snaps) {
     if (m_snap_orders.find(snap.id) == m_snap_orders.end()) {
       ldout(cct, 10) << "Missing order for snap_id=" << snap.id << dendl;
-      finish(m_fail_if_not_sorted ? -EINVAL : 0);
+      if (m_fail_if_not_sorted) {
+        if (!m_retried_snap_orders) {
+          ldout(cct, 10) << "Retrying to fetch missing snap orders..." << dendl;
+          m_retried_snap_orders = true;
+          m_start_after_order = "";
+          list_snap_orders();
+        } else {
+          finish(-EINVAL);
+        }
+      } else {
+        finish(0);
+      }
       return;
     }
   }
diff --git a/src/librbd/group/ListSnapshotsRequest.h b/src/librbd/group/ListSnapshotsRequest.h
index 7152dd981ab7d..4c5a1d047ed28 100644
--- a/src/librbd/group/ListSnapshotsRequest.h
+++ b/src/librbd/group/ListSnapshotsRequest.h
@@ -44,16 +44,24 @@ private:
    * @verbatim
    *
    * <start>    /--------\
-   *    |       |        | (if required. repeat if more
+   *    |       |        | (if requested, repeat if more
    *    v       v        |  entries)
    *  LIST_SNAP_ORDERS --/
+   *    |
    *    |       /--------\
    *    |       |        | (repeat if more
    *    v       v        |  snapshots)
    *  LIST_SNAPS --------/
    *    |
-   *    v
-   *  SORT_SNAPS (if required)  
+   *    |                          /--------\
+   *    |/-------<--------\        |        | (repeat if more
+   *    |                 |        v        |  entries)
+   *    |                LIST_SNAP_ORDERS --/
+   *    |                 ^
+   *    |                 | (retry if ordering is required and
+   *    |                 |  an entry is missing for a snapshot)
+   *    v (if requested)  |
+   *  SORT_SNAPS ---------/
    *    |
    *    v
    *  <finish>
@@ -73,6 +81,7 @@ private:
   cls::rbd::GroupSnapshot m_start_after;
   std::string m_start_after_order;
   bufferlist m_out_bl;
+  bool m_retried_snap_orders = false;
 
   void list_snaps();
   void handle_list_snaps(int r);