librbd: replace existing attach parent calls with new state machine

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

diff --git a/src/librbd/deep_copy/SetHeadRequest.cc b/src/librbd/deep_copy/SetHeadRequest.cc
index 16bbb04ad55a9da0b63d1f0d47e2510229883fc4..34bd93506d25094f5de8de55162f083c313fb068 100644 (file)
--- a/src/librbd/deep_copy/SetHeadRequest.cc
+++ b/src/librbd/deep_copy/SetHeadRequest.cc
@@ -7,6 +7,7 @@
 #include "cls/rbd/cls_rbd_types.h"
 #include "librbd/ExclusiveLock.h"
 #include "librbd/Utils.h"
+#include "librbd/image/AttachParentRequest.h"
 #include "librbd/image/DetachParentRequest.h"
 
 #define dout_subsys ceph_subsys_rbd
@@ -106,7 +107,7 @@ void SetHeadRequest<I>::send_detach_parent() {
       (m_image_ctx->parent_md.spec == m_parent_spec &&
        m_image_ctx->parent_md.overlap == m_parent_overlap)) {
     m_image_ctx->parent_lock.put_read();
-    send_set_parent();
+    send_attach_parent();
     return;
   }
   m_image_ctx->parent_lock.put_read();
@@ -144,11 +145,11 @@ void SetHeadRequest<I>::handle_detach_parent(int r) {
     m_image_ctx->parent_md.overlap = 0;
   }
 
-  send_set_parent();
+  send_attach_parent();
 }
 
 template <typename I>
