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 <dillaman@redhat.com>

src/test/rbd_mirror/CMakeLists.txt

@@ -28,9 +28,9 @@ add_executable(unittest_rbd_mirror
   test_mock_PoolReplayer.cc
   test_mock_PoolWatcher.cc
   test_mock_Throttler.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

src/test/rbd_mirror/image_deleter/test_mock_RemoveRequest.cc → 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<librbd::MockTestImageCtx>* SnapshotPurgeRequest<librbd::Moc
 } // namespace mirror
 } // namespace rbd
 
-#include "tools/rbd_mirror/image_deleter/RemoveRequest.cc"
+#include "tools/rbd_mirror/image_deleter/TrashRemoveRequest.cc"
 
 namespace rbd {
 namespace mirror {
@@ -108,9 +108,9 @@ using ::testing::StrEq;
 using ::testing::WithArg;
 using ::testing::WithArgs;
 
-class TestMockImageDeleterRemoveRequest : public TestMockFixture {
+class TestMockImageDeleterTrashRemoveRequest : public TestMockFixture {
 public:
-  typedef RemoveRequest<librbd::MockTestImageCtx> MockRemoveRequest;
+  typedef TrashRemoveRequest<librbd::MockTestImageCtx> MockTrashRemoveRequest;
   typedef SnapshotPurgeRequest<librbd::MockTestImageCtx> MockSnapshotPurgeRequest;
   typedef librbd::image::RemoveRequest<librbd::MockTestImageCtx> MockImageRemoveRequest;
 
@@ -133,7 +133,8 @@ class TestMockImageDeleterRemoveRequest : public TestMockFixture {
     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);
                 }));
   }
 
@@ -142,12 +143,13 @@ class TestMockImageDeleterRemoveRequest : public TestMockFixture {
     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);
 
@@ -159,14 +161,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);
 
@@ -175,27 +177,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);
 
@@ -204,15 +206,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);
 
@@ -221,14 +223,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);
 
@@ -240,9 +242,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());
 }

src/tools/rbd_mirror/CMakeLists.txt

@@ -22,9 +22,9 @@ set(rbd_mirror_internal
   Threads.cc
   Throttler.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

src/tools/rbd_mirror/ImageDeleter.cc

@@ -25,15 +25,14 @@
 #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/Throttler.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 <map>
 #include <sstream>
@@ -402,7 +401,7 @@ void ImageDeleter<I>::remove_image(DeleteInfoRef delete_info) {
       m_async_op_tracker.finish_op();
     });
 
-  auto req = image_deleter::RemoveRequest<I>::create(
+  auto req = image_deleter::TrashRemoveRequest<I>::create(
     m_local_io_ctx, delete_info->image_id, &delete_info->error_result,
     m_threads->work_queue, ctx);
   req->send();

src/tools/rbd_mirror/image_deleter/RemoveRequest.cc → 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 <typename I>
-void RemoveRequest<I>::send() {
+void TrashRemoveRequest<I>::send() {
   *m_error_result = ERROR_RESULT_RETRY;
 
   get_snap_context();
 }
 
 template <typename I>
-void RemoveRequest<I>::get_snap_context() {
+void TrashRemoveRequest<I>::get_snap_context() {
   dout(10) << dendl;
 
   librados::ObjectReadOperation op;
@@ -43,15 +43,16 @@ void RemoveRequest<I>::get_snap_context() {
   std::string header_oid = librbd::util::header_name(m_image_id);
 
   auto aio_comp = create_rados_callback<
-    RemoveRequest<I>, &RemoveRequest<I>::handle_get_snap_context>(this);
+    TrashRemoveRequest<I>,
+    &TrashRemoveRequest<I>::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);
   aio_comp->release();
 }
 
 template <typename I>
-void RemoveRequest<I>::handle_get_snap_context(int r) {
+void TrashRemoveRequest<I>::handle_get_snap_context(int r) {
   dout(10) << "r=" << r << dendl;
 
   ::SnapContext snapc;
@@ -71,21 +72,22 @@ void RemoveRequest<I>::handle_get_snap_context(int r) {
 }
 
 template <typename I>
-void RemoveRequest<I>::purge_snapshots() {
+void TrashRemoveRequest<I>::purge_snapshots() {
   if (!m_has_snapshots) {
     remove_image();
     return;
   }
 
   dout(10) << dendl;
   auto ctx = create_context_callback<
-    RemoveRequest<I>, &RemoveRequest<I>::handle_purge_snapshots>(this);
+    TrashRemoveRequest<I>,
+    &TrashRemoveRequest<I>::handle_purge_snapshots>(this);
   auto req = SnapshotPurgeRequest<I>::create(m_io_ctx, m_image_id, ctx);
   req->send();
 }
 
 template <typename I>
-void RemoveRequest<I>::handle_purge_snapshots(int r) {
+void TrashRemoveRequest<I>::handle_purge_snapshots(int r) {
   dout(10) << "r=" << r << dendl;
 
   if (r == -EBUSY) {
@@ -103,19 +105,20 @@ void RemoveRequest<I>::handle_purge_snapshots(int r) {
 }
 
 template <typename I>
-void RemoveRequest<I>::remove_image() {
+void TrashRemoveRequest<I>::remove_image() {
   dout(10) << dendl;
 
   auto ctx = create_context_callback<
-    RemoveRequest<I>, &RemoveRequest<I>::handle_remove_image>(this);
+    TrashRemoveRequest<I>,
+    &TrashRemoveRequest<I>::handle_remove_image>(this);
   auto req = librbd::image::RemoveRequest<I>::create(
     m_io_ctx, "", m_image_id, true, true, m_progress_ctx, m_op_work_queue,
     ctx);
   req->send();
 }
 
 template <typename I>
-void RemoveRequest<I>::handle_remove_image(int r) {
+void TrashRemoveRequest<I>::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<I>::handle_remove_image(int r) {
 }
 
 template <typename I>
-void RemoveRequest<I>::finish(int r) {
+void TrashRemoveRequest<I>::finish(int r) {
   dout(10) << "r=" << r << dendl;
 
   m_on_finish->complete(r);
@@ -148,4 +151,4 @@ void RemoveRequest<I>::finish(int r) {
 } // namespace mirror
 } // namespace rbd
 
-template class rbd::mirror::image_deleter::RemoveRequest<librbd::ImageCtx>;
+template class rbd::mirror::image_deleter::TrashRemoveRequest<librbd::ImageCtx>;

src/tools/rbd_mirror/image_deleter/RemoveRequest.h → 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 <typename ImageCtxT = librbd::ImageCtx>
-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 @@ class RemoveRequest {
 } // namespace mirror
 } // namespace rbd
 
-extern template class rbd::mirror::image_deleter::RemoveRequest<librbd::ImageCtx>;
+extern template class rbd::mirror::image_deleter::TrashRemoveRequest<librbd::ImageCtx>;
 
-#endif // CEPH_RBD_MIRROR_IMAGE_DELETER_REMOVE_REQUEST_H
+#endif // CEPH_RBD_MIRROR_IMAGE_DELETER_TRASH_REMOVE_REQUEST_H