From b311fce34dc862a93bd1a58b4f37b29ac6b1da48 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 18 Jul 2017 14:15:05 -0400 Subject: [PATCH] rbd-mirror: image deletion should ensure journal is owned by remote peer Signed-off-by: Jason Dillaman --- src/test/rbd_mirror/test_ImageDeleter.cc | 156 ++++++++++++------ .../rbd_mirror/test_mock_ImageReplayer.cc | 4 +- .../rbd_mirror/test_mock_InstanceReplayer.cc | 4 +- src/tools/rbd_mirror/ImageDeleter.cc | 31 ++-- src/tools/rbd_mirror/ImageDeleter.h | 11 +- src/tools/rbd_mirror/ImageReplayer.cc | 3 +- src/tools/rbd_mirror/InstanceReplayer.cc | 2 +- 7 files changed, 136 insertions(+), 75 deletions(-) diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc index 56f1dde032d53..5ac673314233f 100644 --- a/src/test/rbd_mirror/test_ImageDeleter.cc +++ b/src/test/rbd_mirror/test_ImageDeleter.cc @@ -27,6 +27,7 @@ #include "librbd/internal.h" #include "librbd/Utils.h" #include "librbd/api/Mirror.h" +#include "librbd/journal/DisabledPolicy.h" #include "test/rbd_mirror/test_fixture.h" #include "test/librados/test.h" @@ -72,19 +73,17 @@ public: &m_threads->timer_lock, m_service_daemon.get()); - EXPECT_EQ(0, create_image(rbd, m_local_io_ctx, m_image_name, 1 << 20)); - ImageCtx *ictx = new ImageCtx(m_image_name, "", "", m_local_io_ctx, - false); - EXPECT_EQ(0, ictx->state->open(false)); - m_local_image_id = ictx->id; + m_local_image_id = librbd::util::generate_image_id(m_local_io_ctx); + librbd::ImageOptions image_opts; + image_opts.set(RBD_IMAGE_OPTION_FEATURES, RBD_FEATURES_ALL); + EXPECT_EQ(0, librbd::create(m_local_io_ctx, m_image_name, m_local_image_id, + 1 << 20, image_opts, GLOBAL_IMAGE_ID, + m_remote_mirror_uuid, true)); - cls::rbd::MirrorImage mirror_image(GLOBAL_IMAGE_ID, - MirrorImageState::MIRROR_IMAGE_STATE_ENABLED); - EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, ictx->id, + cls::rbd::MirrorImage mirror_image( + GLOBAL_IMAGE_ID, MirrorImageState::MIRROR_IMAGE_STATE_ENABLED); + EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, m_local_image_id, mirror_image)); - - demote_image(ictx); - EXPECT_EQ(0, ictx->state->close()); } void TearDown() override { @@ -156,17 +155,19 @@ public: ImageCtx *ictx = new ImageCtx("", m_local_image_id, "", m_local_io_ctx, false); EXPECT_EQ(0, ictx->state->open(false)); - promote_image(ictx); + { + RWLock::WLocker snap_locker(ictx->snap_lock); + ictx->set_journal_policy(new librbd::journal::DisabledPolicy()); + } - EXPECT_EQ(0, ictx->operations->snap_create(cls::rbd::UserSnapshotNamespace(), - snap_name.c_str())); + EXPECT_EQ(0, ictx->operations->snap_create( + cls::rbd::UserSnapshotNamespace(), snap_name.c_str())); if (protect) { - EXPECT_EQ(0, ictx->operations->snap_protect(cls::rbd::UserSnapshotNamespace(), - snap_name.c_str())); + EXPECT_EQ(0, ictx->operations->snap_protect( + cls::rbd::UserSnapshotNamespace(), snap_name.c_str())); } - demote_image(ictx); EXPECT_EQ(0, ictx->state->close()); } @@ -174,31 +175,30 @@ public: ImageCtx *ictx = new ImageCtx("", m_local_image_id, "", m_local_io_ctx, false); EXPECT_EQ(0, ictx->state->open(false)); - promote_image(ictx); - - EXPECT_EQ(0, ictx->operations->snap_create(cls::rbd::UserSnapshotNamespace(), - "snap1")); - EXPECT_EQ(0, ictx->operations->snap_protect(cls::rbd::UserSnapshotNamespace(), - "snap1")); - int order = 20; - EXPECT_EQ(0, librbd::clone(m_local_io_ctx, ictx->name.c_str(), "snap1", - m_local_io_ctx, "clone1", ictx->features, - &order, 0, 0)); - std::string clone_id; - ImageCtx *ictx_clone = new ImageCtx("clone1", "", "", m_local_io_ctx, - false); - EXPECT_EQ(0, ictx_clone->state->open(false)); - clone_id = ictx_clone->id; - cls::rbd::MirrorImage mirror_image(GLOBAL_CLONE_IMAGE_ID, - MirrorImageState::MIRROR_IMAGE_STATE_ENABLED); + { + RWLock::WLocker snap_locker(ictx->snap_lock); + ictx->set_journal_policy(new librbd::journal::DisabledPolicy()); + } + + EXPECT_EQ(0, ictx->operations->snap_create( + cls::rbd::UserSnapshotNamespace(), "snap1")); + EXPECT_EQ(0, ictx->operations->snap_protect( + cls::rbd::UserSnapshotNamespace(), "snap1")); + EXPECT_EQ(0, librbd::snap_set(ictx, cls::rbd::UserSnapshotNamespace(), + "snap1")); + + std::string clone_id = librbd::util::generate_image_id(m_local_io_ctx); + librbd::ImageOptions clone_opts; + clone_opts.set(RBD_IMAGE_OPTION_FEATURES, ictx->features); + EXPECT_EQ(0, librbd::clone(ictx, m_local_io_ctx, "clone1", clone_id, + clone_opts, GLOBAL_CLONE_IMAGE_ID, + m_remote_mirror_uuid)); + + cls::rbd::MirrorImage mirror_image( + GLOBAL_CLONE_IMAGE_ID, MirrorImageState::MIRROR_IMAGE_STATE_ENABLED); EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, clone_id, mirror_image)); - demote_image(ictx_clone); - EXPECT_EQ(0, ictx_clone->state->close()); - - demote_image(ictx); EXPECT_EQ(0, ictx->state->close()); - return clone_id; } @@ -224,7 +224,26 @@ int64_t TestImageDeleter::m_local_pool_id; TEST_F(TestImageDeleter, Delete_NonPrimary_Image) { - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); + + C_SaferCond ctx; + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); + EXPECT_EQ(0, ctx.wait()); + + ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); + ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); + + check_image_deleted(); +} + +TEST_F(TestImageDeleter, Delete_Split_Brain_Image) { + promote_image(); + demote_image(); + + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + true); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -240,7 +259,24 @@ TEST_F(TestImageDeleter, Delete_NonPrimary_Image) { TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) { promote_image(); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); + + C_SaferCond ctx; + m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, + &ctx); + EXPECT_EQ(-rbd::mirror::ImageDeleter<>::EISPRM, ctx.wait()); + + ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); + ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); +} + +TEST_F(TestImageDeleter, Fail_Delete_Orphan_Image) { + promote_image(); + demote_image(); + + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -254,7 +290,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) { TEST_F(TestImageDeleter, Delete_Image_With_Child) { create_snapshot(); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -269,7 +306,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) { create_snapshot("snap1"); create_snapshot("snap2"); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -283,7 +321,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) { TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChild) { create_snapshot("snap1", true); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -298,7 +337,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) { create_snapshot("snap1", true); create_snapshot("snap2", true); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -312,7 +352,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) { TEST_F(TestImageDeleter, Delete_Image_With_Clone) { std::string clone_id = create_clone(); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -323,7 +364,7 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) { ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); m_deleter->schedule_image_delete(_rados, m_local_pool_id, - GLOBAL_CLONE_IMAGE_ID); + GLOBAL_CLONE_IMAGE_ID, false); C_SaferCond ctx2; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_CLONE_IMAGE_ID, @@ -347,7 +388,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image) { EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, m_local_image_id, mirror_image)); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -371,7 +413,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) { EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, m_local_image_id, mirror_image)); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -387,7 +430,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) { TEST_F(TestImageDeleter, Delete_NonExistent_Image_Without_MirroringState) { remove_image(); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -405,7 +449,8 @@ TEST_F(TestImageDeleter, Fail_Delete_NonPrimary_Image) { false); EXPECT_EQ(0, ictx->state->open(false)); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -425,7 +470,8 @@ TEST_F(TestImageDeleter, Retry_Failed_Deletes) { m_deleter->set_failed_timer_interval(2); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -450,7 +496,8 @@ TEST_F(TestImageDeleter, Delete_Is_Idempotent) { false); EXPECT_EQ(0, ictx->state->open(false)); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -460,7 +507,8 @@ TEST_F(TestImageDeleter, Delete_Is_Idempotent) { ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); ASSERT_EQ(1u, m_deleter->get_failed_queue_items().size()); - m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID); + m_deleter->schedule_image_delete(_rados, m_local_pool_id, GLOBAL_IMAGE_ID, + false); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); ASSERT_EQ(1u, m_deleter->get_failed_queue_items().size()); diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc index a80ec323ed322..71a94e5896b3e 100644 --- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc @@ -63,8 +63,8 @@ namespace mirror { template <> struct ImageDeleter { - MOCK_METHOD3(schedule_image_delete, void(RadosRef, int64_t, - const std::string&)); + MOCK_METHOD4(schedule_image_delete, void(RadosRef, int64_t, + const std::string&, bool)); }; template<> diff --git a/src/test/rbd_mirror/test_mock_InstanceReplayer.cc b/src/test/rbd_mirror/test_mock_InstanceReplayer.cc index cda56eb4c165d..6e2d58c0aa0e2 100644 --- a/src/test/rbd_mirror/test_mock_InstanceReplayer.cc +++ b/src/test/rbd_mirror/test_mock_InstanceReplayer.cc @@ -42,8 +42,8 @@ struct Threads { template <> struct ImageDeleter { - MOCK_METHOD3(schedule_image_delete, void(RadosRef, int64_t, - const std::string&)); + MOCK_METHOD4(schedule_image_delete, void(RadosRef, int64_t, + const std::string&, bool)); MOCK_METHOD4(wait_for_scheduled_deletion, void(int64_t, const std::string&, Context*, bool)); }; diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index 611b0c20948b0..07de6e93c38cf 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -206,7 +206,8 @@ void ImageDeleter::run() { template void ImageDeleter::schedule_image_delete(RadosRef local_rados, int64_t local_pool_id, - const std::string& global_image_id) { + const std::string& global_image_id, + bool ignore_orphaned) { dout(20) << "enter" << dendl; Mutex::Locker locker(m_delete_lock); @@ -215,12 +216,15 @@ void ImageDeleter::schedule_image_delete(RadosRef local_rados, if (del_info != nullptr) { dout(20) << "image " << global_image_id << " " << "was already scheduled for deletion" << dendl; + if (ignore_orphaned) { + (*del_info)->ignore_orphaned = true; + } return; } m_delete_queue.push_front( unique_ptr(new DeleteInfo(local_rados, local_pool_id, - global_image_id))); + global_image_id, ignore_orphaned))); m_delete_queue_cond.Signal(); } @@ -304,25 +308,30 @@ bool ImageDeleter::process_image_delete() { return true; } - bool is_primary = false; + std::string mirror_uuid; C_SaferCond tag_owner_ctx; - Journal<>::is_tag_owner(ioctx, local_image_id, &is_primary, - m_work_queue, &tag_owner_ctx); + Journal<>::get_tag_owner(ioctx, local_image_id, &mirror_uuid, m_work_queue, + &tag_owner_ctx); r = tag_owner_ctx.wait(); if (r < 0 && r != -ENOENT) { derr << "error retrieving image primary info for image " << global_image_id << ": " << cpp_strerror(r) << dendl; enqueue_failed_delete(r); return true; - } - if (is_primary) { - dout(10) << "image " << global_image_id << " is local primary" << dendl; - complete_active_delete(-EISPRM); - return true; + } else if (r != -ENOENT) { + if (mirror_uuid == Journal<>::LOCAL_MIRROR_UUID) { + dout(10) << "image " << global_image_id << " is local primary" << dendl; + complete_active_delete(-EISPRM); + return true; + } else if (mirror_uuid == Journal<>::ORPHAN_MIRROR_UUID && + !m_active_delete->ignore_orphaned) { + dout(10) << "image " << global_image_id << " is orphaned" << dendl; + complete_active_delete(-EISPRM); + return true; + } } dout(20) << "local image is not the primary" << dendl; - bool has_snapshots; r = image_has_snapshots_and_children(&ioctx, local_image_id, &has_snapshots); if (r < 0) { diff --git a/src/tools/rbd_mirror/ImageDeleter.h b/src/tools/rbd_mirror/ImageDeleter.h index 039c7a11c03a4..d3df9ed633048 100644 --- a/src/tools/rbd_mirror/ImageDeleter.h +++ b/src/tools/rbd_mirror/ImageDeleter.h @@ -50,7 +50,8 @@ public: void schedule_image_delete(RadosRef local_rados, int64_t local_pool_id, - const std::string& global_image_id); + const std::string& global_image_id, + bool ignore_orphaned); void wait_for_scheduled_deletion(int64_t local_pool_id, const std::string &global_image_id, Context *ctx, @@ -82,15 +83,17 @@ private: RadosRef local_rados; int64_t local_pool_id; std::string global_image_id; + bool ignore_orphaned; int error_code = 0; int retries = 0; bool notify_on_failed_retry = true; Context *on_delete = nullptr; DeleteInfo(RadosRef local_rados, int64_t local_pool_id, - const std::string& global_image_id) : - local_rados(local_rados), local_pool_id(local_pool_id), - global_image_id(global_image_id) { + const std::string& global_image_id, + bool ignore_orphaned) + : local_rados(local_rados), local_pool_id(local_pool_id), + global_image_id(global_image_id), ignore_orphaned(ignore_orphaned) { } bool match(int64_t local_pool_id, const std::string &global_image_id) { diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 9783a842d635a..f68d51d40d8d1 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -1639,7 +1639,8 @@ void ImageReplayer::handle_shut_down(int r) { if (m_stopping_for_resync) { m_image_deleter->schedule_image_delete(m_local, m_local_pool_id, - m_global_image_id); + m_global_image_id, + true); m_stopping_for_resync = false; } } diff --git a/src/tools/rbd_mirror/InstanceReplayer.cc b/src/tools/rbd_mirror/InstanceReplayer.cc index fe874875442a2..40a8c1b43205a 100644 --- a/src/tools/rbd_mirror/InstanceReplayer.cc +++ b/src/tools/rbd_mirror/InstanceReplayer.cc @@ -224,7 +224,7 @@ void InstanceReplayer::release_image(const std::string &global_image_id, [this, image_replayer, on_finish] (int r) { auto global_image_id = image_replayer->get_global_image_id(); m_image_deleter->schedule_image_delete( - m_local_rados, m_local_pool_id, global_image_id); + m_local_rados, m_local_pool_id, global_image_id, false); on_finish->complete(0); }); } -- 2.39.5