From 3ec26ff84d3b622a95142587a29b511e48c4bc85 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 26 Apr 2018 12:24:27 -0400 Subject: [PATCH] rbd-mirror: rename asok hook to match image name when not replaying Fixes: http://tracker.ceph.com/issues/23888 Signed-off-by: Jason Dillaman --- .../test_mock_PrepareLocalImageRequest.cc | 48 +++++++++++++++++++ .../rbd_mirror/test_mock_ImageReplayer.cc | 33 +++++++------ src/tools/rbd_mirror/ImageReplayer.cc | 24 ++++++---- src/tools/rbd_mirror/ImageReplayer.h | 4 +- .../PrepareLocalImageRequest.cc | 36 ++++++++++++++ .../image_replayer/PrepareLocalImageRequest.h | 16 +++++-- 6 files changed, 132 insertions(+), 29 deletions(-) diff --git a/src/test/rbd_mirror/image_replayer/test_mock_PrepareLocalImageRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_PrepareLocalImageRequest.cc index f80cdffc7aa4c..cbbc9d7e6f050 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_PrepareLocalImageRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_PrepareLocalImageRequest.cc @@ -87,6 +87,19 @@ public: })); } + void expect_dir_get_name(librados::IoCtx &io_ctx, + const std::string &image_name, int r) { + bufferlist bl; + encode(image_name, bl); + + EXPECT_CALL(get_mock_io_ctx(io_ctx), + exec(RBD_DIRECTORY, _, StrEq("rbd"), StrEq("dir_get_name"), _, _, _)) + .WillOnce(DoAll(WithArg<5>(Invoke([bl](bufferlist *out_bl) { + *out_bl = bl; + })), + Return(r))); + } + void expect_mirror_image_get(librados::IoCtx &io_ctx, cls::rbd::MirrorImageState state, const std::string &global_id, int r) { @@ -122,6 +135,7 @@ TEST_F(TestMockImageReplayerPrepareLocalImageRequest, Success) { MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "local image id", 0); + expect_dir_get_name(m_local_io_ctx, "local image name", 0); expect_mirror_image_get(m_local_io_ctx, cls::rbd::MIRROR_IMAGE_STATE_ENABLED, "global image id", 0); @@ -129,11 +143,13 @@ TEST_F(TestMockImageReplayerPrepareLocalImageRequest, Success) { expect_get_tag_owner(mock_journal, "local image id", "remote mirror uuid", 0); std::string local_image_id; + std::string local_image_name; std::string tag_owner; C_SaferCond ctx; auto req = MockPrepareLocalImageRequest::create(m_local_io_ctx, "global image id", &local_image_id, + &local_image_name, &tag_owner, m_threads->work_queue, &ctx); @@ -141,6 +157,7 @@ TEST_F(TestMockImageReplayerPrepareLocalImageRequest, Success) { ASSERT_EQ(0, ctx.wait()); ASSERT_EQ(std::string("local image id"), local_image_id); + ASSERT_EQ(std::string("local image name"), local_image_name); ASSERT_EQ(std::string("remote mirror uuid"), tag_owner); } @@ -150,11 +167,13 @@ TEST_F(TestMockImageReplayerPrepareLocalImageRequest, MirrorImageIdError) { expect_get_mirror_image_id(mock_get_mirror_image_id_request, "", -EINVAL); std::string local_image_id; + std::string local_image_name; std::string tag_owner; C_SaferCond ctx; auto req = MockPrepareLocalImageRequest::create(m_local_io_ctx, "global image id", &local_image_id, + &local_image_name, &tag_owner, m_threads->work_queue, &ctx); @@ -163,20 +182,46 @@ TEST_F(TestMockImageReplayerPrepareLocalImageRequest, MirrorImageIdError) { ASSERT_EQ(-EINVAL, ctx.wait()); } +TEST_F(TestMockImageReplayerPrepareLocalImageRequest, DirGetNameError) { + InSequence seq; + MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; + expect_get_mirror_image_id(mock_get_mirror_image_id_request, "local image id", + 0); + expect_dir_get_name(m_local_io_ctx, "", -ENOENT); + + std::string local_image_id; + std::string local_image_name; + std::string tag_owner; + C_SaferCond ctx; + auto req = MockPrepareLocalImageRequest::create(m_local_io_ctx, + "global image id", + &local_image_id, + &local_image_name, + &tag_owner, + m_threads->work_queue, + &ctx); + req->send(); + + ASSERT_EQ(-ENOENT, ctx.wait()); +} + TEST_F(TestMockImageReplayerPrepareLocalImageRequest, MirrorImageError) { InSequence seq; MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "local image id", 0); + expect_dir_get_name(m_local_io_ctx, "local image name", 0); expect_mirror_image_get(m_local_io_ctx, cls::rbd::MIRROR_IMAGE_STATE_DISABLED, "", -EINVAL); std::string local_image_id; + std::string local_image_name; std::string tag_owner; C_SaferCond ctx; auto req = MockPrepareLocalImageRequest::create(m_local_io_ctx, "global image id", &local_image_id, + &local_image_name, &tag_owner, m_threads->work_queue, &ctx); @@ -190,6 +235,7 @@ TEST_F(TestMockImageReplayerPrepareLocalImageRequest, TagOwnerError) { MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "local image id", 0); + expect_dir_get_name(m_local_io_ctx, "local image name", 0); expect_mirror_image_get(m_local_io_ctx, cls::rbd::MIRROR_IMAGE_STATE_ENABLED, "global image id", 0); @@ -198,11 +244,13 @@ TEST_F(TestMockImageReplayerPrepareLocalImageRequest, TagOwnerError) { -ENOENT); std::string local_image_id; + std::string local_image_name; std::string tag_owner; C_SaferCond ctx; auto req = MockPrepareLocalImageRequest::create(m_local_io_ctx, "global image id", &local_image_id, + &local_image_name, &tag_owner, m_threads->work_queue, &ctx); diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc index 2d510352ce460..fd43290c6760b 100644 --- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc @@ -124,17 +124,20 @@ template<> struct PrepareLocalImageRequest { static PrepareLocalImageRequest* s_instance; std::string *local_image_id = nullptr; + std::string *local_image_name = nullptr; std::string *tag_owner = nullptr; Context *on_finish = nullptr; static PrepareLocalImageRequest* create(librados::IoCtx &, const std::string &global_image_id, std::string *local_image_id, + std::string *local_image_name, std::string *tag_owner, MockContextWQ *work_queue, Context *on_finish) { assert(s_instance != nullptr); s_instance->local_image_id = local_image_id; + s_instance->local_image_name = local_image_name; s_instance->tag_owner = tag_owner; s_instance->on_finish = on_finish; return s_instance; @@ -417,12 +420,14 @@ public: void expect_send(MockPrepareLocalImageRequest &mock_request, const std::string &local_image_id, + const std::string &local_image_name, const std::string &tag_owner, int r) { EXPECT_CALL(mock_request, send()) - .WillOnce(Invoke([&mock_request, local_image_id, tag_owner, r]() { + .WillOnce(Invoke([&mock_request, local_image_id, local_image_name, tag_owner, r]() { if (r == 0) { *mock_request.local_image_id = local_image_id; + *mock_request.local_image_name = local_image_name; *mock_request.tag_owner = tag_owner; } mock_request.on_finish->complete(r); @@ -614,7 +619,7 @@ TEST_F(TestMockImageReplayer, StartStop) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); @@ -677,7 +682,7 @@ TEST_F(TestMockImageReplayer, LocalImagePrimary) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "", 0); + mock_local_image_ctx.name, "", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", "remote image id", 0); EXPECT_CALL(mock_remote_journaler, construct()); @@ -709,7 +714,7 @@ TEST_F(TestMockImageReplayer, LocalImageDNE) { expect_get_or_send_update(mock_replay_status_formatter); InSequence seq; - expect_send(mock_prepare_local_image_request, "", "", -ENOENT); + expect_send(mock_prepare_local_image_request, "", "", "", -ENOENT); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); @@ -741,7 +746,7 @@ TEST_F(TestMockImageReplayer, PrepareLocalImageError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", -EINVAL); + mock_local_image_ctx.name, "remote mirror uuid", -EINVAL); create_image_replayer(mock_threads); @@ -767,7 +772,7 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdDNE) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", "", -ENOENT); expect_trash_move(mock_image_deleter, "global image id", false, 0); @@ -796,7 +801,7 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdNonLinkedDNE) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "some other mirror uuid", 0); + mock_local_image_ctx.name, "some other mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", "", -ENOENT); @@ -824,7 +829,7 @@ TEST_F(TestMockImageReplayer, GetRemoteImageIdError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, -EINVAL); @@ -855,7 +860,7 @@ TEST_F(TestMockImageReplayer, BootstrapError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); @@ -897,7 +902,7 @@ TEST_F(TestMockImageReplayer, StartExternalReplayError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); @@ -954,7 +959,7 @@ TEST_F(TestMockImageReplayer, StopError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); @@ -1024,7 +1029,7 @@ TEST_F(TestMockImageReplayer, Replay) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); @@ -1131,7 +1136,7 @@ TEST_F(TestMockImageReplayer, DecodeError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); @@ -1232,7 +1237,7 @@ TEST_F(TestMockImageReplayer, DelayedReplay) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, - "remote mirror uuid", 0); + mock_local_image_ctx.name, "remote mirror uuid", 0); expect_send(mock_prepare_remote_image_request, "remote mirror uuid", m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 482f5e073f795..3176edaea21ec 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -270,7 +270,7 @@ ImageReplayer::ImageReplayer(Threads *threads, m_local(local), m_local_mirror_uuid(local_mirror_uuid), m_local_pool_id(local_pool_id), - m_global_image_id(global_image_id), + m_global_image_id(global_image_id), m_local_image_name(global_image_id), m_lock("rbd::mirror::ImageReplayer " + stringify(local_pool_id) + " " + global_image_id), m_progress_cxt(this), @@ -402,7 +402,7 @@ void ImageReplayer::prepare_local_image() { Context *ctx = create_context_callback< ImageReplayer, &ImageReplayer::handle_prepare_local_image>(this); auto req = PrepareLocalImageRequest::create( - *m_local_ioctx, m_global_image_id, &m_local_image_id, + *m_local_ioctx, m_global_image_id, &m_local_image_id, &m_local_image_name, &m_local_image_tag_owner, m_threads->work_queue, ctx); req->send(); } @@ -416,6 +416,8 @@ void ImageReplayer::handle_prepare_local_image(int r) { } else if (r < 0) { on_start_fail(r, "error preparing local image for replay"); return; + } else { + reregister_admin_socket_hook(); } // local image doesn't exist or is non-primary @@ -558,8 +560,6 @@ void ImageReplayer::handle_bootstrap(int r) { return; } - on_name_changed(); - update_mirror_image_status(false, boost::none); init_remote_journaler(); } @@ -1198,7 +1198,12 @@ void ImageReplayer::handle_process_entry_ready(int r) { dout(20) << dendl; assert(r == 0); - on_name_changed(); + { + RWLock::RLocker snap_locker(m_local_image_ctx->snap_lock); + m_local_image_name = m_local_image_ctx->name; + } + + reregister_admin_socket_hook(); // attempt to process the next event handle_replay_ready(); @@ -1745,7 +1750,7 @@ void ImageReplayer::register_admin_socket_hook() { return; } - dout(20) << "registered asok hook: " << m_name << dendl; + dout(15) << "registered asok hook: " << m_name << dendl; asok_hook = new ImageReplayerAdminSocketHook(g_ceph_context, m_name, this); int r = asok_hook->register_commands(); @@ -1760,7 +1765,7 @@ void ImageReplayer::register_admin_socket_hook() { template void ImageReplayer::unregister_admin_socket_hook() { - dout(20) << dendl; + dout(15) << dendl; AdminSocketHook *asok_hook = nullptr; { @@ -1771,11 +1776,10 @@ void ImageReplayer::unregister_admin_socket_hook() { } template -void ImageReplayer::on_name_changed() { +void ImageReplayer::reregister_admin_socket_hook() { { Mutex::Locker locker(m_lock); - std::string name = m_local_ioctx->get_pool_name() + "/" + - m_local_image_ctx->name; + auto name = m_local_ioctx->get_pool_name() + "/" + m_local_image_name; if (m_name == name) { return; } diff --git a/src/tools/rbd_mirror/ImageReplayer.h b/src/tools/rbd_mirror/ImageReplayer.h index 41aa3774d5f2e..1b24376146b56 100644 --- a/src/tools/rbd_mirror/ImageReplayer.h +++ b/src/tools/rbd_mirror/ImageReplayer.h @@ -279,6 +279,7 @@ private: int64_t m_local_pool_id; std::string m_local_image_id; std::string m_global_image_id; + std::string m_local_image_name; std::string m_name; mutable Mutex m_lock; @@ -417,8 +418,7 @@ private: void register_admin_socket_hook(); void unregister_admin_socket_hook(); - - void on_name_changed(); + void reregister_admin_socket_hook(); }; } // namespace mirror diff --git a/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.cc b/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.cc index a86d982ba31b1..b65ccb1aa0c00 100644 --- a/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.cc @@ -55,6 +55,42 @@ void PrepareLocalImageRequest::handle_get_local_image_id(int r) { return; } + get_local_image_name(); +} + +template +void PrepareLocalImageRequest::get_local_image_name() { + dout(20) << dendl; + + librados::ObjectReadOperation op; + librbd::cls_client::dir_get_name_start(&op, *m_local_image_id); + + m_out_bl.clear(); + librados::AioCompletion *aio_comp = create_rados_callback< + PrepareLocalImageRequest, + &PrepareLocalImageRequest::handle_get_local_image_name>(this); + int r = m_io_ctx.aio_operate(RBD_DIRECTORY, aio_comp, &op, &m_out_bl); + assert(r == 0); + aio_comp->release(); +} + +template +void PrepareLocalImageRequest::handle_get_local_image_name(int r) { + dout(20) << "r=" << r << dendl; + + if (r == 0) { + bufferlist::iterator it = m_out_bl.begin(); + r = librbd::cls_client::dir_get_name_finish(&it, m_local_image_name); + } + + if (r < 0) { + if (r != -ENOENT) { + derr << "failed to retrieve image name: " << cpp_strerror(r) << dendl; + } + finish(r); + return; + } + get_mirror_state(); } diff --git a/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.h b/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.h index 913bfd1c242c2..33ba24f4e7bfc 100644 --- a/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.h +++ b/src/tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.h @@ -23,22 +23,25 @@ public: static PrepareLocalImageRequest *create(librados::IoCtx &io_ctx, const std::string &global_image_id, std::string *local_image_id, + std::string *local_image_name, std::string *tag_owner, ContextWQ *work_queue, Context *on_finish) { return new PrepareLocalImageRequest(io_ctx, global_image_id, local_image_id, - tag_owner, work_queue, on_finish); + local_image_name, tag_owner, work_queue, + on_finish); } PrepareLocalImageRequest(librados::IoCtx &io_ctx, const std::string &global_image_id, std::string *local_image_id, + std::string *local_image_name, std::string *tag_owner, ContextWQ *work_queue, Context *on_finish) : m_io_ctx(io_ctx), m_global_image_id(global_image_id), - m_local_image_id(local_image_id), m_tag_owner(tag_owner), - m_work_queue(work_queue), m_on_finish(on_finish) { + m_local_image_id(local_image_id), m_local_image_name(local_image_name), + m_tag_owner(tag_owner), m_work_queue(work_queue), m_on_finish(on_finish) { } void send(); @@ -53,6 +56,9 @@ private: * GET_LOCAL_IMAGE_ID * | * v + * GET_LOCAL_IMAGE_NAME + * | + * v * GET_MIRROR_STATE * | * v @@ -64,6 +70,7 @@ private: librados::IoCtx &m_io_ctx; std::string m_global_image_id; std::string *m_local_image_id; + std::string *m_local_image_name; std::string *m_tag_owner; ContextWQ *m_work_queue; Context *m_on_finish; @@ -73,6 +80,9 @@ private: void get_local_image_id(); void handle_get_local_image_id(int r); + void get_local_image_name(); + void handle_get_local_image_name(int r); + void get_mirror_state(); void handle_get_mirror_state(int r); -- 2.39.5