From 1cadc397bec4ba274e3e4d33173e82be7cc95518 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 11 Sep 2019 15:28:28 -0400 Subject: [PATCH] rbd-mirror: renamed RemoveRequest state machine to TrashRemoveRequest This better matches the current behavior where the images are only removed from the trash. Signed-off-by: Jason Dillaman (cherry picked from commit 55daa8e1e28f457a897070adfabf1583093aadd3) Conflicts: src/test/rbd_mirror/CMakeLists.txt: trivial resolution src/tools/rbd_mirror/ImageDeleter.cc: trivial resolution --- src/test/rbd_mirror/CMakeLists.txt | 2 +- ...est.cc => test_mock_TrashRemoveRequest.cc} | 62 ++++++++++--------- src/tools/rbd_mirror/CMakeLists.txt | 2 +- src/tools/rbd_mirror/ImageDeleter.cc | 5 +- ...RemoveRequest.cc => TrashRemoveRequest.cc} | 31 +++++----- .../{RemoveRequest.h => TrashRemoveRequest.h} | 29 ++++----- 6 files changed, 68 insertions(+), 63 deletions(-) rename src/test/rbd_mirror/image_deleter/{test_mock_RemoveRequest.cc => test_mock_TrashRemoveRequest.cc} (75%) rename src/tools/rbd_mirror/image_deleter/{RemoveRequest.cc => TrashRemoveRequest.cc} (78%) rename src/tools/rbd_mirror/image_deleter/{RemoveRequest.h => TrashRemoveRequest.h} (60%) diff --git a/src/test/rbd_mirror/CMakeLists.txt b/src/test/rbd_mirror/CMakeLists.txt index 2af8680b2f2d2..1d8a975767399 100644 --- a/src/test/rbd_mirror/CMakeLists.txt +++ b/src/test/rbd_mirror/CMakeLists.txt @@ -27,9 +27,9 @@ add_executable(unittest_rbd_mirror test_mock_LeaderWatcher.cc test_mock_PoolReplayer.cc test_mock_PoolWatcher.cc - image_deleter/test_mock_RemoveRequest.cc image_deleter/test_mock_SnapshotPurgeRequest.cc image_deleter/test_mock_TrashMoveRequest.cc + image_deleter/test_mock_TrashRemoveRequest.cc image_deleter/test_mock_TrashWatcher.cc image_replayer/test_mock_BootstrapRequest.cc image_replayer/test_mock_CreateImageRequest.cc diff --git a/src/test/rbd_mirror/image_deleter/test_mock_RemoveRequest.cc b/src/test/rbd_mirror/image_deleter/test_mock_TrashRemoveRequest.cc similarity index 75% rename from src/test/rbd_mirror/image_deleter/test_mock_RemoveRequest.cc rename to src/test/rbd_mirror/image_deleter/test_mock_TrashRemoveRequest.cc index 66d613caa3dd0..0da7988394aed 100644 --- a/src/test/rbd_mirror/image_deleter/test_mock_RemoveRequest.cc +++ b/src/test/rbd_mirror/image_deleter/test_mock_TrashRemoveRequest.cc @@ -7,8 +7,8 @@ #include "librbd/Utils.h" #include "librbd/image/RemoveRequest.h" #include "tools/rbd_mirror/Threads.h" -#include "tools/rbd_mirror/image_deleter/RemoveRequest.h" #include "tools/rbd_mirror/image_deleter/SnapshotPurgeRequest.h" +#include "tools/rbd_mirror/image_deleter/TrashRemoveRequest.h" #include "test/librados_test_stub/MockTestMemIoCtxImpl.h" #include "test/librbd/mock/MockImageCtx.h" @@ -93,7 +93,7 @@ SnapshotPurgeRequest* SnapshotPurgeRequest MockRemoveRequest; + typedef TrashRemoveRequest MockTrashRemoveRequest; typedef SnapshotPurgeRequest MockSnapshotPurgeRequest; typedef librbd::image::RemoveRequest MockImageRemoveRequest; @@ -132,7 +132,8 @@ public: EXPECT_CALL(snapshot_purge_request, construct(image_id)); EXPECT_CALL(snapshot_purge_request, send()) .WillOnce(Invoke([this, &snapshot_purge_request, r]() { - m_threads->work_queue->queue(snapshot_purge_request.on_finish, r); + m_threads->work_queue->queue( + snapshot_purge_request.on_finish, r); })); } @@ -141,12 +142,13 @@ public: EXPECT_CALL(image_remove_request, construct(image_id)); EXPECT_CALL(image_remove_request, send()) .WillOnce(Invoke([this, &image_remove_request, r]() { - m_threads->work_queue->queue(image_remove_request.on_finish, r); + m_threads->work_queue->queue( + image_remove_request.on_finish, r); })); } }; -TEST_F(TestMockImageDeleterRemoveRequest, Success) { +TEST_F(TestMockImageDeleterTrashRemoveRequest, Success) { InSequence seq; expect_get_snapcontext("image id", {1, {1}}, 0); @@ -158,14 +160,14 @@ TEST_F(TestMockImageDeleterRemoveRequest, Success) { C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", - &error_result, m_threads->work_queue, - &ctx); + auto req = MockTrashRemoveRequest::create(m_local_io_ctx, "image id", + &error_result, + m_threads->work_queue, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } -TEST_F(TestMockImageDeleterRemoveRequest, GetSnapContextDNE) { +TEST_F(TestMockImageDeleterTrashRemoveRequest, GetSnapContextDNE) { InSequence seq; expect_get_snapcontext("image id", {1, {1}}, -ENOENT); @@ -174,27 +176,27 @@ TEST_F(TestMockImageDeleterRemoveRequest, GetSnapContextDNE) { C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", - &error_result, m_threads->work_queue, - &ctx); + auto req = MockTrashRemoveRequest::create(m_local_io_ctx, "image id", + &error_result, + m_threads->work_queue, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } -TEST_F(TestMockImageDeleterRemoveRequest, GetSnapContextError) { +TEST_F(TestMockImageDeleterTrashRemoveRequest, GetSnapContextError) { InSequence seq; expect_get_snapcontext("image id", {1, {1}}, -EINVAL); C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", - &error_result, m_threads->work_queue, - &ctx); + auto req = MockTrashRemoveRequest::create(m_local_io_ctx, "image id", + &error_result, + m_threads->work_queue, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockImageDeleterRemoveRequest, PurgeSnapshotBusy) { +TEST_F(TestMockImageDeleterTrashRemoveRequest, PurgeSnapshotBusy) { InSequence seq; expect_get_snapcontext("image id", {1, {1}}, 0); @@ -203,15 +205,15 @@ TEST_F(TestMockImageDeleterRemoveRequest, PurgeSnapshotBusy) { C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", - &error_result, m_threads->work_queue, - &ctx); + auto req = MockTrashRemoveRequest::create(m_local_io_ctx, "image id", + &error_result, + m_threads->work_queue, &ctx); req->send(); ASSERT_EQ(-EBUSY, ctx.wait()); ASSERT_EQ(ERROR_RESULT_RETRY_IMMEDIATELY, error_result); } -TEST_F(TestMockImageDeleterRemoveRequest, PurgeSnapshotError) { +TEST_F(TestMockImageDeleterTrashRemoveRequest, PurgeSnapshotError) { InSequence seq; expect_get_snapcontext("image id", {1, {1}}, 0); @@ -220,14 +222,14 @@ TEST_F(TestMockImageDeleterRemoveRequest, PurgeSnapshotError) { C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", - &error_result, m_threads->work_queue, - &ctx); + auto req = MockTrashRemoveRequest::create(m_local_io_ctx, "image id", + &error_result, + m_threads->work_queue, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockImageDeleterRemoveRequest, RemoveError) { +TEST_F(TestMockImageDeleterTrashRemoveRequest, RemoveError) { InSequence seq; expect_get_snapcontext("image id", {1, {1}}, 0); @@ -239,9 +241,9 @@ TEST_F(TestMockImageDeleterRemoveRequest, RemoveError) { C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", - &error_result, m_threads->work_queue, - &ctx); + auto req = MockTrashRemoveRequest::create(m_local_io_ctx, "image id", + &error_result, + m_threads->work_queue, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } diff --git a/src/tools/rbd_mirror/CMakeLists.txt b/src/tools/rbd_mirror/CMakeLists.txt index 6184e418d97bf..30106a863c043 100644 --- a/src/tools/rbd_mirror/CMakeLists.txt +++ b/src/tools/rbd_mirror/CMakeLists.txt @@ -21,9 +21,9 @@ set(rbd_mirror_internal ServiceDaemon.cc Threads.cc Types.cc - image_deleter/RemoveRequest.cc image_deleter/SnapshotPurgeRequest.cc image_deleter/TrashMoveRequest.cc + image_deleter/TrashRemoveRequest.cc image_deleter/TrashWatcher.cc image_map/LoadRequest.cc image_map/Policy.cc diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index 93ddf5cea1b25..f4d928caa51cf 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -25,14 +25,13 @@ #include "librbd/ImageState.h" #include "librbd/Journal.h" #include "librbd/Operations.h" -#include "librbd/image/RemoveRequest.h" #include "cls/rbd/cls_rbd_client.h" #include "cls/rbd/cls_rbd_types.h" #include "librbd/Utils.h" #include "ImageDeleter.h" #include "tools/rbd_mirror/Threads.h" -#include "tools/rbd_mirror/image_deleter/RemoveRequest.h" #include "tools/rbd_mirror/image_deleter/TrashMoveRequest.h" +#include "tools/rbd_mirror/image_deleter/TrashRemoveRequest.h" #include "tools/rbd_mirror/image_deleter/TrashWatcher.h" #include #include @@ -388,7 +387,7 @@ void ImageDeleter::remove_image(DeleteInfoRef delete_info) { m_async_op_tracker.finish_op(); }); - auto req = image_deleter::RemoveRequest::create( + auto req = image_deleter::TrashRemoveRequest::create( m_local_io_ctx, delete_info->image_id, &delete_info->error_result, m_threads->work_queue, ctx); req->send(); diff --git a/src/tools/rbd_mirror/image_deleter/RemoveRequest.cc b/src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.cc similarity index 78% rename from src/tools/rbd_mirror/image_deleter/RemoveRequest.cc rename to src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.cc index 36f06866fe0e2..c08a0a8c0a5fa 100644 --- a/src/tools/rbd_mirror/image_deleter/RemoveRequest.cc +++ b/src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.cc @@ -1,7 +1,7 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab -#include "tools/rbd_mirror/image_deleter/RemoveRequest.h" +#include "tools/rbd_mirror/image_deleter/TrashRemoveRequest.h" #include "include/ceph_assert.h" #include "common/debug.h" #include "common/errno.h" @@ -16,7 +16,7 @@ #define dout_context g_ceph_context #define dout_subsys ceph_subsys_rbd_mirror #undef dout_prefix -#define dout_prefix *_dout << "rbd::mirror::image_deleter::RemoveRequest: " \ +#define dout_prefix *_dout << "rbd::mirror::image_deleter::TrashRemoveRequest: " \ << this << " " << __func__ << ": " namespace rbd { @@ -27,14 +27,14 @@ using librbd::util::create_context_callback; using librbd::util::create_rados_callback; template -void RemoveRequest::send() { +void TrashRemoveRequest::send() { *m_error_result = ERROR_RESULT_RETRY; get_snap_context(); } template -void RemoveRequest::get_snap_context() { +void TrashRemoveRequest::get_snap_context() { dout(10) << dendl; librados::ObjectReadOperation op; @@ -43,7 +43,8 @@ void RemoveRequest::get_snap_context() { std::string header_oid = librbd::util::header_name(m_image_id); auto aio_comp = create_rados_callback< - RemoveRequest, &RemoveRequest::handle_get_snap_context>(this); + TrashRemoveRequest, + &TrashRemoveRequest::handle_get_snap_context>(this); m_out_bl.clear(); int r = m_io_ctx.aio_operate(header_oid, aio_comp, &op, &m_out_bl); ceph_assert(r == 0); @@ -51,7 +52,7 @@ void RemoveRequest::get_snap_context() { } template -void RemoveRequest::handle_get_snap_context(int r) { +void TrashRemoveRequest::handle_get_snap_context(int r) { dout(10) << "r=" << r << dendl; ::SnapContext snapc; @@ -71,7 +72,7 @@ void RemoveRequest::handle_get_snap_context(int r) { } template -void RemoveRequest::purge_snapshots() { +void TrashRemoveRequest::purge_snapshots() { if (!m_has_snapshots) { remove_image(); return; @@ -79,13 +80,14 @@ void RemoveRequest::purge_snapshots() { dout(10) << dendl; auto ctx = create_context_callback< - RemoveRequest, &RemoveRequest::handle_purge_snapshots>(this); + TrashRemoveRequest, + &TrashRemoveRequest::handle_purge_snapshots>(this); auto req = SnapshotPurgeRequest::create(m_io_ctx, m_image_id, ctx); req->send(); } template -void RemoveRequest::handle_purge_snapshots(int r) { +void TrashRemoveRequest::handle_purge_snapshots(int r) { dout(10) << "r=" << r << dendl; if (r == -EBUSY) { @@ -103,11 +105,12 @@ void RemoveRequest::handle_purge_snapshots(int r) { } template -void RemoveRequest::remove_image() { +void TrashRemoveRequest::remove_image() { dout(10) << dendl; auto ctx = create_context_callback< - RemoveRequest, &RemoveRequest::handle_remove_image>(this); + TrashRemoveRequest, + &TrashRemoveRequest::handle_remove_image>(this); auto req = librbd::image::RemoveRequest::create( m_io_ctx, "", m_image_id, true, true, m_progress_ctx, m_op_work_queue, ctx); @@ -115,7 +118,7 @@ void RemoveRequest::remove_image() { } template -void RemoveRequest::handle_remove_image(int r) { +void TrashRemoveRequest::handle_remove_image(int r) { dout(10) << "r=" << r << dendl; if (r == -ENOTEMPTY) { // image must have clone v2 snapshot still associated to child @@ -137,7 +140,7 @@ void RemoveRequest::handle_remove_image(int r) { } template -void RemoveRequest::finish(int r) { +void TrashRemoveRequest::finish(int r) { dout(10) << "r=" << r << dendl; m_on_finish->complete(r); @@ -148,4 +151,4 @@ void RemoveRequest::finish(int r) { } // namespace mirror } // namespace rbd -template class rbd::mirror::image_deleter::RemoveRequest; +template class rbd::mirror::image_deleter::TrashRemoveRequest; diff --git a/src/tools/rbd_mirror/image_deleter/RemoveRequest.h b/src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.h similarity index 60% rename from src/tools/rbd_mirror/image_deleter/RemoveRequest.h rename to src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.h index 75353aa43d127..c2aef03801721 100644 --- a/src/tools/rbd_mirror/image_deleter/RemoveRequest.h +++ b/src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.h @@ -1,8 +1,8 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab -#ifndef CEPH_RBD_MIRROR_IMAGE_DELETER_REMOVE_REQUEST_H -#define CEPH_RBD_MIRROR_IMAGE_DELETER_REMOVE_REQUEST_H +#ifndef CEPH_RBD_MIRROR_IMAGE_DELETER_TRASH_REMOVE_REQUEST_H +#define CEPH_RBD_MIRROR_IMAGE_DELETER_TRASH_REMOVE_REQUEST_H #include "include/rados/librados.hpp" #include "include/buffer.h" @@ -20,19 +20,20 @@ namespace mirror { namespace image_deleter { template -class RemoveRequest { +class TrashRemoveRequest { public: - static RemoveRequest* create(librados::IoCtx &io_ctx, - const std::string &image_id, - ErrorResult *error_result, - ContextWQ *op_work_queue, Context *on_finish) { - return new RemoveRequest(io_ctx, image_id, error_result, op_work_queue, - on_finish); + static TrashRemoveRequest* create(librados::IoCtx &io_ctx, + const std::string &image_id, + ErrorResult *error_result, + ContextWQ *op_work_queue, + Context *on_finish) { + return new TrashRemoveRequest(io_ctx, image_id, error_result, op_work_queue, + on_finish); } - RemoveRequest(librados::IoCtx &io_ctx, const std::string &image_id, - ErrorResult *error_result, ContextWQ *op_work_queue, - Context *on_finish) + TrashRemoveRequest(librados::IoCtx &io_ctx, const std::string &image_id, + ErrorResult *error_result, ContextWQ *op_work_queue, + Context *on_finish) : m_io_ctx(io_ctx), m_image_id(image_id), m_error_result(error_result), m_op_work_queue(op_work_queue), m_on_finish(on_finish) { } @@ -87,6 +88,6 @@ private: } // namespace mirror } // namespace rbd -extern template class rbd::mirror::image_deleter::RemoveRequest; +extern template class rbd::mirror::image_deleter::TrashRemoveRequest; -#endif // CEPH_RBD_MIRROR_IMAGE_DELETER_REMOVE_REQUEST_H +#endif // CEPH_RBD_MIRROR_IMAGE_DELETER_TRASH_REMOVE_REQUEST_H -- 2.39.5