Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rbd-mirror: image deleter should use pool id + global image uuid for key #10484

Merged
merged 2 commits into from
Aug 2, 2016
Merged

rbd-mirror: image deleter should use pool id + global image uuid for key #10484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25203a8
rbd-mirror: use pool id + global image id as deletion primary key
Jul 13, 2016
87b32d1
librbd: support deleting image by id instead of name
Jul 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 22 additions & 9 deletions src/librbd/internal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1687,7 +1687,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
err_close_child:
c_imctx->state->close();
err_remove:
partial_r = remove(c_ioctx, c_name, no_op);
partial_r = remove(c_ioctx, c_name, "", no_op);
if (partial_r < 0) {
lderr(cct) << "Error removing failed clone: "
<< cpp_strerror(partial_r) << dendl;
Expand Down Expand Up @@ -2145,16 +2145,20 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
return 0;
}

int remove(IoCtx& io_ctx, const char *imgname, ProgressContext& prog_ctx,
int remove(IoCtx& io_ctx, const std::string &image_name,
const std::string &image_id, ProgressContext& prog_ctx,
bool force)
{
CephContext *cct((CephContext *)io_ctx.cct());
ldout(cct, 20) << "remove " << &io_ctx << " " << imgname << dendl;
ldout(cct, 20) << "remove " << &io_ctx << " "
<< (image_id.empty() ? image_name : image_id) << dendl;

string id;
std::string name(image_name);
std::string id(image_id);
bool old_format = false;
bool unknown_format = true;
ImageCtx *ictx = new ImageCtx(imgname, "", NULL, io_ctx, false);
ImageCtx *ictx = new ImageCtx(
(id.empty() ? name : std::string()), id, nullptr, io_ctx, false);
int r = ictx->state->open();
if (r < 0) {
ldout(cct, 2) << "error opening image: " << cpp_strerror(-r) << dendl;
Expand All @@ -2163,6 +2167,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
string header_oid = ictx->header_oid;
old_format = ictx->old_format;
unknown_format = false;
name = ictx->name;
id = ictx->id;

ictx->owner_lock.get_read();
Expand Down Expand Up @@ -2257,7 +2262,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,

if (old_format || unknown_format) {
ldout(cct, 2) << "removing rbd image from v1 directory..." << dendl;
r = tmap_rm(io_ctx, imgname);
r = tmap_rm(io_ctx, name);
old_format = (r == 0);
if (r < 0 && !unknown_format) {
if (r != -ENOENT) {
Expand All @@ -2270,12 +2275,20 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
if (!old_format) {
if (id.empty()) {
ldout(cct, 5) << "attempting to determine image id" << dendl;
r = cls_client::dir_get_id(&io_ctx, RBD_DIRECTORY, imgname, &id);
r = cls_client::dir_get_id(&io_ctx, RBD_DIRECTORY, name, &id);
if (r < 0 && r != -ENOENT) {
lderr(cct) << "error getting id of image" << dendl;
return r;
}
} else if (name.empty()) {
ldout(cct, 5) << "attempting to determine image name" << dendl;
r = cls_client::dir_get_name(&io_ctx, RBD_DIRECTORY, id, &name);
if (r < 0 && r != -ENOENT) {
lderr(cct) << "error getting name of image" << dendl;
return r;
}
}

if (!id.empty()) {
ldout(cct, 10) << "removing journal..." << dendl;
r = Journal<>::remove(io_ctx, id);
Expand All @@ -2302,15 +2315,15 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
}

ldout(cct, 2) << "removing id object..." << dendl;
r = io_ctx.remove(util::id_obj_name(imgname));
r = io_ctx.remove(util::id_obj_name(name));
if (r < 0 && r != -ENOENT) {
lderr(cct) << "error removing id object: " << cpp_strerror(r)
<< dendl;
return r;
}

ldout(cct, 2) << "removing rbd image from v2 directory..." << dendl;
r = cls_client::dir_remove_image(&io_ctx, RBD_DIRECTORY, imgname, id);
r = cls_client::dir_remove_image(&io_ctx, RBD_DIRECTORY, name, id);
if (r < 0) {
if (r != -ENOENT) {
lderr(cct) << "error removing image from v2 directory: "
Expand Down
5 changes: 3 additions & 2 deletions src/librbd/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ namespace librbd {
int set_image_notification(ImageCtx *ictx, int fd, int type);
int is_exclusive_lock_owner(ImageCtx *ictx, bool *is_owner);

int remove(librados::IoCtx& io_ctx, const char *imgname,
ProgressContext& prog_ctx, bool force=false);
int remove(librados::IoCtx& io_ctx, const std::string &image_name,
const std::string &image_id, ProgressContext& prog_ctx,
bool force=false);
int snap_list(ImageCtx *ictx, std::vector<snap_info_t>& snaps);
int snap_exists(ImageCtx *ictx, const char *snap_name, bool *exists);
int snap_get_limit(ImageCtx *ictx, uint64_t *limit);
Expand Down
8 changes: 4 additions & 4 deletions src/librbd/librbd.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ namespace librbd {
TracepointProvider::initialize<tracepoint_traits>(get_cct(io_ctx));
tracepoint(librbd, remove_enter, io_ctx.get_pool_name().c_str(), io_ctx.get_id(), name);
librbd::NoOpProgressContext prog_ctx;
int r = librbd::remove(io_ctx, name, prog_ctx);
int r = librbd::remove(io_ctx, name, "", prog_ctx);
tracepoint(librbd, remove_exit, r);
return r;
}
Expand All @@ -377,7 +377,7 @@ namespace librbd {
{
TracepointProvider::initialize<tracepoint_traits>(get_cct(io_ctx));
tracepoint(librbd, remove_enter, io_ctx.get_pool_name().c_str(), io_ctx.get_id(), name);
int r = librbd::remove(io_ctx, name, pctx);
int r = librbd::remove(io_ctx, name, "", pctx);
tracepoint(librbd, remove_exit, r);
return r;
}
Expand Down Expand Up @@ -1786,7 +1786,7 @@ extern "C" int rbd_remove(rados_ioctx_t p, const char *name)
TracepointProvider::initialize<tracepoint_traits>(get_cct(io_ctx));
tracepoint(librbd, remove_enter, io_ctx.get_pool_name().c_str(), io_ctx.get_id(), name);
librbd::NoOpProgressContext prog_ctx;
int r = librbd::remove(io_ctx, name, prog_ctx);
int r = librbd::remove(io_ctx, name, "", prog_ctx);
tracepoint(librbd, remove_exit, r);
return r;
}
Expand All @@ -1799,7 +1799,7 @@ extern "C" int rbd_remove_with_progress(rados_ioctx_t p, const char *name,
TracepointProvider::initialize<tracepoint_traits>(get_cct(io_ctx));
tracepoint(librbd, remove_enter, io_ctx.get_pool_name().c_str(), io_ctx.get_id(), name);
librbd::CProgressContext prog_ctx(cb, cbdata);
int r = librbd::remove(io_ctx, name, prog_ctx);
int r = librbd::remove(io_ctx, name, "", prog_ctx);
tracepoint(librbd, remove_exit, r);
return r;
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/librbd/image/test_mock_RefreshRequest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessChild) {
}

librbd::NoOpProgressContext no_op;
ASSERT_EQ(0, librbd::remove(m_ioctx, clone_name.c_str(), no_op));
ASSERT_EQ(0, librbd::remove(m_ioctx, clone_name, "", no_op));
ASSERT_EQ(0, ictx->operations->snap_unprotect("snap"));
};

Expand Down
17 changes: 15 additions & 2 deletions src/test/librbd/test_internal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ TEST_F(TestInternal, FlattenFailsToLockImage) {
parent->unlock_image();
}
librbd::NoOpProgressContext no_op;
ASSERT_EQ(0, librbd::remove(m_ioctx, clone_name.c_str(), no_op));
ASSERT_EQ(0, librbd::remove(m_ioctx, clone_name, "", no_op));
} BOOST_SCOPE_EXIT_END;

ASSERT_EQ(0, open_image(clone_name, &ictx2));
Expand Down Expand Up @@ -808,7 +808,7 @@ TEST_F(TestInternal, WriteFullCopyup) {
}

librbd::NoOpProgressContext remove_no_op;
ASSERT_EQ(0, librbd::remove(m_ioctx, clone_name.c_str(), remove_no_op));
ASSERT_EQ(0, librbd::remove(m_ioctx, clone_name, "", remove_no_op));
} BOOST_SCOPE_EXIT_END;

ASSERT_EQ(0, open_image(clone_name, &ictx2));
Expand All @@ -835,3 +835,16 @@ TEST_F(TestInternal, WriteFullCopyup) {
read_bl.c_str(), 0));
ASSERT_TRUE(bl.contents_equal(read_bl));
}

TEST_F(TestInternal, RemoveById) {
REQUIRE_FEATURE(RBD_FEATURE_LAYERING);

librbd::ImageCtx *ictx;
ASSERT_EQ(0, open_image(m_image_name, &ictx));

std::string image_id = ictx->id;
close_image(ictx);

librbd::NoOpProgressContext remove_no_op;
ASSERT_EQ(0, librbd::remove(m_ioctx, "", image_id, remove_no_op));
}
53 changes: 35 additions & 18 deletions src/test/rbd_mirror/test_ImageDeleter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class TestImageDeleter : public TestFixture {
promote_image();
}
NoOpProgressContext ctx;
int r = remove(m_local_io_ctx, m_image_name.c_str(), ctx, force);
int r = remove(m_local_io_ctx, m_image_name, "", ctx, force);
EXPECT_EQ(1, r == 0 || r == -ENOENT);
}

Expand Down Expand Up @@ -216,7 +216,8 @@ TEST_F(TestImageDeleter, Delete_NonPrimary_Image) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &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());
Expand All @@ -232,7 +233,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &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());
Expand All @@ -247,7 +249,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Diff_GlobalId) {
m_image_name, "diff global id");

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, "diff global id",
&ctx);
EXPECT_EQ(-EINVAL, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -261,7 +264,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Child) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &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());
Expand All @@ -276,7 +280,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &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());
Expand All @@ -290,7 +295,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChild) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &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());
Expand All @@ -305,7 +311,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &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());
Expand All @@ -319,7 +326,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-EBUSY, ctx.wait());

