From d80aa6057c3f47dbafd7ad5eb1006eaf13674d71 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 21 Nov 2017 10:08:13 -0500 Subject: [PATCH] rbd-mirror: fix possible race condition in error path tests Signed-off-by: Jason Dillaman --- src/test/rbd_mirror/test_ImageDeleter.cc | 69 +++++++++---------- .../rbd_mirror/test_mock_ImageReplayer.cc | 5 +- src/tools/rbd_mirror/ImageDeleter.cc | 17 ++++- src/tools/rbd_mirror/ImageDeleter.h | 11 +-- src/tools/rbd_mirror/ImageReplayer.cc | 5 +- 5 files changed, 58 insertions(+), 49 deletions(-) diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc index 281237f607d9d..6ee55d7c08369 100644 --- a/src/test/rbd_mirror/test_ImageDeleter.cc +++ b/src/test/rbd_mirror/test_ImageDeleter.cc @@ -229,7 +229,8 @@ int64_t TestImageDeleter::m_local_pool_id; TEST_F(TestImageDeleter, Delete_NonPrimary_Image) { - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + nullptr); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -246,7 +247,8 @@ TEST_F(TestImageDeleter, Delete_Split_Brain_Image) { promote_image(); demote_image(); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, true); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, true, + nullptr); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -262,11 +264,9 @@ TEST_F(TestImageDeleter, Delete_Split_Brain_Image) { TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) { promote_image(); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, - &ctx); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + &ctx); EXPECT_EQ(-EPERM, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -277,11 +277,9 @@ TEST_F(TestImageDeleter, Fail_Delete_Orphan_Image) { promote_image(); demote_image(); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, - &ctx); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + &ctx); EXPECT_EQ(-EPERM, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -291,7 +289,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Orphan_Image) { TEST_F(TestImageDeleter, Delete_Image_With_Child) { create_snapshot(); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + nullptr); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -306,7 +305,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) { create_snapshot("snap1"); create_snapshot("snap2"); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + nullptr); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -320,7 +320,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) { TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChild) { create_snapshot("snap1", true); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + nullptr); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -335,7 +336,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) { create_snapshot("snap1", true); create_snapshot("snap2", true); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + nullptr); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -349,20 +351,15 @@ 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(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); - m_deleter->set_busy_timer_interval(0.1); - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, - &ctx); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + &ctx); + m_deleter->set_busy_timer_interval(0.1); EXPECT_EQ(-EBUSY, ctx.wait()); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_CLONE_IMAGE_ID, - false); - C_SaferCond ctx2; - m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_CLONE_IMAGE_ID, - &ctx2); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_CLONE_IMAGE_ID, + false, &ctx2); EXPECT_EQ(0, ctx2.wait()); C_SaferCond ctx3; @@ -382,7 +379,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(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + nullptr); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -406,7 +404,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(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + nullptr); C_SaferCond ctx; m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, @@ -422,11 +421,9 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) { TEST_F(TestImageDeleter, Delete_NonExistent_Image_Without_MirroringState) { remove_image(); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, - &ctx); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + &ctx); EXPECT_EQ(-ENOENT, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -440,11 +437,9 @@ TEST_F(TestImageDeleter, Fail_Delete_NonPrimary_Image) { false); EXPECT_EQ(0, ictx->state->open(false)); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, - &ctx); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + &ctx); EXPECT_EQ(-EBUSY, ctx.wait()); EXPECT_EQ(0, ictx->state->close()); @@ -456,11 +451,9 @@ TEST_F(TestImageDeleter, Retry_Failed_Deletes) { false); EXPECT_EQ(0, ictx->state->open(false)); - m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false); - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID, - &ctx); + m_deleter->schedule_image_delete(m_local_io_ctx_ref, GLOBAL_IMAGE_ID, false, + &ctx); EXPECT_EQ(-EBUSY, ctx.wait()); EXPECT_EQ(0, ictx->state->close()); diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc index 0821da96a120f..3df247be94a46 100644 --- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc @@ -85,7 +85,8 @@ struct Threads { template <> struct ImageDeleter { - MOCK_METHOD3(schedule_image_delete, void(IoCtxRef, const std::string&, bool)); + MOCK_METHOD4(schedule_image_delete, void(IoCtxRef, const std::string&, bool, + Context*)); MOCK_METHOD4(wait_for_scheduled_deletion, void(int64_t, const std::string&, Context*, bool)); MOCK_METHOD2(cancel_waiter, void(int64_t, const std::string&)); @@ -399,7 +400,7 @@ public: const std::string& global_image_id, bool ignore_orphan) { EXPECT_CALL(mock_image_deleter, - schedule_image_delete(_, global_image_id, ignore_orphan)); + schedule_image_delete(_, global_image_id, ignore_orphan, nullptr)); } bufferlist encode_tag_data(const librbd::journal::TagData &tag_data) { diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index 23a53793bdd4c..19a3c79aa8482 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -165,11 +165,18 @@ ImageDeleter::~ImageDeleter() { template void ImageDeleter::schedule_image_delete(IoCtxRef local_io_ctx, const std::string& global_image_id, - bool ignore_orphaned) { + bool ignore_orphaned, + Context *on_delete) { int64_t local_pool_id = local_io_ctx->get_id(); dout(5) << "local_pool_id=" << local_pool_id << ", " << "global_image_id=" << global_image_id << dendl; + if (on_delete != nullptr) { + on_delete = new FunctionContext([this, on_delete](int r) { + m_work_queue->queue(on_delete, r); + }); + } + { Mutex::Locker locker(m_lock); auto del_info = find_delete_info(local_pool_id, global_image_id); @@ -179,11 +186,17 @@ void ImageDeleter::schedule_image_delete(IoCtxRef local_io_ctx, if (ignore_orphaned) { del_info->ignore_orphaned = true; } + + if (del_info->on_delete != nullptr) { + del_info->on_delete->complete(-ESTALE); + } + del_info->on_delete = on_delete; return; } m_delete_queue.emplace_back(new DeleteInfo(local_pool_id, global_image_id, - local_io_ctx, ignore_orphaned)); + local_io_ctx, ignore_orphaned, + on_delete)); } remove_images(); } diff --git a/src/tools/rbd_mirror/ImageDeleter.h b/src/tools/rbd_mirror/ImageDeleter.h index b2c87dfe906f2..83a6aba21796f 100644 --- a/src/tools/rbd_mirror/ImageDeleter.h +++ b/src/tools/rbd_mirror/ImageDeleter.h @@ -51,7 +51,8 @@ public: void schedule_image_delete(IoCtxRef local_io_ctx, const std::string& global_image_id, - bool ignore_orphaned); + bool ignore_orphaned, + Context *on_finish); void wait_for_scheduled_deletion(int64_t local_pool_id, const std::string &global_image_id, Context *ctx, @@ -76,22 +77,24 @@ private: std::string global_image_id; IoCtxRef local_io_ctx; bool ignore_orphaned = false; + Context *on_delete = nullptr; image_deleter::ErrorResult error_result = {}; int error_code = 0; utime_t retry_time = {}; int retries = 0; bool notify_on_failed_retry = true; - Context *on_delete = nullptr; DeleteInfo(int64_t local_pool_id, const std::string& global_image_id) : local_pool_id(local_pool_id), global_image_id(global_image_id) { } DeleteInfo(int64_t local_pool_id, const std::string& global_image_id, - IoCtxRef local_io_ctx, bool ignore_orphaned) + IoCtxRef local_io_ctx, bool ignore_orphaned, + Context *on_delete) : local_pool_id(local_pool_id), global_image_id(global_image_id), - local_io_ctx(local_io_ctx), ignore_orphaned(ignore_orphaned) { + local_io_ctx(local_io_ctx), ignore_orphaned(ignore_orphaned), + on_delete(on_delete) { } inline bool operator==(const DeleteInfo& delete_info) const { diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 98094091d4242..64c4ee3f047f1 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -1657,9 +1657,8 @@ void ImageReplayer::handle_shut_down(int r) { delete_requested = true; } if (delete_requested || m_resync_requested) { - m_image_deleter->schedule_image_delete(m_local_ioctx, - m_global_image_id, - m_resync_requested); + m_image_deleter->schedule_image_delete(m_local_ioctx, m_global_image_id, + m_resync_requested, nullptr); m_local_image_id = ""; m_resync_requested = false; -- 2.39.5