From: Ilya Dryomov <idryomov@gmail.com>
Date: Tue, 30 Aug 2022 19:33:04 +0000 (+0200)
Subject: librbd: check *result consistently in RefreshRequest
X-Git-Tag: v18.0.0~80^2~4
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ce6dff89c0f005c1ae1dc71cadfbef9f82df37a4;p=ceph.git

librbd: check *result consistently in RefreshRequest

Stick to *result >= 0 checks everywhere and add missing checks for
op_features_get_finish() and image_group_get_finish() errors.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---

diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc
index 9b7138815af5..7310062bc937 100644
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -95,7 +95,7 @@ Context *RefreshRequest<I>::handle_get_migration_header(int *result) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << *result << dendl;
 
-  if (*result == 0) {
+  if (*result >= 0) {
     auto it = m_out_bl.cbegin();
     *result = cls_client::migration_get_finish(&it, &m_migration_spec);
   } else if (*result == -ENOENT) {
@@ -252,7 +252,7 @@ Context *RefreshRequest<I>::handle_v1_get_snapshots(int *result) {
 
   std::vector<std::string> snap_names;
   std::vector<uint64_t> snap_sizes;
-  if (*result == 0) {
+  if (*result >= 0) {
     auto it = m_out_bl.cbegin();
     *result = cls_client::old_snapshot_list_finish(&it, &snap_names,
                                                    &snap_sizes, &m_snapc);
@@ -305,15 +305,16 @@ Context *RefreshRequest<I>::handle_v1_get_locks(int *result) {
   ldout(cct, 10) << this << " " << __func__ << ": "
                  << "r=" << *result << dendl;
 
-  if (*result == 0) {
+  if (*result >= 0) {
     auto it = m_out_bl.cbegin();
     ClsLockType lock_type;
     *result = rados::cls::lock::get_lock_info_finish(&it, &m_lockers,
                                                      &lock_type, &m_lock_tag);
-    if (*result == 0) {
+    if (*result >= 0) {
       m_exclusive_locked = (lock_type == ClsLockType::EXCLUSIVE);
     }
   }
+
   if (*result < 0) {
     lderr(cct) << "failed to retrieve locks: " << cpp_strerror(*result)
                << dendl;
@@ -405,10 +406,10 @@ Context *RefreshRequest<I>::handle_v2_get_mutable_metadata(int *result) {
   }
 
   if (*result >= 0) {
-    ClsLockType lock_type = ClsLockType::NONE;
+    ClsLockType lock_type;
     *result = rados::cls::lock::get_lock_info_finish(&it, &m_lockers,
                                                      &lock_type, &m_lock_tag);
-    if (*result == 0) {
+    if (*result >= 0) {
       m_exclusive_locked = (lock_type == ClsLockType::EXCLUSIVE);
     }
   }
@@ -488,20 +489,20 @@ Context *RefreshRequest<I>::handle_v2_get_parent(int *result) {
 
   auto it = m_out_bl.cbegin();
   if (!m_legacy_parent) {
-    if (*result == 0) {
+    if (*result >= 0) {
       *result = cls_client::parent_get_finish(&it, &m_parent_md.spec);
     }
 
     std::optional<uint64_t> parent_overlap;
-    if (*result == 0) {
+    if (*result >= 0) {
       *result = cls_client::parent_overlap_get_finish(&it, &parent_overlap);
     }
 
-    if (*result == 0 && parent_overlap) {
+    if (*result >= 0 && parent_overlap) {
       m_parent_md.overlap = *parent_overlap;
       m_head_parent_overlap = true;
     }
-  } else if (*result == 0) {
+  } else if (*result >= 0) {
     *result = cls_client::get_parent_finish(&it, &m_parent_md.spec,
                                             &m_parent_md.overlap);
     m_head_parent_overlap = true;
@@ -512,7 +513,7 @@ Context *RefreshRequest<I>::handle_v2_get_parent(int *result) {
     m_legacy_parent = true;
     send_v2_get_parent();
     return nullptr;
-  } if (*result < 0) {
+  } else if (*result < 0) {
     lderr(cct) << "failed to retrieve parent: " << cpp_strerror(*result)
                << dendl;
     return m_on_finish;
@@ -618,10 +619,12 @@ Context *RefreshRequest<I>::handle_v2_get_op_features(int *result) {
 
   // -EOPNOTSUPP handler not required since feature bit implies OSD
   // supports the method
-  if (*result == 0) {
+  if (*result >= 0) {
     auto it = m_out_bl.cbegin();
-    cls_client::op_features_get_finish(&it, &m_op_features);
-  } else if (*result < 0) {
+    *result = cls_client::op_features_get_finish(&it, &m_op_features);
+  }
+
+  if (*result < 0) {
     lderr(cct) << "failed to retrieve op features: " << cpp_strerror(*result)
                << dendl;
     return m_on_finish;
@@ -655,10 +658,11 @@ Context *RefreshRequest<I>::handle_v2_get_group(int *result) {
   ldout(cct, 10) << this << " " << __func__ << ": "
                  << "r=" << *result << dendl;
 
-  if (*result == 0) {
+  if (*result >= 0) {
     auto it = m_out_bl.cbegin();
-    cls_client::image_group_get_finish(&it, &m_group_spec);
+    *result = cls_client::image_group_get_finish(&it, &m_group_spec);
   }
+
   if (*result < 0 && *result != -EOPNOTSUPP) {
     lderr(cct) << "failed to retrieve group: " << cpp_strerror(*result)
                << dendl;
@@ -754,14 +758,14 @@ Context *RefreshRequest<I>::handle_v2_get_snapshots(int *result) {
       *result = cls_client::snapshot_get_finish(&it, &m_snap_infos[i]);
     }
 
-    if (*result == 0) {
+    if (*result >= 0) {
       if (m_legacy_parent) {
         *result = cls_client::get_parent_finish(&it, &m_snap_parents[i].spec,
                                                 &m_snap_parents[i].overlap);
       } else {
         std::optional<uint64_t> parent_overlap;
         *result = cls_client::parent_overlap_get_finish(&it, &parent_overlap);
-        if (*result == 0 && parent_overlap && m_parent_md.spec.pool_id > -1) {
+        if (*result >= 0 && parent_overlap && m_parent_md.spec.pool_id > -1) {
           m_snap_parents[i].spec = m_parent_md.spec;
           m_snap_parents[i].overlap = *parent_overlap;
         }