From: Mykola Golub Date: Sun, 27 May 2018 11:58:27 +0000 (+0300) Subject: librbd: use 'open snap by id' in clone request X-Git-Tag: v14.0.1~590^2~7 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=894ffca543894d5a8805d26832b450a9edc233d1;p=ceph.git librbd: use 'open snap by id' in clone request Signed-off-by: Mykola Golub --- diff --git a/src/librbd/image/CloneRequest.cc b/src/librbd/image/CloneRequest.cc index 7d6ef5764eaf4..279a46d58dc1d 100644 --- a/src/librbd/image/CloneRequest.cc +++ b/src/librbd/image/CloneRequest.cc @@ -117,8 +117,16 @@ void CloneRequest::validate_options() { template void CloneRequest::open_parent() { ldout(m_cct, 20) << dendl; + assert(m_parent_snap_name.empty() ^ (m_parent_snap_id == CEPH_NOSNAP)); - m_parent_image_ctx = I::create("", m_parent_image_id, "", m_parent_io_ctx, true); + if (m_parent_snap_id != CEPH_NOSNAP) { + m_parent_image_ctx = I::create("", m_parent_image_id, m_parent_snap_id, + m_parent_io_ctx, true); + } else { + m_parent_image_ctx = I::create("", m_parent_image_id, + m_parent_snap_name.c_str(), m_parent_io_ctx, + true); + } Context *ctx = create_context_callback< CloneRequest, &CloneRequest::handle_open_parent>(this); @@ -138,52 +146,7 @@ void CloneRequest::handle_open_parent(int r) { return; } - set_parent_snap(); -} - -template -void CloneRequest::set_parent_snap() { - assert((m_parent_snap_name.empty()) ^ (m_parent_snap_id == CEPH_NOSNAP)); - - if (m_parent_snap_id == CEPH_NOSNAP) { - // look up user snapshot by name - m_parent_image_ctx->snap_lock.get_read(); - auto it = m_parent_image_ctx->snap_ids.find( - {cls::rbd::UserSnapshotNamespace{}, m_parent_snap_name}); - if (it == m_parent_image_ctx->snap_ids.end()) { - m_parent_image_ctx->snap_lock.put_read(); - - lderr(m_cct) << "failed to located parent snapshot: " << - m_parent_snap_name << dendl; - m_r_saved = -ENOENT; - close_parent(); - return; - } - - m_parent_snap_id = it->second; - m_parent_image_ctx->snap_lock.put_read(); - } - - ldout(m_cct, 20) << "parent_snap_id=" << m_parent_snap_id << dendl; - - Context *ctx = create_context_callback< - CloneRequest, &CloneRequest::handle_set_parent_snap>(this); - m_parent_image_ctx->state->snap_set(m_parent_snap_id, ctx); -} - -template -void CloneRequest::handle_set_parent_snap(int r) { - ldout(m_cct, 20) << "r=" << r << dendl; - - if (r < 0) { - lderr(m_cct) << "failed to set parent snapshot: " << cpp_strerror(r) - << dendl; - - m_r_saved = r; - close_parent(); - return; - } - + m_parent_snap_id = m_parent_image_ctx->snap_id; m_pspec = {m_parent_io_ctx.get_id(), m_parent_image_id, m_parent_snap_id}; validate_parent(); } diff --git a/src/librbd/image/CloneRequest.h b/src/librbd/image/CloneRequest.h index d98ebc4f6487f..3707d9dc05a48 100644 --- a/src/librbd/image/CloneRequest.h +++ b/src/librbd/image/CloneRequest.h @@ -52,17 +52,14 @@ private: * * | * v - * OPEN PARENT + * OPEN PARENT + * | + * v + * VALIDATE CHILD * | ^ * v | - * SET PARENT SNAP * * * * * * * > CLOSE PARENT - * | * ^ - * v * | - * VALIDATE CHILD * | - * | * | - * v * | - * CREATE CHILD * * * * * * * * | - * | | + * CREATE CHILD * * * * * * * * * > CLOSE PARENT + * | ^ * v | * OPEN CHILD * * * * * * * * * * > REMOVE CHILD * | ^ @@ -141,9 +138,6 @@ private: void open_parent(); void handle_open_parent(int r); - void set_parent_snap(); - void handle_set_parent_snap(int r); - void validate_parent(); void validate_child(); diff --git a/src/test/librbd/image/test_mock_CloneRequest.cc b/src/test/librbd/image/test_mock_CloneRequest.cc index f149008ebbb5f..8c4d270701316 100644 --- a/src/test/librbd/image/test_mock_CloneRequest.cc +++ b/src/test/librbd/image/test_mock_CloneRequest.cc @@ -29,6 +29,13 @@ struct MockTestImageCtx : public MockImageCtx { assert(s_instance != nullptr); return s_instance; } + static MockTestImageCtx* create(const std::string &image_name, + const std::string &image_id, + librados::snap_t snap_id, IoCtx& p, + bool read_only) { + assert(s_instance != nullptr); + return s_instance; + } MockTestImageCtx(ImageCtx &image_ctx) : MockImageCtx(image_ctx) { s_instance = this; @@ -229,14 +236,6 @@ public: } } - void expect_snap_set(librbd::MockTestImageCtx &mock_image_ctx, - uint64_t snap_id, int r) { - EXPECT_CALL(*mock_image_ctx.state, snap_set(snap_id, _)) - .WillOnce(WithArg<1>(Invoke([this, r](Context *on_finish) { - image_ctx->op_work_queue->queue(on_finish, r); - }))); - } - 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"), @@ -364,7 +363,6 @@ TEST_F(TestMockImageCloneRequest, SuccessV1) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -413,7 +411,6 @@ TEST_F(TestMockImageCloneRequest, SuccessV2) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -462,7 +459,6 @@ TEST_F(TestMockImageCloneRequest, SuccessAuto) { InSequence seq; expect_get_min_compat_client(1, 0); expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -521,26 +517,6 @@ TEST_F(TestMockImageCloneRequest, OpenParentError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockImageCloneRequest, SetParentSnapError) { - REQUIRE_FEATURE(RBD_FEATURE_LAYERING); - - MockTestImageCtx mock_image_ctx(*image_ctx); - expect_op_work_queue(mock_image_ctx); - - InSequence seq; - expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, -EINVAL); - expect_close(mock_image_ctx, 0); - - C_SaferCond ctx; - ImageOptions clone_opts; - auto req = new MockCloneRequest(m_ioctx, "parent id", "", 123, m_ioctx, - "clone name", "clone id", clone_opts, "", "", - image_ctx->op_work_queue, &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - TEST_F(TestMockImageCloneRequest, CreateError) { REQUIRE_FEATURE(RBD_FEATURE_LAYERING); @@ -549,7 +525,6 @@ TEST_F(TestMockImageCloneRequest, CreateError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -576,7 +551,6 @@ TEST_F(TestMockImageCloneRequest, OpenError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -608,7 +582,6 @@ TEST_F(TestMockImageCloneRequest, SetParentError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -643,7 +616,6 @@ TEST_F(TestMockImageCloneRequest, AddChildError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -679,7 +651,6 @@ TEST_F(TestMockImageCloneRequest, RefreshError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -719,7 +690,6 @@ TEST_F(TestMockImageCloneRequest, MetadataListError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -761,7 +731,6 @@ TEST_F(TestMockImageCloneRequest, MetadataSetError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -803,7 +772,6 @@ TEST_F(TestMockImageCloneRequest, GetMirrorModeError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -848,7 +816,6 @@ TEST_F(TestMockImageCloneRequest, MirrorEnableError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -894,7 +861,6 @@ TEST_F(TestMockImageCloneRequest, CloseError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -935,7 +901,6 @@ TEST_F(TestMockImageCloneRequest, RemoveError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0); @@ -967,7 +932,6 @@ TEST_F(TestMockImageCloneRequest, CloseParentError) { InSequence seq; expect_open(mock_image_ctx, 0); - expect_snap_set(mock_image_ctx, 123, 0); expect_get_image_size(mock_image_ctx, mock_image_ctx.snaps.front(), 123); expect_is_snap_protected(mock_image_ctx, true, 0);