-void SetHeadRequest<I>::send_set_parent() {
+void SetHeadRequest<I>::send_attach_parent() {
   m_image_ctx->parent_lock.get_read();
   if (m_image_ctx->parent_md.spec == m_parent_spec &&
       m_image_ctx->parent_md.overlap == m_parent_overlap) {
@@ -159,10 +160,6 @@ void SetHeadRequest<I>::send_set_parent() {
   m_image_ctx->parent_lock.put_read();
 
   ldout(m_cct, 20) << dendl;
-
-  librados::ObjectWriteOperation op;
-  librbd::cls_client::set_parent(&op, m_parent_spec, m_parent_overlap);
-
   auto finish_op_ctx = start_lock_op();
   if (finish_op_ctx == nullptr) {
     lderr(m_cct) << "lost exclusive lock" << dendl;
@@ -171,21 +168,23 @@ void SetHeadRequest<I>::send_set_parent() {
   }
 
   auto ctx = new FunctionContext([this, finish_op_ctx](int r) {
-      handle_set_parent(r);
+      handle_attach_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::AttachParentRequest<I>::create(
+    *m_image_ctx,
+    {m_parent_spec.pool_id, "", m_parent_spec.image_id,
+     m_parent_spec.snap_id},
+    m_parent_overlap, ctx);
+  req->send();
 }
 
 template <typename I>
-void SetHeadRequest<I>::handle_set_parent(int r) {
+void SetHeadRequest<I>::handle_attach_parent(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
   if (r < 0) {
-    lderr(m_cct) << "failed to set parent: " << cpp_strerror(r) << dendl;
+    lderr(m_cct) << "failed to attach parent: " << cpp_strerror(r) << dendl;
     finish(r);
     return;
   }

diff --git a/src/librbd/deep_copy/SetHeadRequest.h b/src/librbd/deep_copy/SetHeadRequest.h
index 14d8013599aac97a21702518537aefaad2b681cf..cddad95785abae1f8d56836aa92a1d6e756bf507 100644 (file)
--- a/src/librbd/deep_copy/SetHeadRequest.h
+++ b/src/librbd/deep_copy/SetHeadRequest.h
@@ -49,7 +49,7 @@ private:
    * DETACH_PARENT
    *    |
    *    v (skip if not needed)
-   * SET_PARENT
+   * ATTACH_PARENT
    *    |
    *    v
    * <finish>
@@ -71,8 +71,8 @@ private:
   void send_detach_parent();
   void handle_detach_parent(int r);
 
-  void send_set_parent();
-  void handle_set_parent(int r);
+  void send_attach_parent();
+  void handle_attach_parent(int r);
 
   Context *start_lock_op();
 

diff --git a/src/librbd/image/CloneRequest.cc b/src/librbd/image/CloneRequest.cc
index 818f2354994a39c76424d35c9372957faebb2da8..f1b0acb72afaedf699f6f78d30befc4e0f6ac84e 100644 (file)
--- a/src/librbd/image/CloneRequest.cc
+++ b/src/librbd/image/CloneRequest.cc
@@ -7,6 +7,7 @@
 #include "common/errno.h"
 #include "include/ceph_assert.h"
 #include "librbd/ImageState.h"
+#include "librbd/image/AttachParentRequest.h"
 #include "librbd/image/CloneRequest.h"
 #include "librbd/image/CreateRequest.h"
 #include "librbd/image/RemoveRequest.h"
@@ -316,30 +317,27 @@ void CloneRequest<I>::handle_open_child(int r) {
     return;
   }
 
-  set_parent();
+  attach_parent();
 }
 
 template <typename I>
-void CloneRequest<I>::set_parent() {
+void CloneRequest<I>::attach_parent() {
   ldout(m_cct, 20) << dendl;
 
-  librados::ObjectWriteOperation op;
-  librbd::cls_client::set_parent(&op, m_pspec, m_size);
-
-  using klass = CloneRequest<I>;
-  librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_set_parent>(this);
-  int r = m_imctx->md_ctx.aio_operate(m_imctx->header_oid, comp, &op);
-  ceph_assert(r == 0);
-  comp->release();
+  auto ctx = create_context_callback<
+    CloneRequest<I>, &CloneRequest<I>::handle_attach_parent>(this);
+  auto req = AttachParentRequest<I>::create(
+    *m_imctx, {m_pspec.pool_id, "", m_pspec.image_id, m_pspec.snap_id},
+    m_size, ctx);
+  req->send();
 }
 
 template <typename I>
-void CloneRequest<I>::handle_set_parent(int r) {
+void CloneRequest<I>::handle_attach_parent(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
   if (r < 0) {
-    lderr(m_cct) << "couldn't set parent: " << cpp_strerror(r) << dendl;
+    lderr(m_cct) << "failed to attach parent: " << cpp_strerror(r) << dendl;
     m_r_saved = r;
     close_child();
     return;

diff --git a/src/librbd/image/CloneRequest.h b/src/librbd/image/CloneRequest.h
index 3707d9dc05a48561d23080af789a68a16b674cbd..afcbbd2386b9917085da2cd98bb9050c63999978 100644 (file)
--- a/src/librbd/image/CloneRequest.h
+++ b/src/librbd/image/CloneRequest.h
@@ -149,8 +149,8 @@ private:
   void open_child();
   void handle_open_child(int r);
 
-  void set_parent();
-  void handle_set_parent(int r);
+  void attach_parent();
+  void handle_attach_parent(int r);
 
   void v2_set_op_feature();
   void handle_v2_set_op_feature(int r);

diff --git a/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc b/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc
index c94ccf4214e22eeb39db3a52d2e5ded509d8f558..3eef98a6117df08f4f57baddbfa8b0368a682845 100644 (file)
--- a/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc
+++ b/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc
@@ -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/AttachParentRequest.h"
 #include "librbd/image/DetachParentRequest.h"
 
 namespace librbd {
@@ -25,6 +26,28 @@ struct MockTestImageCtx : public librbd::MockImageCtx {
 
 namespace image {
 
+template <>
+struct AttachParentRequest<MockTestImageCtx> {
+  Context* on_finish = nullptr;
+  static AttachParentRequest* s_instance;
+  static AttachParentRequest* create(MockTestImageCtx&,
+                                     const cls::rbd::ParentImageSpec& pspec,
+                                     uint64_t parent_overlap,
+                                     Context *on_finish) {
+    ceph_assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  MOCK_METHOD0(send, void());
+
+  AttachParentRequest() {
+    s_instance = this;
+  }
+};
+
+AttachParentRequest<MockTestImageCtx>* AttachParentRequest<MockTestImageCtx>::s_instance = nullptr;
+
 template <>
 class DetachParentRequest<MockTestImageCtx> {
 public:
@@ -70,6 +93,7 @@ using ::testing::WithArg;
 class TestMockDeepCopySetHeadRequest : public TestMockFixture {
 public:
   typedef SetHeadRequest<librbd::MockTestImageCtx> MockSetHeadRequest;
+  typedef image::AttachParentRequest<MockTestImageCtx> MockAttachParentRequest;
   typedef image::DetachParentRequest<MockTestImageCtx> MockDetachParentRequest;
 
   librbd::ImageCtx *m_image_ctx;
@@ -108,10 +132,10 @@ public:
       .WillOnce(FinishRequest(&mock_request, r, &mock_image_ctx));
   }
 
-  void expect_set_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("set_parent"), _, _, _))
-                  .WillOnce(Return(r));
+  void expect_attach_parent(MockImageCtx &mock_image_ctx,
+                            MockAttachParentRequest& mock_request, int r) {
+    EXPECT_CALL(mock_request, send())
+      .WillOnce(FinishRequest(&mock_request, r, &mock_image_ctx));
   }
 
   MockSetHeadRequest *create_request(
@@ -201,7 +225,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, RemoveSetParent) {
   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);
+  MockAttachParentRequest mock_attach_parent;
+  expect_attach_parent(mock_image_ctx, mock_attach_parent, 0);
 
   C_SaferCond ctx;
   auto request = create_request(mock_image_ctx, m_image_ctx->size,
@@ -217,7 +242,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, SetParentSpec) {
 
   InSequence seq;
   expect_start_op(mock_exclusive_lock);
-  expect_set_parent(mock_image_ctx, 0);
+  MockAttachParentRequest mock_attach_parent;
+  expect_attach_parent(mock_image_ctx, mock_attach_parent, 0);
 
   C_SaferCond ctx;
   auto request = create_request(mock_image_ctx, m_image_ctx->size,
@@ -253,7 +279,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, SetParentError) {
 
   InSequence seq;
   expect_start_op(mock_exclusive_lock);
-  expect_set_parent(mock_image_ctx, -ESTALE);
+  MockAttachParentRequest mock_attach_parent;
+  expect_attach_parent(mock_image_ctx, mock_attach_parent, -ESTALE);
 
   C_SaferCond ctx;
   auto request = create_request(mock_image_ctx, m_image_ctx->size,

diff --git a/src/test/librbd/image/test_mock_CloneRequest.cc b/src/test/librbd/image/test_mock_CloneRequest.cc
index 94b89265d72e68e4c16ce936b03fb8305dceb46a..7a103a1ecc2d7150893f4b632a7c2358d3ff6556 100644 (file)
--- a/src/test/librbd/image/test_mock_CloneRequest.cc
+++ b/src/test/librbd/image/test_mock_CloneRequest.cc
@@ -10,6 +10,7 @@
 #include "librbd/ImageState.h"
 #include "librbd/Operations.h"
 #include "librbd/image/TypeTraits.h"
+#include "librbd/image/AttachParentRequest.h"
 #include "librbd/image/CreateRequest.h"
 #include "librbd/image/RemoveRequest.h"
 #include "librbd/image/RefreshRequest.h"
@@ -48,6 +49,28 @@ MockTestImageCtx* MockTestImageCtx::s_instance = nullptr;
 
 namespace image {
 
+template <>
+struct AttachParentRequest<MockTestImageCtx> {
+  Context* on_finish = nullptr;
+  static AttachParentRequest* s_instance;
+  static AttachParentRequest* create(MockTestImageCtx&,
+                                     const cls::rbd::ParentImageSpec& pspec,
+                                     uint64_t parent_overlap,
+                                     Context *on_finish) {
+    ceph_assert(s_instance != nullptr);
+    s_instance->on_finish = on_finish;
+    return s_instance;
+  }
+
+  MOCK_METHOD0(send, void());
+
+  AttachParentRequest() {
+    s_instance = this;
+  }
+};
+
+AttachParentRequest<MockTestImageCtx>* AttachParentRequest<MockTestImageCtx>::s_instance = nullptr;
+
 template <>
 struct CreateRequest<MockTestImageCtx> {
   Context* on_finish = nullptr;
@@ -167,6 +190,7 @@ using ::testing::WithArg;
 class TestMockImageCloneRequest : public TestMockFixture {
 public:
   typedef CloneRequest<MockTestImageCtx> MockCloneRequest;
+  typedef AttachParentRequest<MockTestImageCtx> MockAttachParentRequest;
   typedef CreateRequest<MockTestImageCtx> MockCreateRequest;
   typedef RefreshRequest<MockTestImageCtx> MockRefreshRequest;
   typedef RemoveRequest<MockTestImageCtx> MockRemoveRequest;
@@ -236,12 +260,10 @@ public:
     }
   }
 
-  void expect_set_parent(MockImageCtx &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("set_parent"), _, _, _))
-      .WillOnce(InvokeWithoutArgs([r]() {
-                  return r;
+  void expect_attach_parent(MockAttachParentRequest& mock_request, int r) {
+    EXPECT_CALL(mock_request, send())
+      .WillOnce(Invoke([this, &mock_request, r]() {
+                  image_ctx->op_work_queue->queue(mock_request.on_finish, r);
                 }));
   }
 
@@ -372,7 +394,10 @@ TEST_F(TestMockImageCloneRequest, SuccessV1) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
+
   expect_add_child(m_ioctx, 0);
 
   MockRefreshRequest mock_refresh_request;
@@ -420,7 +445,9 @@ TEST_F(TestMockImageCloneRequest, SuccessV2) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
 
   expect_op_features_set(m_ioctx, mock_image_ctx.id, 0);
   expect_child_attach(mock_image_ctx, 0);
@@ -468,7 +495,10 @@ TEST_F(TestMockImageCloneRequest, SuccessAuto) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
+
   expect_add_child(m_ioctx, 0);
 
   MockRefreshRequest mock_refresh_request;
@@ -575,7 +605,7 @@ TEST_F(TestMockImageCloneRequest, OpenError) {
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
 
-TEST_F(TestMockImageCloneRequest, SetParentError) {
+TEST_F(TestMockImageCloneRequest, AttachParentError) {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
 
   MockTestImageCtx mock_image_ctx(*image_ctx);
@@ -591,7 +621,10 @@ TEST_F(TestMockImageCloneRequest, SetParentError) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, -EINVAL);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, -EINVAL);
+
   expect_close(mock_image_ctx, 0);
 
   MockRemoveRequest mock_remove_request;
@@ -625,7 +658,10 @@ TEST_F(TestMockImageCloneRequest, AddChildError) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
+
   expect_add_child(m_ioctx, -EINVAL);
   expect_close(mock_image_ctx, 0);
 
@@ -660,7 +696,10 @@ TEST_F(TestMockImageCloneRequest, RefreshError) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
+
   expect_add_child(m_ioctx, 0);
 
   MockRefreshRequest mock_refresh_request;
@@ -699,7 +738,10 @@ TEST_F(TestMockImageCloneRequest, MetadataListError) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
+
   expect_add_child(m_ioctx, 0);
 
   MockRefreshRequest mock_refresh_request;
@@ -740,7 +782,9 @@ TEST_F(TestMockImageCloneRequest, MetadataSetError) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
 
   expect_op_features_set(m_ioctx, mock_image_ctx.id, 0);
   expect_child_attach(mock_image_ctx, 0);
@@ -781,7 +825,10 @@ TEST_F(TestMockImageCloneRequest, GetMirrorModeError) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
+
   expect_add_child(m_ioctx, 0);
 
   MockRefreshRequest mock_refresh_request;
@@ -825,7 +872,9 @@ TEST_F(TestMockImageCloneRequest, MirrorEnableError) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
 
   expect_op_features_set(m_ioctx, mock_image_ctx.id, 0);
   expect_child_attach(mock_image_ctx, 0);
@@ -870,7 +919,9 @@ TEST_F(TestMockImageCloneRequest, CloseError) {
   expect_create(mock_create_request, 0);
 
   expect_open(mock_image_ctx, 0);
-  expect_set_parent(mock_image_ctx, 0);
+
+  MockAttachParentRequest mock_attach_parent_request;
+  expect_attach_parent(mock_attach_parent_request, 0);
 
   expect_op_features_set(m_ioctx, mock_image_ctx.id, 0);
   expect_child_attach(mock_image_ctx, 0);