librbd: clean up remove request state machine

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc
index 885e280a8ae0d2b09124b74a0eb5409ffdbf2356..48da5221e88459ce43c00d83dd975b191ccb34ca 100644 (file)
--- a/src/librbd/image/RemoveRequest.cc
+++ b/src/librbd/image/RemoveRequest.cc
@@ -17,7 +17,7 @@
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
 #define dout_prefix *_dout << "librbd::image::RemoveRequest: " << this << " " \
-                           << __func__ << " "
+                           << __func__ << ": "
 
 namespace librbd {
 namespace image {
@@ -62,20 +62,21 @@ void RemoveRequest<I>::open_image() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_open_image(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_open_image(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result < 0) {
+  if (r < 0) {
     m_image_ctx->destroy();
     m_image_ctx = nullptr;
 
-    if (*result != -ENOENT) {
-      lderr(m_cct) << "error opening image: " << cpp_strerror(*result) << dendl;
-      return m_on_finish;
+    if (r != -ENOENT) {
+      lderr(m_cct) << "error opening image: " << cpp_strerror(r) << dendl;
+      finish(r);
+      return;
     }
 
     remove_image();
-    return nullptr;
+    return;
   }
 
   m_image_id = m_image_ctx->id;
@@ -85,7 +86,6 @@ Context *RemoveRequest<I>::handle_open_image(int *result) {
   m_unknown_format = false;
 
   check_exclusive_lock();
-  return nullptr;
 }
 
 template<typename I>
@@ -118,36 +118,34 @@ void RemoveRequest<I>::acquire_exclusive_lock() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_exclusive_lock_force(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_exclusive_lock_force(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
   delete m_exclusive_lock;
   m_exclusive_lock = nullptr;
 
-  if (*result < 0) {
+  if (r < 0) {
     lderr(m_cct) << "error shutting down exclusive lock: "
-                 << cpp_strerror(*result) << dendl;
-    send_close_image(*result);
-    return nullptr;
+                 << cpp_strerror(r) << dendl;
+    send_close_image(r);
+    return;
   }
 
   assert(m_image_ctx->exclusive_lock == nullptr);
   validate_image_removal();
-  return nullptr;
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_exclusive_lock(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_exclusive_lock(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if ((*result < 0) || !m_image_ctx->exclusive_lock->is_lock_owner()) {
+  if (r < 0 || !m_image_ctx->exclusive_lock->is_lock_owner()) {
     lderr(m_cct) << "cannot obtain exclusive lock - not removing" << dendl;
     send_close_image(-EBUSY);
-    return nullptr;
+    return;
   }
 
   validate_image_removal();
-  return nullptr;
 }
 
 template<typename I>
@@ -229,14 +227,14 @@ void RemoveRequest<I>::filter_out_mirror_watchers() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_check_image_watchers(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_check_image_watchers(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result < 0) {
-    lderr(m_cct) << "error listing image watchers: " << cpp_strerror(*result)
+  if (r < 0) {
+    lderr(m_cct) << "error listing image watchers: " << cpp_strerror(r)
                  << dendl;
-    send_close_image(*result);
-    return nullptr;
+    send_close_image(r);
+    return;
   }
 
   // If an image is being bootstrapped by rbd-mirror, it implies
@@ -247,11 +245,10 @@ Context *RemoveRequest<I>::handle_check_image_watchers(int *result) {
   if (m_watchers.size() > 1) {
     lderr(m_cct) << "image has watchers - not removing" << dendl;
     send_close_image(-EBUSY);
-    return nullptr;
+    return;
   }
 
   check_group();
-  return nullptr;
 }
 
 template<typename I>
@@ -272,29 +269,28 @@ void RemoveRequest<I>::check_group() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_check_group(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_check_group(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
   cls::rbd::GroupSpec s;
-  if (*result == 0) {
+  if (r == 0) {
     bufferlist::iterator it = m_out_bl.begin();
-    *result = librbd::cls_client::image_get_group_finish(&it, &s);
+    r = librbd::cls_client::image_get_group_finish(&it, &s);
   }
-  if (*result < 0 && *result != -EOPNOTSUPP) {
+  if (r < 0 && r != -EOPNOTSUPP) {
     lderr(m_cct) << "error fetching group for image: "
-                 << cpp_strerror(*result) << dendl;
-    send_close_image(*result);
-    return nullptr;
+                 << cpp_strerror(r) << dendl;
+    send_close_image(r);
+    return;
   }
 
   if (s.is_valid()) {
     lderr(m_cct) << "image is in a group - not removing" << dendl;
     send_close_image(-EMLINK);
-    return nullptr;
+    return;
   }
 
   trim_image();
-  return nullptr;
 }
 
 template<typename I>
@@ -313,21 +309,20 @@ void RemoveRequest<I>::trim_image() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_trim_image(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_trim_image(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result < 0) {
+  if (r < 0) {
     lderr(m_cct) << "warning: failed to remove some object(s): "
-                 << cpp_strerror(*result) << dendl;
+                 << cpp_strerror(r) << dendl;
   }
 
   if (m_old_format) {
-    send_close_image(*result);
-    return nullptr;
+    send_close_image(r);
+    return;
   }
 
   remove_child();
-  return nullptr;
 }
 
 template<typename I>
@@ -350,21 +345,20 @@ void RemoveRequest<I>::remove_child() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_remove_child(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_remove_child(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result == -ENOENT) {
-    *result = 0;
-  } else if (*result < 0) {
+  if (r == -ENOENT) {
+    r = 0;
+  } else if (r < 0) {
     lderr(m_cct) << "error removing child from children list: "
-                 << cpp_strerror(*result) << dendl;
-    send_close_image(*result);
-    return nullptr;
+                 << cpp_strerror(r) << dendl;
+    send_close_image(r);
+    return;
   }
 
 
   send_disable_mirror();
-  return nullptr;
 }
 
 template<typename I>
@@ -381,18 +375,17 @@ void RemoveRequest<I>::send_disable_mirror() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_disable_mirror(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_disable_mirror(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result == -EOPNOTSUPP) {
-    *result = 0;
-  } else if (*result < 0) {
+  if (r == -EOPNOTSUPP) {
+    r = 0;
+  } else if (r < 0) {
     lderr(m_cct) << "error disabling image mirroring: "
-                 << cpp_strerror(*result) << dendl;
+                 << cpp_strerror(r) << dendl;
   }
 
-  send_close_image(*result);
-  return nullptr;
+  send_close_image(r);
 }
 
 template<typename I>
@@ -408,23 +401,23 @@ void RemoveRequest<I>::send_close_image(int r) {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_send_close_image(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_send_close_image(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result < 0) {
+  if (r < 0) {
     lderr(m_cct) << "error encountered while closing image: "
-                 << cpp_strerror(*result) << dendl;
+                 << cpp_strerror(r) << dendl;
   }
 
   m_image_ctx->destroy();
   m_image_ctx = nullptr;
   if (m_ret_val < 0) {
-    *result = m_ret_val;
-    return m_on_finish;
+    r = m_ret_val;
+    finish(r);
+    return;
   }
 
   remove_header();
-  return nullptr;
 }
 
 template<typename I>
@@ -440,16 +433,15 @@ void RemoveRequest<I>::remove_header() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_remove_header(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_remove_header(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if ((*result < 0) && (*result != -ENOENT)) {
-    lderr(m_cct) << "error removing header: " << cpp_strerror(*result) << dendl;
-    m_ret_val = *result;
+  if (r < 0 && r != -ENOENT) {
+    lderr(m_cct) << "error removing header: " << cpp_strerror(r) << dendl;
+    m_ret_val = r;
   }
 
   remove_image();
-  return nullptr;
 }
 
 template<typename I>
@@ -469,16 +461,16 @@ void RemoveRequest<I>::remove_header_v2() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_remove_header_v2(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_remove_header_v2(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if ((*result < 0) && (*result != -ENOENT)) {
-    lderr(m_cct) << "error removing header: " << cpp_strerror(*result) << dendl;
-    return m_on_finish;
+  if (r < 0 && r != -ENOENT) {
+    lderr(m_cct) << "error removing header: " << cpp_strerror(r) << dendl;
+    finish(r);
+    return;
   }
 
   send_journal_remove();
-  return nullptr;
 }
 
 template<typename I>
@@ -495,19 +487,19 @@ void RemoveRequest<I>::send_journal_remove() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_journal_remove(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_journal_remove(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result < 0 && *result != -ENOENT) {
-    lderr(m_cct) << "failed to remove image journal: " << cpp_strerror(*result)
+  if (r < 0 && r != -ENOENT) {
+    lderr(m_cct) << "failed to remove image journal: " << cpp_strerror(r)
                  << dendl;
-    return m_on_finish;
+    finish(r);
+    return;
   } else {
-    *result = 0;
+    r = 0;
   }
 
   send_object_map_remove();
-  return nullptr;
 }
 
 template<typename I>
@@ -526,19 +518,19 @@ void RemoveRequest<I>::send_object_map_remove() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_object_map_remove(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_object_map_remove(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result < 0 && *result != -ENOENT) {
-    lderr(m_cct) << "failed to remove image journal: " << cpp_strerror(*result)
+  if (r < 0 && r != -ENOENT) {
+    lderr(m_cct) << "failed to remove image journal: " << cpp_strerror(r)
                  << dendl;
-    return m_on_finish;
+    finish(r);
+    return;
   } else {
-    *result = 0;
+    r = 0;
   }
 
   mirror_image_remove();
-  return nullptr;
 }
 
 template<typename I>
@@ -557,27 +549,24 @@ void RemoveRequest<I>::mirror_image_remove() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_mirror_image_remove(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_mirror_image_remove(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (*result < 0 && *result != -ENOENT && *result != -EOPNOTSUPP) {
+  if (r < 0 && r != -ENOENT && r != -EOPNOTSUPP) {
     lderr(m_cct) << "failed to remove mirror image state: "
-                 << cpp_strerror(*result) << dendl;
-    return m_on_finish;
-  } else {
-    *result = 0;
+                 << cpp_strerror(r) << dendl;
+    finish(r);
+    return;
   }
 
   if (m_from_trash_remove) {
     // both the id object and the directory entry have been removed in
     // a previous call to trash_move.
-
-    *result = 0;
-    return m_on_finish;
+    finish(0);
+    return;
   }
 
   remove_id_object();
-  return nullptr;
 }
 
 template<typename I>
@@ -605,11 +594,11 @@ void RemoveRequest<I>::remove_v1_image() {
 
 template<typename I>
 void RemoveRequest<I>::handle_remove_v1_image(int r) {
-  ldout(m_cct, 20) << ": r=" << r << dendl;
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
   m_old_format = (r == 0);
-  if ((r == 0) || ((r < 0) && !m_unknown_format)) {
-    if ((r < 0) && (r != -ENOENT)) {
+  if (r == 0 || (r < 0 && !m_unknown_format)) {
+    if (r < 0 && r != -ENOENT) {
       lderr(m_cct) << "error removing image from v1 directory: "
                    << cpp_strerror(r) << dendl;
     }
@@ -657,25 +646,26 @@ void RemoveRequest<I>::dir_get_image_id() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_dir_get_image_id(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_dir_get_image_id(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if ((*result < 0) && (*result != -ENOENT)) {
-    lderr(m_cct) << "error fetching image id: " << cpp_strerror(*result)
+  if (r < 0 && r != -ENOENT) {
+    lderr(m_cct) << "error fetching image id: " << cpp_strerror(r)
                  << dendl;
-    return m_on_finish;
+    finish(r);
+    return;
   }
 
-  if (!*result) {
+  if (r == 0) {
     bufferlist::iterator iter = m_out_bl.begin();
-    *result = librbd::cls_client::dir_get_id_finish(&iter, &m_image_id);
-    if (*result < 0) {
-      return m_on_finish;
+    r = librbd::cls_client::dir_get_id_finish(&iter, &m_image_id);
+    if (r < 0) {
+      finish(r);
+      return;
     }
   }
 
   remove_header_v2();
-  return nullptr;
 }
 
 template<typename I>
@@ -695,25 +685,26 @@ void RemoveRequest<I>::dir_get_image_name() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_dir_get_image_name(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_dir_get_image_name(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if ((*result < 0) && (*result != -ENOENT)) {
-    lderr(m_cct) << "error fetching image name: " << cpp_strerror(*result)
+  if (r < 0 && r != -ENOENT) {
+    lderr(m_cct) << "error fetching image name: " << cpp_strerror(r)
                  << dendl;
-    return m_on_finish;
+    finish(r);
+    return;
   }
 
-  if (!*result) {
+  if (r == 0) {
     bufferlist::iterator iter = m_out_bl.begin();
-    *result = librbd::cls_client::dir_get_name_finish(&iter, &m_image_name);
-    if (*result < 0) {
-      return m_on_finish;
+    r = librbd::cls_client::dir_get_name_finish(&iter, &m_image_name);
+    if (r < 0) {
+      finish(r);
+      return;
     }
   }
 
   remove_header_v2();
-  return nullptr;
 }
 
 template<typename I>
@@ -729,17 +720,17 @@ void RemoveRequest<I>::remove_id_object() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_remove_id_object(int *result) {
-  ldout(m_cct, 20) << ": r=" << *result << dendl;
+void RemoveRequest<I>::handle_remove_id_object(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if ((*result < 0) && (*result != -ENOENT)) {
-    lderr(m_cct) << "error removing id object: " << cpp_strerror(*result)
+  if (r < 0 && r != -ENOENT) {
+    lderr(m_cct) << "error removing id object: " << cpp_strerror(r)
                  << dendl;
-    return m_on_finish;
+    finish(r);
+    return;
   }
 
   dir_remove_image();
-  return nullptr;
 }
 
 template<typename I>
@@ -758,15 +749,23 @@ void RemoveRequest<I>::dir_remove_image() {
 }
 
 template<typename I>
-Context *RemoveRequest<I>::handle_dir_remove_image(int *result) {
-  ldout(m_cct, 20) << ":r =" << *result << dendl;
+void RemoveRequest<I>::handle_dir_remove_image(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if ((*result < 0) && (*result != -ENOENT)) {
+  if (r < 0 && r != -ENOENT) {
     lderr(m_cct) << "error removing image from v2 directory: "
-                 << cpp_strerror(*result) << dendl;
+                 << cpp_strerror(r) << dendl;
   }
 
-  return m_on_finish;
+  finish(r);
+}
+
+template<typename I>
+void RemoveRequest<I>::finish(int r) {
+  ldout(m_cct, 20) << "r=" << r << dendl;
+
+  m_on_finish->complete(r);
+  delete this;
 }
 
 } // namespace image

diff --git a/src/librbd/image/RemoveRequest.h b/src/librbd/image/RemoveRequest.h
index 2a1ec3b603378907cb2a1ca547c81d2ed8b0acdf..24d94d86b9c8fa558502587aa94d3cd49bc947f2 100644 (file)
--- a/src/librbd/image/RemoveRequest.h
+++ b/src/librbd/image/RemoveRequest.h
@@ -125,50 +125,50 @@ private:
   std::list<obj_watch_t> m_watchers;
 
   void open_image();
-  Context *handle_open_image(int *result);
+  void handle_open_image(int r);
 
   void send_journal_remove();
-  Context* handle_journal_remove(int *result);
+  void handle_journal_remove(int r);
 
   void send_object_map_remove();
-  Context* handle_object_map_remove(int *result);
+  void handle_object_map_remove(int r);
 
   void mirror_image_remove();
-  Context* handle_mirror_image_remove(int *result);
+  void handle_mirror_image_remove(int r);
 
   void check_exclusive_lock();
 
   void acquire_exclusive_lock();
-  Context *handle_exclusive_lock(int *result);
-  Context *handle_exclusive_lock_force(int *result);
+  void handle_exclusive_lock(int r);
+  void handle_exclusive_lock_force(int r);
 
   void validate_image_removal();
   void check_image_snaps();
 
   void filter_out_mirror_watchers();
   void check_image_watchers();
-  Context *handle_check_image_watchers(int *result);
+  void handle_check_image_watchers(int r);
 
   void check_group();
-  Context *handle_check_group(int *result);
+  void handle_check_group(int r);
 
   void trim_image();
-  Context *handle_trim_image(int *result);
+  void handle_trim_image(int r);
 
   void remove_child();
-  Context *handle_remove_child(int *result);
+  void handle_remove_child(int r);
 
   void send_disable_mirror();
-  Context *handle_disable_mirror(int *result);
+  void handle_disable_mirror(int r);
 
   void send_close_image(int r);
-  Context *handle_send_close_image(int *result);
+  void handle_send_close_image(int r);
 
   void remove_header();
-  Context *handle_remove_header(int *result);
+  void handle_remove_header(int r);
 
   void remove_header_v2();
-  Context *handle_remove_header_v2(int *result);
+  void handle_remove_header_v2(int r);
 
   void remove_image();
 
@@ -178,16 +178,18 @@ private:
   void remove_v2_image();
 
   void dir_get_image_id();
-  Context *handle_dir_get_image_id(int *result);
+  void handle_dir_get_image_id(int r);
 
   void dir_get_image_name();
-  Context *handle_dir_get_image_name(int *result);
+  void handle_dir_get_image_name(int r);
 
   void remove_id_object();
-  Context *handle_remove_id_object(int *result);
+  void handle_remove_id_object(int r);
 
   void dir_remove_image();
-  Context *handle_dir_remove_image(int *result);
+  void handle_dir_remove_image(int r);
+
+  void finish(int r);
 };
 
 } // namespace image