librbd: remove request now handles clone v2

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

diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc
index 65cc9b72cca4e79c63510a598c0fe1574a0d1575..0013bee5c0fe23b82c561b8cad439b493404f6a6 100644 (file)
--- a/src/librbd/image/RemoveRequest.cc
+++ b/src/librbd/image/RemoveRequest.cc
@@ -1,6 +1,7 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
 
+#include "librbd/image/RemoveRequest.h"
 #include "common/dout.h"
 #include "common/errno.h"
 #include "librbd/internal.h"
@@ -9,10 +10,10 @@
 #include "librbd/ObjectMap.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/MirroringWatcher.h"
+#include "librbd/image/DetachChildRequest.h"
 #include "librbd/journal/RemoveRequest.h"
-#include "librbd/image/RemoveRequest.h"
-#include "librbd/operation/TrimRequest.h"
 #include "librbd/mirror/DisableRequest.h"
+#include "librbd/operation/TrimRequest.h"
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
@@ -364,42 +365,30 @@ void RemoveRequest<I>::handle_trim_image(int r) {
     return;
   }
 
-  remove_child();
+  detach_child();
 }
 
 template<typename I>
-void RemoveRequest<I>::remove_child() {
+void RemoveRequest<I>::detach_child() {
   ldout(m_cct, 20) << dendl;
 
-  m_image_ctx->parent_lock.get_read();
-  ParentInfo parent_info = m_image_ctx->parent_md;
-  m_image_ctx->parent_lock.put_read();
-
-  librados::ObjectWriteOperation op;
-  librbd::cls_client::remove_child(&op, parent_info.spec, m_image_id);
-
-  using klass = RemoveRequest<I>;
-  librados::AioCompletion *rados_completion =
-    create_rados_callback<klass, &klass::handle_remove_child>(this);
-  int r = m_image_ctx->md_ctx.aio_operate(RBD_CHILDREN, rados_completion, &op);
-  assert(r == 0);
-  rados_completion->release();
+  auto ctx = create_context_callback<
+    RemoveRequest<I>, &RemoveRequest<I>::handle_detach_child>(this);
+  auto req = DetachChildRequest<I>::create(*m_image_ctx, ctx);
+  req->send();
 }
 
 template<typename I>
-void RemoveRequest<I>::handle_remove_child(int r) {
+void RemoveRequest<I>::handle_detach_child(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (r == -ENOENT) {
-    r = 0;
-  } else if (r < 0) {
-    lderr(m_cct) << "error removing child from children list: "
+  if (r < 0) {
+    lderr(m_cct) << "failed to detach child from parent: "
                  << cpp_strerror(r) << dendl;
     send_close_image(r);
     return;
   }
 
-
   send_disable_mirror();
 }
 

diff --git a/src/librbd/image/RemoveRequest.h b/src/librbd/image/RemoveRequest.h
index 05adeaa18fd7997e0f67c730ff94c5e81736853e..ab16ed72d15338f4509da7cf5721168d5ed58ade 100644 (file)
--- a/src/librbd/image/RemoveRequest.h
+++ b/src/librbd/image/RemoveRequest.h
@@ -4,6 +4,8 @@
 #ifndef CEPH_LIBRBD_IMAGE_REMOVE_REQUEST_H
 #define CEPH_LIBRBD_IMAGE_REMOVE_REQUEST_H
 
+#include "include/rados/librados.hpp"
+#include "librbd/ImageCtx.h"
 #include "librbd/image/TypeTraits.h"
 
 class Context;
@@ -12,7 +14,6 @@ class SafeTimer;
 
 namespace librbd {
 
-class ImageCtx;
 class ProgressContext;
 
 namespace image {
@@ -63,7 +64,7 @@ private:
    * |                              TRIM IMAGE                  |
    * |                                   |                      |
    * v                                   v                      |
-   * |                            REMOVE CHILD                  |
+   * |                              DETACH CHILD                |
    * |                                   |                      |
    * |                                   v                      v
    * \--------->------------------>CLOSE IMAGE                  |
@@ -118,6 +119,8 @@ private:
   bool m_unknown_format = true;
   ImageCtxT *m_image_ctx;
 
+  librados::IoCtx m_parent_io_ctx;
+
   decltype(m_image_ctx->exclusive_lock) m_exclusive_lock = nullptr;
 
   int m_ret_val = 0;
@@ -163,8 +166,8 @@ private:
   void trim_image();
   void handle_trim_image(int r);
 
-  void remove_child();
-  void handle_remove_child(int r);
+  void detach_child();
+  void handle_detach_child(int r);
 
   void send_disable_mirror();
   void handle_disable_mirror(int r);

diff --git a/src/test/librbd/image/test_mock_RemoveRequest.cc b/src/test/librbd/image/test_mock_RemoveRequest.cc
index a4cb4c76251c3507b7b87b287cd1775637f7c35a..3b565d594b903bff9d82328f2bf6d609d31176d7 100644 (file)
--- a/src/test/librbd/image/test_mock_RemoveRequest.cc
+++ b/src/test/librbd/image/test_mock_RemoveRequest.cc
@@ -13,6 +13,7 @@
 #include "librbd/Operations.h"
 #include "librbd/operation/TrimRequest.h"
 #include "librbd/image/TypeTraits.h"
+#include "librbd/image/DetachChildRequest.h"
 #include "librbd/image/RemoveRequest.h"
 #include "librbd/image/RefreshParentRequest.h"
 #include "librbd/mirror/DisableRequest.h"
@@ -23,20 +24,64 @@
 #include <boost/scope_exit.hpp>
 
 namespace librbd {
+namespace {
+
+struct MockTestImageCtx : public MockImageCtx {
+  static MockTestImageCtx* s_instance;
+  static MockTestImageCtx* create(const std::string &image_name,
+                                  const std::string &image_id,
+                                  const char *snap, librados::IoCtx& p,
+                                  bool read_only) {
+    assert(s_instance != nullptr);
+    return s_instance;
+  }
+
+  MockTestImageCtx(ImageCtx &image_ctx) : MockImageCtx(image_ctx) {
+    s_instance = this;
+  }
+};
+
+MockTestImageCtx* MockTestImageCtx::s_instance = nullptr;
+
+} // anonymous namespace
 namespace image {
+
 template <>
-struct TypeTraits<MockImageCtx> {
+struct TypeTraits<MockTestImageCtx> {
   typedef librbd::MockContextWQ ContextWQ;
 };
-}
+
+template <>
+class DetachChildRequest<MockTestImageCtx> {
+public:
+  static DetachChildRequest *s_instance;
+  static DetachChildRequest *create(MockTestImageCtx &image_ctx,
+                                    Context *on_finish) {
+    assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  Context *on_finish = nullptr;
+
+  DetachChildRequest() {
+    s_instance = this;
+  }
+
+  MOCK_METHOD0(send, void());
+};
+
+DetachChildRequest<MockTestImageCtx> *DetachChildRequest<MockTestImageCtx>::s_instance;
+
+} // namespace image
 
 namespace operation {
 
 template <>
-class TrimRequest<MockImageCtx> {
+class TrimRequest<MockTestImageCtx> {
 public:
   static TrimRequest *s_instance;
-  static TrimRequest *create(MockImageCtx &image_ctx, Context *on_finish,
+  static TrimRequest *create(MockTestImageCtx &image_ctx, Context *on_finish,
                              uint64_t original_size, uint64_t new_size,
                              ProgressContext &prog_ctx) {
     assert(s_instance != nullptr);
@@ -53,13 +98,16 @@ public:
   MOCK_METHOD0(send, void());
 };
 
+TrimRequest<MockTestImageCtx> *TrimRequest<MockTestImageCtx>::s_instance;
+
 } // namespace operation
 
 namespace journal {
+
 template <>
-class RemoveRequest<MockImageCtx> {
+class RemoveRequest<MockTestImageCtx> {
 private:
-  typedef ::librbd::image::TypeTraits<MockImageCtx> TypeTraits;
+  typedef ::librbd::image::TypeTraits<MockTestImageCtx> TypeTraits;
   typedef typename TypeTraits::ContextWQ ContextWQ;
 public:
   static RemoveRequest *s_instance;
@@ -79,18 +127,20 @@ public:
 
   MOCK_METHOD0(send, void());
 };
-RemoveRequest<MockImageCtx> *RemoveRequest<MockImageCtx>::s_instance = nullptr;
+
+RemoveRequest<MockTestImageCtx> *RemoveRequest<MockTestImageCtx>::s_instance = nullptr;
+
 } // namespace journal
 
 namespace mirror {
 
 template<>
-class DisableRequest<MockImageCtx> {
+class DisableRequest<MockTestImageCtx> {
 public:
   static DisableRequest *s_instance;
   Context *on_finish = nullptr;
 
-  static DisableRequest *create(MockImageCtx *image_ctx, bool force,
+  static DisableRequest *create(MockTestImageCtx *image_ctx, bool force,
                                 bool remove, Context *on_finish) {
     assert(s_instance != nullptr);
     s_instance->on_finish = on_finish;
@@ -104,14 +154,13 @@ public:
   MOCK_METHOD0(send, void());
 };
 
-DisableRequest<MockImageCtx> *DisableRequest<MockImageCtx>::s_instance;
+DisableRequest<MockTestImageCtx> *DisableRequest<MockTestImageCtx>::s_instance;
 
 } // namespace mirror
 } // namespace librbd
 
 // template definitions
 #include "librbd/image/RemoveRequest.cc"
-template class librbd::image::RemoveRequest<librbd::MockImageCtx>;
 
 namespace librbd {
 namespace image {
@@ -128,28 +177,29 @@ using ::testing::StrEq;
 
 class TestMockImageRemoveRequest : public TestMockFixture {
 public:
-  typedef ::librbd::image::TypeTraits<MockImageCtx> TypeTraits;
+  typedef ::librbd::image::TypeTraits<MockTestImageCtx> TypeTraits;
   typedef typename TypeTraits::ContextWQ ContextWQ;
-  typedef RemoveRequest<MockImageCtx> MockRemoveRequest;
-  typedef librbd::operation::TrimRequest<MockImageCtx> MockTrimRequest;
-  typedef librbd::journal::RemoveRequest<MockImageCtx> MockJournalRemoveRequest;
-  typedef librbd::mirror::DisableRequest<MockImageCtx> MockMirrorDisableRequest;
+  typedef RemoveRequest<MockTestImageCtx> MockRemoveRequest;
+  typedef DetachChildRequest<MockTestImageCtx> MockDetachChildRequest;
+  typedef librbd::operation::TrimRequest<MockTestImageCtx> MockTrimRequest;
+  typedef librbd::journal::RemoveRequest<MockTestImageCtx> MockJournalRemoveRequest;
+  typedef librbd::mirror::DisableRequest<MockTestImageCtx> MockMirrorDisableRequest;
 
   librbd::ImageCtx *m_test_imctx = NULL;
-  MockImageCtx *m_mock_imctx = NULL;
+  MockTestImageCtx *m_mock_imctx = NULL;
 
 
   void TestImageRemoveSetUp() {
     ASSERT_EQ(0, open_image(m_image_name, &m_test_imctx));
-    m_mock_imctx = new MockImageCtx(*m_test_imctx);
-    librbd::MockImageCtx::s_instance = m_mock_imctx;
+    m_mock_imctx = new MockTestImageCtx(*m_test_imctx);
+    librbd::MockTestImageCtx::s_instance = m_mock_imctx;
   }
   void TestImageRemoveTearDown() {
-    librbd::MockImageCtx::s_instance = NULL;
+    librbd::MockTestImageCtx::s_instance = NULL;
     delete m_mock_imctx;
   }
 
-  void expect_state_open(MockImageCtx &mock_image_ctx, int r) {
+  void expect_state_open(MockTestImageCtx &mock_image_ctx, int r) {
     EXPECT_CALL(*mock_image_ctx.state, open(_, _))
       .WillOnce(Invoke([r](bool open_parent, Context *on_ready) {
                  on_ready->complete(r);
@@ -159,7 +209,7 @@ public:
     }
   }
 
-  void expect_state_close(MockImageCtx &mock_image_ctx) {
+  void expect_state_close(MockTestImageCtx &mock_image_ctx) {
     EXPECT_CALL(*mock_image_ctx.state, close(_))
       .WillOnce(Invoke([](Context *on_ready) {
                   on_ready->complete(0);
@@ -174,7 +224,7 @@ public:
                 }));
   }
 
-  void expect_get_group(MockImageCtx &mock_image_ctx, int r) {
+  void expect_get_group(MockTestImageCtx &mock_image_ctx, int r) {
     auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                                exec(mock_image_ctx.header_oid, _, StrEq("rbd"),
                                     StrEq("image_group_get"), _, _, _));
@@ -185,31 +235,24 @@ public:
     }
   }
 
-  void expect_trim(MockImageCtx &mock_image_ctx,
+  void expect_trim(MockTestImageCtx &mock_image_ctx,
                    MockTrimRequest &mock_trim_request, int r) {
     EXPECT_CALL(mock_trim_request, send())
                   .WillOnce(FinishRequest(&mock_trim_request, r, &mock_image_ctx));
   }
 
-  void expect_journal_remove(MockImageCtx &mock_image_ctx,
+  void expect_journal_remove(MockTestImageCtx &mock_image_ctx,
                    MockJournalRemoveRequest &mock_journal_remove_request, int r) {
     EXPECT_CALL(mock_journal_remove_request, send())
                   .WillOnce(FinishRequest(&mock_journal_remove_request, r, &mock_image_ctx));
   }
 
-  void expect_mirror_disable(MockImageCtx &mock_image_ctx,
+  void expect_mirror_disable(MockTestImageCtx &mock_image_ctx,
                    MockMirrorDisableRequest &mock_mirror_disable_request, int r) {
     EXPECT_CALL(mock_mirror_disable_request, send())
                   .WillOnce(FinishRequest(&mock_mirror_disable_request, r, &mock_image_ctx));
   }
 
-  void expect_remove_child(MockImageCtx &mock_image_ctx, int r) {
-    EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
-                               exec(RBD_CHILDREN, _, StrEq("rbd"), StrEq("remove_child"), _,
-                                    _, _))
-      .WillOnce(Return(r));
-  }
-
   void expect_remove_mirror_image(librados::IoCtx &ioctx, int r) {
     EXPECT_CALL(get_mock_io_ctx(ioctx),
                 exec(StrEq("rbd_mirroring"), _, StrEq("rbd"), StrEq("mirror_image_remove"),
@@ -217,7 +260,7 @@ public:
       .WillOnce(Return(r));
   }
 
-  void expect_mirror_image_get(MockImageCtx &mock_image_ctx, int r) {
+  void expect_mirror_image_get(MockTestImageCtx &mock_image_ctx, int r) {
     EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                 exec(RBD_MIRRORING, _, StrEq("rbd"), StrEq("mirror_image_get"),
                      _, _, _))
@@ -230,6 +273,12 @@ public:
                      _, _, _))
       .WillOnce(Return(r));
   }
+
+  void expect_detach_child(MockTestImageCtx &mock_image_ctx,
+                           MockDetachChildRequest& mock_request, int r) {
+    EXPECT_CALL(mock_request, send())
+      .WillOnce(FinishRequest(&mock_request, r, &mock_image_ctx));
+  }
 };
 
 TEST_F(TestMockImageRemoveRequest, SuccessV1) {
@@ -281,7 +330,48 @@ TEST_F(TestMockImageRemoveRequest, OpenFailV1) {
   TestImageRemoveTearDown();
 }
 
-TEST_F(TestMockImageRemoveRequest, SuccessV2) {
+TEST_F(TestMockImageRemoveRequest, SuccessV2CloneV1) {
+  REQUIRE_FEATURE(RBD_FEATURE_JOURNALING);
+  TestImageRemoveSetUp();
+
+  C_SaferCond ctx;
+  librbd::NoOpProgressContext no_op;
+  ContextWQ op_work_queue;
+  MockTrimRequest mock_trim_request;
+  MockJournalRemoveRequest mock_journal_remove_request;
+  MockMirrorDisableRequest mock_mirror_disable_request;
+
+  m_mock_imctx->parent_md.spec.pool_id = m_ioctx.get_id();
+  m_mock_imctx->parent_md.spec.image_id = "parent id";
+  m_mock_imctx->parent_md.spec.snap_id = 234;
+
+  InSequence seq;
+  expect_state_open(*m_mock_imctx, 0);
+  expect_mirror_image_get(*m_mock_imctx, 0);
+  expect_get_group(*m_mock_imctx, 0);
+  expect_trim(*m_mock_imctx, mock_trim_request, 0);
+  expect_op_work_queue(*m_mock_imctx);
+
+  MockDetachChildRequest mock_detach_child_request;
+  expect_detach_child(*m_mock_imctx, mock_detach_child_request, 0);
+
+  expect_mirror_disable(*m_mock_imctx, mock_mirror_disable_request, 0);
+  expect_state_close(*m_mock_imctx);
+  expect_wq_queue(op_work_queue, 0);
+  expect_journal_remove(*m_mock_imctx, mock_journal_remove_request, 0);
+  expect_remove_mirror_image(m_ioctx, 0);
+  expect_dir_remove_image(m_ioctx, 0);
+
+  MockRemoveRequest *req = MockRemoveRequest::create(m_ioctx, m_image_name, "",
+                                             true, false, no_op, &op_work_queue, &ctx);
+  req->send();
+
+  ASSERT_EQ(0, ctx.wait());
+
+  TestImageRemoveTearDown();
+}
+
+TEST_F(TestMockImageRemoveRequest, SuccessV2CloneV2) {
   REQUIRE_FEATURE(RBD_FEATURE_JOURNALING);
   TestImageRemoveSetUp();
 
@@ -292,13 +382,20 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2) {
   MockJournalRemoveRequest mock_journal_remove_request;
   MockMirrorDisableRequest mock_mirror_disable_request;
 
+  m_mock_imctx->parent_md.spec.pool_id = m_ioctx.get_id();
+  m_mock_imctx->parent_md.spec.image_id = "parent id";
+  m_mock_imctx->parent_md.spec.snap_id = 234;
+
   InSequence seq;
   expect_state_open(*m_mock_imctx, 0);
   expect_mirror_image_get(*m_mock_imctx, 0);
   expect_get_group(*m_mock_imctx, 0);
   expect_trim(*m_mock_imctx, mock_trim_request, 0);
   expect_op_work_queue(*m_mock_imctx);
-  expect_remove_child(*m_mock_imctx, 0);
+
+  MockDetachChildRequest mock_detach_child_request;
+  expect_detach_child(*m_mock_imctx, mock_detach_child_request, 0);
+
   expect_mirror_disable(*m_mock_imctx, mock_mirror_disable_request, 0);
   expect_state_close(*m_mock_imctx);
   expect_wq_queue(op_work_queue, 0);
@@ -326,13 +423,20 @@ TEST_F(TestMockImageRemoveRequest, NotExistsV2) {
   MockJournalRemoveRequest mock_journal_remove_request;
   MockMirrorDisableRequest mock_mirror_disable_request;
 
+  m_mock_imctx->parent_md.spec.pool_id = m_ioctx.get_id();
+  m_mock_imctx->parent_md.spec.image_id = "parent id";
+  m_mock_imctx->parent_md.spec.snap_id = 234;
+
   InSequence seq;
   expect_state_open(*m_mock_imctx, 0);
   expect_mirror_image_get(*m_mock_imctx, 0);
   expect_get_group(*m_mock_imctx, 0);
   expect_trim(*m_mock_imctx, mock_trim_request, 0);
   expect_op_work_queue(*m_mock_imctx);
-  expect_remove_child(*m_mock_imctx, 0);
+
+  MockDetachChildRequest mock_detach_child_request;
+  expect_detach_child(*m_mock_imctx, mock_detach_child_request, 0);
+
   expect_mirror_disable(*m_mock_imctx, mock_mirror_disable_request, 0);
   expect_state_close(*m_mock_imctx);
   expect_wq_queue(op_work_queue, 0);