librbd: removed child cleanup from failed clone

This logic is already included in the remove image state machine
and will be executed when the clone is deleted post-failure.

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

diff --git a/src/librbd/image/CloneRequest.cc b/src/librbd/image/CloneRequest.cc
index abf3b86fd07ab7c5ec65239f98477b9e674dbc74..f0f850e1e796d8039998791c4f2875a6157b0c12 100644 (file)
--- a/src/librbd/image/CloneRequest.cc
+++ b/src/librbd/image/CloneRequest.cc
@@ -315,7 +315,7 @@ void CloneRequest<I>::handle_refresh(int r) {
 
   if (r < 0 || !snap_protected) {
     m_r_saved = -EINVAL;
-    send_remove_child();
+    send_close();
     return;
   }
 
@@ -356,7 +356,7 @@ void CloneRequest<I>::handle_metadata_list(int r) {
     } else {
       lderr(m_cct) << "couldn't list metadata: " << cpp_strerror(r) << dendl;
       m_r_saved = r;
-      send_remove_child();
+      send_close();
     }
     return;
   }
@@ -400,7 +400,7 @@ void CloneRequest<I>::handle_metadata_set(int r) {
   if (r < 0) {
     lderr(m_cct) << "couldn't set metadata: " << cpp_strerror(r) << dendl;
     m_r_saved = r;
-    send_remove_child();
+    send_close();
   } else {
     get_mirror_mode();
   }
@@ -441,7 +441,7 @@ void CloneRequest<I>::handle_get_mirror_mode(int r) {
                  << dendl;
 
     m_r_saved = r;
-    send_remove_child();
+    send_close();
   } else {
     if (m_mirror_mode == cls::rbd::MIRROR_MODE_POOL ||
        !m_non_primary_global_image_id.empty()) {
@@ -474,10 +474,8 @@ void CloneRequest<I>::handle_enable_mirror(int r) {
     lderr(m_cct) << "failed to enable mirroring: " << cpp_strerror(r)
                << dendl;
     m_r_saved = r;
-    send_remove_child();
-  } else {
-    send_close();
   }
+  send_close();
 }
 
 template <typename I>
@@ -513,33 +511,6 @@ void CloneRequest<I>::handle_close(int r) {
   }
 }
 
-template <typename I>
-void CloneRequest<I>::send_remove_child() {
-  ldout(m_cct, 20) << this << " " << __func__ << dendl;
-
-  librados::ObjectWriteOperation op;
-  cls_client::remove_child(&op, m_pspec, m_id);
-
-  using klass = CloneRequest<I>;
-  librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_remove_child>(this);
-  int r = m_ioctx.aio_operate(RBD_CHILDREN, comp, &op);
-  assert(r == 0);
-  comp->release();
-}
-
-template <typename I>
-void CloneRequest<I>::handle_remove_child(int r) {
-  ldout(m_cct, 20) << this << " " << __func__ << " r=" << r << dendl;
-
-  if (r < 0) {
-     lderr(m_cct) << "Error removing failed clone from list of children: "
-                 << cpp_strerror(r) << dendl;
-  }
-
-  send_close();
-}
-
 template <typename I>
 void CloneRequest<I>::send_remove() {
   ldout(m_cct, 20) << this << " " << __func__ << dendl;

diff --git a/src/librbd/image/CloneRequest.h b/src/librbd/image/CloneRequest.h
index 851a2021e16e9bb2e59bf6db7693ebaf2bc6b607..75b294d90d1603bf00633a3a31027e6c5d6ec0cc 100644 (file)
--- a/src/librbd/image/CloneRequest.h
+++ b/src/librbd/image/CloneRequest.h
@@ -62,8 +62,8 @@ private:
    * |               | \              /  |
    * |               |  *<-----------/   v
    * |               |                REFRESH
-   * |               |                /  |                     
-   * |   CLEAN DIR_CHILDREN <--------/   v            (meta is empty)
+   * |               |                   |
+   * |               |                   v            (meta is empty)
    * |               |\         GET META IN PARENT  . . . . . . .
    * |               | \              /  |                      .
    * v               |  *<-----------/   v     (journaling disabled)           .
@@ -148,9 +148,6 @@ private:
   void send_remove();
   void handle_remove(int r);
 
-  void send_remove_child();
-  void handle_remove_child(int r);
-
   void complete(int r);
 };
 

diff --git a/src/test/librbd/image/test_mock_CloneRequest.cc b/src/test/librbd/image/test_mock_CloneRequest.cc
index 71b53e4aeda01ed54cf77c57d979729dfd45b951..8a0f543dff33323f4547db2b7902b07c041ad4ab 100644 (file)
--- a/src/test/librbd/image/test_mock_CloneRequest.cc
+++ b/src/test/librbd/image/test_mock_CloneRequest.cc
@@ -296,12 +296,6 @@ public:
     EXPECT_CALL(mock_image_ctx, destroy());
   }
 
