]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
librbd: replace existing detach parent calls with new state machine
authorJason Dillaman <dillaman@redhat.com>
Wed, 29 Aug 2018 17:03:59 +0000 (13:03 -0400)
committerJason Dillaman <dillaman@redhat.com>
Wed, 19 Sep 2018 12:05:29 +0000 (08:05 -0400)
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
src/librbd/deep_copy/SetHeadRequest.cc
src/librbd/deep_copy/SetHeadRequest.h
src/librbd/operation/FlattenRequest.cc
src/librbd/operation/FlattenRequest.h
src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc

index 35da747164185d156069384e8ed2eb64f05436e6..16bbb04ad55a9da0b63d1f0d47e2510229883fc4 100644 (file)
@@ -7,6 +7,7 @@
 #include "cls/rbd/cls_rbd_types.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/Utils.h"
+#include "librbd/image/DetachParentRequest.h"
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
@@ -40,7 +41,7 @@ void SetHeadRequest<I>::send_set_size() {
   m_image_ctx->snap_lock.get_read();
   if (m_image_ctx->size == m_size) {
     m_image_ctx->snap_lock.put_read();
-    send_remove_parent();
+    send_detach_parent();
     return;
   }
   m_image_ctx->snap_lock.put_read();
@@ -95,11 +96,11 @@ void SetHeadRequest<I>::handle_set_size(int r) {
     m_image_ctx->size = m_size;
   }
 
-  send_remove_parent();
+  send_detach_parent();
 }
 
 template <typename I>
