Skip to content

Commit

Permalink
rbd-mirror: fix possible condition race condition in error path tests
Browse files Browse the repository at this point in the history 
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
  • Loading branch information
Jason Dillaman committed Nov 21, 2017
1 parent 273edbc commit cf6d5d5
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 49 deletions.
69 changes: 31 additions & 38 deletions src/test/rbd_mirror/test_ImageDeleter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand Down
5 changes: 3 additions & 2 deletions src/test/rbd_mirror/test_mock_ImageReplayer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ struct Threads<librbd::MockTestImageCtx> {

template <>
struct ImageDeleter<librbd::MockTestImageCtx> {
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&));
Expand Down Expand Up @@ -399,7 +400,7 @@ class TestMockImageReplayer : public TestMockFixture {
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) {
Expand Down
17 changes: 15 additions & 2 deletions src/tools/rbd_mirror/ImageDeleter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,18 @@ ImageDeleter<I>::~ImageDeleter() {
template <typename I>
void ImageDeleter<I>::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);
Expand All @@ -179,11 +186,17 @@ void ImageDeleter<I>::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();
}
Expand Down
11 changes: 7 additions & 4 deletions src/tools/rbd_mirror/ImageDeleter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ class ImageDeleter {

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,
Expand All @@ -76,22 +77,24 @@ class ImageDeleter {
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 {
Expand Down
5 changes: 2 additions & 3 deletions src/tools/rbd_mirror/ImageReplayer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1657,9 +1657,8 @@ void ImageReplayer<I>::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;
Expand Down

0 comments on commit cf6d5d5

Please sign in to comment.