ASSERT_EQ(1u, m_deleter->get_delete_queue_items().size());
Expand All @@ -329,11 +337,13 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) {
"clone1", GLOBAL_CLONE_IMAGE_ID);

C_SaferCond ctx2;
m_deleter->wait_for_scheduled_deletion("clone1", &ctx2);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_CLONE_IMAGE_ID,
&ctx2);
EXPECT_EQ(0, ctx2.wait());

C_SaferCond ctx3;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx3);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx3);
EXPECT_EQ(0, ctx3.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -352,7 +362,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &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());
Expand All @@ -376,7 +387,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &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());
Expand All @@ -392,7 +404,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_Without_MirroringState) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-ENOENT, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -410,7 +423,8 @@ TEST_F(TestImageDeleter, Fail_Delete_NonPrimary_Image) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-EBUSY, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -430,13 +444,15 @@ TEST_F(TestImageDeleter, Retry_Failed_Deletes) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-EBUSY, ctx.wait());

EXPECT_EQ(0, ictx->state->close());

C_SaferCond ctx2;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx2);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx2);
EXPECT_EQ(0, ctx2.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -454,7 +470,8 @@ TEST_F(TestImageDeleter, Delete_Is_Idempotent) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-EBUSY, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand Down
6 changes: 3 additions & 3 deletions src/test/rbd_mirror/test_ImageReplayer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ TEST_F(TestImageReplayer, Resync)

C_SaferCond delete_ctx;
m_image_deleter->wait_for_scheduled_deletion(
m_replayer->get_local_image_name(), &delete_ctx);
m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx);
EXPECT_EQ(0, delete_ctx.wait());

C_SaferCond cond;
Expand Down Expand Up @@ -631,7 +631,7 @@ TEST_F(TestImageReplayer, Resync_While_Stop)

C_SaferCond delete_ctx;
m_image_deleter->wait_for_scheduled_deletion(
m_replayer->get_local_image_name(), &delete_ctx);
m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx);
EXPECT_EQ(0, delete_ctx.wait());

C_SaferCond cond3;
Expand Down Expand Up @@ -672,7 +672,7 @@ TEST_F(TestImageReplayer, Resync_StartInterrupted)

C_SaferCond delete_ctx;
m_image_deleter->wait_for_scheduled_deletion(
m_replayer->get_local_image_name(), &delete_ctx);
m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx);
EXPECT_EQ(0, delete_ctx.wait());

C_SaferCond cond2;
Expand Down