-void SetHeadRequest<I>::send_remove_parent() {
+void SetHeadRequest<I>::send_detach_parent() {
   m_image_ctx->parent_lock.get_read();
   if (m_image_ctx->parent_md.spec.pool_id == -1 ||
       (m_image_ctx->parent_md.spec == m_parent_spec &&
@@ -111,10 +112,6 @@ void SetHeadRequest<I>::send_remove_parent() {
   m_image_ctx->parent_lock.put_read();
 
   ldout(m_cct, 20) << dendl;
-
-  librados::ObjectWriteOperation op;
-  librbd::cls_client::remove_parent(&op);
-
   auto finish_op_ctx = start_lock_op();
   if (finish_op_ctx == nullptr) {
     lderr(m_cct) << "lost exclusive lock" << dendl;
@@ -123,17 +120,15 @@ void SetHeadRequest<I>::send_remove_parent() {
   }
 
   auto ctx = new FunctionContext([this, finish_op_ctx](int r) {
-      handle_remove_parent(r);
+      handle_detach_parent(r);
       finish_op_ctx->complete(0);
     });
-  librados::AioCompletion *comp = create_rados_callback(ctx);
-  int r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, comp, &op);
-  ceph_assert(r == 0);
-  comp->release();
+  auto req = image::DetachParentRequest<I>::create(*m_image_ctx, ctx);
+  req->send();
 }
 
 template <typename I>
-void SetHeadRequest<I>::handle_remove_parent(int r) {
+void SetHeadRequest<I>::handle_detach_parent(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
   if (r < 0) {
index 7754ff73553514bacad2b1a3ebfa42dc01dcf537..14d8013599aac97a21702518537aefaad2b681cf 100644 (file)
@@ -46,7 +46,7 @@ private:
    * SET_SIZE
    *    |
    *    v (skip if not needed)
-   * REMOVE_PARENT
+   * DETACH_PARENT
    *    |
    *    v (skip if not needed)
    * SET_PARENT
@@ -68,8 +68,8 @@ private:
   void send_set_size();
   void handle_set_size(int r);
 
-  void send_remove_parent();
-  void handle_remove_parent(int r);
+  void send_detach_parent();
+  void handle_detach_parent(int r);
 
   void send_set_parent();
   void handle_set_parent(int r);
index 0bfdfcd09fa04ffd51e5cebe38ed66feccca7349..e55daf2670b80107d688e0a475346cccb0cfab64 100644 (file)
@@ -6,6 +6,7 @@
 #include "librbd/ExclusiveLock.h"
 #include "librbd/ImageCtx.h"
 #include "librbd/image/DetachChildRequest.h"
+#include "librbd/image/DetachParentRequest.h"
 #include "librbd/Types.h"
 #include "librbd/io/ObjectRequest.h"
 #include "common/dout.h"
@@ -147,7 +148,7 @@ void FlattenRequest<I>::detach_child() {
       !image_ctx.snaps.empty()) {
     image_ctx.snap_lock.put_read();
     image_ctx.owner_lock.put_read();
-    remove_parent();
+    detach_parent();
     return;
   }
   image_ctx.snap_lock.put_read();
@@ -173,11 +174,11 @@ void FlattenRequest<I>::handle_detach_child(int r) {
     return;
   }
 
-  remove_parent();
+  detach_parent();
 }
 
 template <typename I>
-void FlattenRequest<I>::remove_parent() {
+void FlattenRequest<I>::detach_parent() {
   I &image_ctx = this->m_image_ctx;
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << dendl;
@@ -200,25 +201,21 @@ void FlattenRequest<I>::remove_parent() {
   image_ctx.parent_lock.put_read();
 
   // remove parent from this (base) image
-  librados::ObjectWriteOperation op;
-  cls_client::remove_parent(&op);
-
-  auto aio_comp = create_rados_callback<
+  auto ctx = create_context_callback<
     FlattenRequest<I>,
-    &FlattenRequest<I>::handle_remove_parent>(this);
-  int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, aio_comp, &op);
-  ceph_assert(r == 0);
-  aio_comp->release();
+    &FlattenRequest<I>::handle_detach_parent>(this);
+  auto req = image::DetachParentRequest<I>::create(image_ctx, ctx);
+  req->send();
   image_ctx.owner_lock.put_read();
 }
 
 template <typename I>
-void FlattenRequest<I>::handle_remove_parent(int r) {
+void FlattenRequest<I>::handle_detach_parent(int r) {
   I &image_ctx = this->m_image_ctx;
   CephContext *cct = image_ctx.cct;
   ldout(cct, 5) << "r=" << r << dendl;
 
-  if (r < 0 && r != -ENOENT) {
+  if (r < 0) {
     lderr(cct) << "remove parent encountered an error: " << cpp_strerror(r)
                << dendl;
   }
index 171ebae0075df48a7eadc4c1853fc1721986f24f..78730963cd64fc9654a75626aa0957a7e16e0a79 100644 (file)
@@ -46,7 +46,7 @@ private:
    * DETACH_CHILD
    *    |
    *    v
-   * REMOVE_PARENT
+   * DETACH_PARENT
    *    |
    *    v
    * <finish>
@@ -64,8 +64,8 @@ private:
   void detach_child();
   void handle_detach_child(int r);
 
-  void remove_parent();
-  void handle_remove_parent(int r);
+  void detach_parent();
+  void handle_detach_parent(int r);
 
 };
 
index 4477fa0f4c1ffee8e047cb3417704d0445add4cf..c94ccf4214e22eeb39db3a52d2e5ded509d8f558 100644 (file)
@@ -10,6 +10,7 @@
 #include "test/librados_test_stub/MockTestMemIoCtxImpl.h"
 #include "test/librbd/mock/MockImageCtx.h"
 #include "librbd/deep_copy/SetHeadRequest.h"
+#include "librbd/image/DetachParentRequest.h"
 
 namespace librbd {
 namespace {
@@ -21,6 +22,32 @@ struct MockTestImageCtx : public librbd::MockImageCtx {
 };
 
 } // anonymous namespace
+
+namespace image {
+
+template <>
+class DetachParentRequest<MockTestImageCtx> {
+public:
+  static DetachParentRequest *s_instance;
+  static DetachParentRequest *create(MockTestImageCtx &image_ctx,
+                                     Context *on_finish) {
+    ceph_assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  Context *on_finish = nullptr;
+
+  DetachParentRequest() {
+    s_instance = this;
+  }
+
+  MOCK_METHOD0(send, void());
+};
+
+DetachParentRequest<MockTestImageCtx> *DetachParentRequest<MockTestImageCtx>::s_instance;
+
+} // namespace image
 } // namespace librbd
 
 // template definitions
@@ -43,6 +70,7 @@ using ::testing::WithArg;
 class TestMockDeepCopySetHeadRequest : public TestMockFixture {
 public:
   typedef SetHeadRequest<librbd::MockTestImageCtx> MockSetHeadRequest;
+  typedef image::DetachParentRequest<MockTestImageCtx> MockDetachParentRequest;
 
   librbd::ImageCtx *m_image_ctx;
   ThreadPool *m_thread_pool;
@@ -74,10 +102,10 @@ public:
                   .WillOnce(Return(r));
   }
 
-  void expect_remove_parent(librbd::MockTestImageCtx &mock_image_ctx, int r) {
-    EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
-                exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("remove_parent"), _, _, _))
-                  .WillOnce(Return(r));
+  void expect_detach_parent(MockImageCtx &mock_image_ctx,
+                            MockDetachParentRequest& mock_request, int r) {
+    EXPECT_CALL(mock_request, send())
+      .WillOnce(FinishRequest(&mock_request, r, &mock_image_ctx));
   }
 
   void expect_set_parent(librbd::MockTestImageCtx &mock_image_ctx, int r) {
@@ -134,7 +162,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, RemoveParent) {
 
   InSequence seq;
   expect_start_op(mock_exclusive_lock);
-  expect_remove_parent(mock_image_ctx, 0);
+  MockDetachParentRequest mock_detach_parent;
+  expect_detach_parent(mock_image_ctx, mock_detach_parent, 0);
 
   C_SaferCond ctx;
   auto request = create_request(mock_image_ctx, m_image_ctx->size, {}, 0, &ctx);
@@ -151,7 +180,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, RemoveParentError) {
 
   InSequence seq;
   expect_start_op(mock_exclusive_lock);
-  expect_remove_parent(mock_image_ctx, -EINVAL);
+  MockDetachParentRequest mock_detach_parent;
+  expect_detach_parent(mock_image_ctx, mock_detach_parent, -EINVAL);
 
   C_SaferCond ctx;
   auto request = create_request(mock_image_ctx, m_image_ctx->size, {}, 0, &ctx);
@@ -168,7 +198,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, RemoveSetParent) {
 
   InSequence seq;
   expect_start_op(mock_exclusive_lock);
-  expect_remove_parent(mock_image_ctx, 0);
+  MockDetachParentRequest mock_detach_parent;
+  expect_detach_parent(mock_image_ctx, mock_detach_parent, 0);
   expect_start_op(mock_exclusive_lock);
   expect_set_parent(mock_image_ctx, 0);