From e3cd2c756f20135e48b4ce8bad7b769f48c6008d Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 24 Jan 2018 12:38:50 -0500 Subject: [PATCH] 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 --- src/librbd/image/CloneRequest.cc | 39 ++-------------- src/librbd/image/CloneRequest.h | 7 +-- .../librbd/image/test_mock_CloneRequest.cc | 46 ------------------- 3 files changed, 7 insertions(+), 85 deletions(-) diff --git a/src/librbd/image/CloneRequest.cc b/src/librbd/image/CloneRequest.cc index abf3b86fd07..f0f850e1e79 100644 --- a/src/librbd/image/CloneRequest.cc +++ b/src/librbd/image/CloneRequest.cc @@ -315,7 +315,7 @@ void CloneRequest::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::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::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::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::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 @@ -513,33 +511,6 @@ void CloneRequest::handle_close(int r) { } } -template -void CloneRequest::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; - librados::AioCompletion *comp = - create_rados_callback(this); - int r = m_ioctx.aio_operate(RBD_CHILDREN, comp, &op); - assert(r == 0); - comp->release(); -} - -template -void CloneRequest::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 void CloneRequest::send_remove() { ldout(m_cct, 20) << this << " " << __func__ << dendl; diff --git a/src/librbd/image/CloneRequest.h b/src/librbd/image/CloneRequest.h index 851a2021e16..75b294d90d1 100644 --- 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 71b53e4aeda..8a0f543dff3 100644 --- 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); -- 2.39.5