-  void expect_remove_child(librados::IoCtx& io_ctx, int r) {
-    EXPECT_CALL(get_mock_io_ctx(io_ctx),
-                exec(RBD_CHILDREN, _, StrEq("rbd"), StrEq("remove_child"), _, _, _))
-      .WillOnce(Return(r));
-  }
-
   void expect_remove(MockRemoveRequest& mock_remove_request, int r) {
     EXPECT_CALL(mock_remove_request, send())
       .WillOnce(Invoke([this, &mock_remove_request, r]() {
@@ -485,7 +479,6 @@ TEST_F(TestMockImageCloneRequest, RefreshError) {
   MockRefreshRequest mock_refresh_request;
   expect_refresh(mock_refresh_request, -EINVAL);
 
-  expect_remove_child(m_ioctx, 0);
   expect_close(mock_image_ctx, 0);
 
   MockRemoveRequest mock_remove_request;
@@ -523,7 +516,6 @@ TEST_F(TestMockImageCloneRequest, MetadataListError) {
 
   expect_metadata_list(mock_image_ctx, {{"key", {}}}, -EINVAL);
 
-  expect_remove_child(m_ioctx, 0);
   expect_close(mock_image_ctx, 0);
 
   MockRemoveRequest mock_remove_request;
@@ -562,7 +554,6 @@ TEST_F(TestMockImageCloneRequest, MetadataSetError) {
   expect_metadata_list(mock_image_ctx, {{"key", {}}}, 0);
   expect_metadata_set(m_ioctx, mock_image_ctx, {{"key", {}}}, -EINVAL);
 
-  expect_remove_child(m_ioctx, 0);
   expect_close(mock_image_ctx, 0);
 
   MockRemoveRequest mock_remove_request;
@@ -603,7 +594,6 @@ TEST_F(TestMockImageCloneRequest, GetMirrorModeError) {
   expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING, true);
   expect_mirror_mode_get(mock_image_ctx, cls::rbd::MIRROR_MODE_POOL, -EINVAL);
 
-  expect_remove_child(m_ioctx, 0);
   expect_close(mock_image_ctx, 0);
 
   MockRemoveRequest mock_remove_request;
@@ -647,7 +637,6 @@ TEST_F(TestMockImageCloneRequest, MirrorEnableError) {
   MockMirrorEnableRequest mock_mirror_enable_request;
   expect_mirror_enable(mock_mirror_enable_request, -EINVAL);
 
-  expect_remove_child(m_ioctx, 0);
   expect_close(mock_image_ctx, 0);
 
   MockRemoveRequest mock_remove_request;
@@ -697,41 +686,6 @@ TEST_F(TestMockImageCloneRequest, CloseError) {
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
 
-TEST_F(TestMockImageCloneRequest, RemoveChildError) {
-  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
-
-  MockTestImageCtx mock_image_ctx(*image_ctx);
-  expect_op_work_queue(mock_image_ctx);
-
-  InSequence seq;
-  expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123);
-  expect_is_snap_protected(mock_image_ctx, true, 0);
-
-  MockCreateRequest mock_create_request;
-  expect_create(mock_create_request, 0);
-
-  expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
-  expect_add_child(m_ioctx, 0);
-
-  MockRefreshRequest mock_refresh_request;
-  expect_refresh(mock_refresh_request, -EINVAL);
-
-  expect_remove_child(m_ioctx, -EPERM);
-  expect_close(mock_image_ctx, 0);
-
-  MockRemoveRequest mock_remove_request;
-  expect_remove(mock_remove_request, 0);
-
-  C_SaferCond ctx;
-  ImageOptions clone_opts;
-  auto req = new MockCloneRequest(&mock_image_ctx, m_ioctx, "clone name",
-                                  "clone id", clone_opts, "", "",
-                                  image_ctx->op_work_queue, &ctx);
-  req->send();
-  ASSERT_EQ(-EINVAL, ctx.wait());
-}
-
 TEST_F(TestMockImageCloneRequest, RemoveError) {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);