From 54d6df4449ca9672df06e8c943be210adb870647 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 15 Feb 2019 08:29:36 -0500 Subject: [PATCH] librbd: fixed some edge cases when moving images to trash upon remove Signed-off-by: Jason Dillaman --- src/librbd/api/Image.cc | 114 +++++++++--------- src/librbd/api/Image.h | 3 +- src/librbd/api/Migration.cc | 2 +- src/librbd/api/Trash.cc | 81 +++++++------ src/librbd/api/Trash.h | 8 +- src/librbd/librbd.cc | 12 +- .../librbd/image/test_mock_RefreshRequest.cc | 8 +- src/test/librbd/test_internal.cc | 20 +-- src/test/librbd/test_mirroring.cc | 7 +- src/test/rbd_mirror/test_ImageDeleter.cc | 28 ++--- 10 files changed, 137 insertions(+), 146 deletions(-) diff --git a/src/librbd/api/Image.cc b/src/librbd/api/Image.cc index 1d36070eb0a..1e586cc775d 100644 --- a/src/librbd/api/Image.cc +++ b/src/librbd/api/Image.cc @@ -11,13 +11,12 @@ #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageState.h" -#include "librbd/Utils.h" -#include "librbd/image/CloneRequest.h" -#include "librbd/image/RemoveRequest.h" #include "librbd/internal.h" #include "librbd/Utils.h" #include "librbd/api/Config.h" #include "librbd/api/Trash.h" +#include "librbd/image/RemoveRequest.h" +#include "librbd/image/CloneRequest.h" #include #define dout_subsys ceph_subsys_rbd @@ -563,8 +562,7 @@ int Image::deep_copy(I *src, librados::IoCtx& dest_md_ctx, } opts.set(RBD_IMAGE_OPTION_ORDER, static_cast(order)); - ImageCtx *dest = new librbd::ImageCtx(destname, "", nullptr, - dest_md_ctx, false); + auto dest = new I(destname, "", nullptr, dest_md_ctx, false); r = dest->state->open(0); if (r < 0) { lderr(cct) << "failed to read newly created header" << dendl; @@ -617,9 +615,9 @@ int Image::deep_copy(I *src, I *dest, bool flatten, C_SaferCond cond; SnapSeqs snap_seqs; - auto req = DeepCopyRequest<>::create(src, dest, snap_id_start, snap_id_end, - flatten, boost::none, op_work_queue, - &snap_seqs, &prog_ctx, &cond); + auto req = DeepCopyRequest::create(src, dest, snap_id_start, snap_id_end, + flatten, boost::none, op_work_queue, + &snap_seqs, &prog_ctx, &cond); req->send(); int r = cond.wait(); if (r < 0) { @@ -679,60 +677,62 @@ int Image::snap_set(I *ictx, uint64_t snap_id) { template int Image::remove(IoCtx& io_ctx, const std::string &image_name, - const std::string &image_id, ProgressContext& prog_ctx, - bool force, bool from_trash_remove) + ProgressContext& prog_ctx) { CephContext *cct((CephContext *)io_ctx.cct()); - ldout(cct, 20) << (image_id.empty() ? image_name : image_id) << dendl; + ldout(cct, 20) << "name=" << image_name << dendl; + + // look up the V2 image id based on the image name + std::string image_id; + int r = cls_client::dir_get_id(&io_ctx, RBD_DIRECTORY, image_name, + &image_id); + if (r == -ENOENT) { + // check if it already exists in trash from an aborted trash remove attempt + std::vector trash_entries; + r = Trash::list(io_ctx, trash_entries); + if (r < 0) { + return r; + } else if (r >= 0) { + for (auto& entry : trash_entries) { + if (entry.name == image_name && + entry.source == RBD_TRASH_IMAGE_SOURCE_REMOVE) { + return Trash::remove(io_ctx, entry.id, true, prog_ctx); + } + } + } - if (image_id.empty()) { - // id will only be supplied when used internally + // fall-through if we failed to locate the image in the V2 directory and + // trash + } else if (r < 0) { + lderr(cct) << "failed to retrieve image id: " << cpp_strerror(r) << dendl; + return r; + } else { + // attempt to move the image to the trash (and optionally immediately + // delete the image) ConfigProxy config(cct->_conf); Config::apply_pool_overrides(io_ctx, &config); - std::string image_id; - int r = cls_client::dir_get_id(&io_ctx, RBD_DIRECTORY, image_name, - &image_id); - if (r < 0 && r != -ENOENT) { - lderr(cct) << "failed to retrieve image id: " << cpp_strerror(r) << dendl; - return r; - } - - if(r == 0) { - auto expire_seconds = config.get_val( + rbd_trash_image_source_t trash_image_source = RBD_TRASH_IMAGE_SOURCE_REMOVE; + uint64_t expire_seconds = 0; + bool check_watchers = true; + if (config.get_val("rbd_move_to_trash_on_remove")) { + // keep the image in the trash upon remove requests + trash_image_source = RBD_TRASH_IMAGE_SOURCE_USER; + expire_seconds = config.get_val( "rbd_move_to_trash_on_remove_expire_seconds"); + check_watchers = false; + } - if (config.get_val("rbd_move_to_trash_on_remove")) { - return Trash::move_by_id( - io_ctx, RBD_TRASH_IMAGE_SOURCE_USER, image_id, image_name, - expire_seconds, true); - } else { - r = Trash::move_by_id( - io_ctx, RBD_TRASH_IMAGE_SOURCE_REMOVE, image_id, image_name, - expire_seconds, true); - - if (r == -ENOENT) { - // check if it already exists in trash from an aborted - // remove attempt - std::vector trash_entries; - r = Trash::list(io_ctx, trash_entries); - if (r < 0) { - return r; - } - - for (auto& entry : trash_entries) { - if (entry.name == image_name && - entry.source == RBD_TRASH_IMAGE_SOURCE_REMOVE) { - return Trash::remove(io_ctx, image_id, force, prog_ctx); - } - } - return -ENOENT; - } else if (r < 0) { - return r; - } - - return Trash::remove(io_ctx, image_id, force, prog_ctx); + r = Trash::move(io_ctx, trash_image_source, image_name, image_id, + expire_seconds, check_watchers); + if (r >= 0) { + if (trash_image_source == RBD_TRASH_IMAGE_SOURCE_REMOVE) { + // proceed with attempting to immediately remove the image + r = Trash::remove(io_ctx, image_id, true, prog_ctx); } + return r; + } else if (r < 0 && r != -EOPNOTSUPP) { + return r; } } @@ -740,10 +740,12 @@ int Image::remove(IoCtx& io_ctx, const std::string &image_name, ContextWQ *op_work_queue; ImageCtx::get_thread_pool_instance(cct, &thread_pool, &op_work_queue); + // might be a V1 image format that cannot be moved to the trash + // and would not have been listed in the V2 directory -- or the OSDs + // are too old and don't support the trash feature C_SaferCond cond; - auto req = librbd::image::RemoveRequest<>::create( - io_ctx, image_name, image_id, force, from_trash_remove, prog_ctx, - op_work_queue, &cond); + auto req = librbd::image::RemoveRequest::create( + io_ctx, image_name, "", false, false, prog_ctx, op_work_queue, &cond); req->send(); return cond.wait(); diff --git a/src/librbd/api/Image.h b/src/librbd/api/Image.h index 73687da37c0..59fa9ffcebf 100644 --- a/src/librbd/api/Image.h +++ b/src/librbd/api/Image.h @@ -65,8 +65,7 @@ struct Image { static int snap_set(ImageCtxT *ictx, uint64_t snap_id); static int remove(librados::IoCtx& io_ctx, const std::string &image_name, - const std::string &image_id, ProgressContext& prog_ctx, - bool force=false, bool from_trash_remove=false); + ProgressContext& prog_ctx); }; diff --git a/src/librbd/api/Migration.cc b/src/librbd/api/Migration.cc index 21a71c50f50..3c7df40bee8 100644 --- a/src/librbd/api/Migration.cc +++ b/src/librbd/api/Migration.cc @@ -1138,7 +1138,7 @@ int Migration::v2_unlink_src_image() { } int r = Trash::move(m_src_io_ctx, RBD_TRASH_IMAGE_SOURCE_MIGRATION, - m_src_image_ctx->name, 0); + m_src_image_ctx->name, 0, false); if (r < 0) { lderr(m_cct) << "failed moving image to trash: " << cpp_strerror(r) << dendl; diff --git a/src/librbd/api/Trash.cc b/src/librbd/api/Trash.cc index effc7a583dc..6f81b8c83e0 100644 --- a/src/librbd/api/Trash.cc +++ b/src/librbd/api/Trash.cc @@ -15,7 +15,7 @@ #include "librbd/TrashWatcher.h" #include "librbd/Utils.h" #include "librbd/api/DiffIterate.h" -#include "librbd/api/Image.h" +#include "librbd/image/RemoveRequest.h" #include "librbd/mirror/DisableRequest.h" #include "librbd/mirror/EnableRequest.h" #include "librbd/trash/MoveRequest.h" @@ -118,30 +118,20 @@ int enable_mirroring(IoCtx &io_ctx, const std::string &image_id) { } // anonymous namespace template -int Trash::move_by_id(librados::IoCtx &io_ctx, rbd_trash_image_source_t source, - const std::string &image_id, const std::string &image_name, +int Trash::move(librados::IoCtx &io_ctx, rbd_trash_image_source_t source, + const std::string &image_name, const std::string &image_id, uint64_t delay, bool check_for_watchers) { + ceph_assert(!image_name.empty() && !image_id.empty()); CephContext *cct((CephContext *)io_ctx.cct()); - ldout(cct, 20) << &io_ctx << " " << (image_id.empty() ? image_name : image_id) + ldout(cct, 20) << &io_ctx << " name=" << image_name << ", id=" << image_id << dendl; - ImageCtx *ictx = new ImageCtx((image_id.empty() ? image_name : ""), - image_id, nullptr, io_ctx, false); + auto ictx = new I("", image_id, nullptr, io_ctx, false); int r = ictx->state->open(OPEN_FLAG_SKIP_OPEN_PARENT); if (r < 0 && r != -ENOENT) { lderr(cct) << "failed to open image: " << cpp_strerror(r) << dendl; return r; - } else if (ictx->old_format) { - ldout(cct, 10) << "cannot move v1 image to trash" << dendl; - ictx->state->close(); - return -EOPNOTSUPP; - } - - std::string id = ictx->id; - - if (id.empty()) { - return r; } if (r == 0) { @@ -164,17 +154,14 @@ int Trash::move_by_id(librados::IoCtx &io_ctx, rbd_trash_image_source_t sourc } ictx->owner_lock.put_read(); + ictx->snap_lock.get_read(); if (!ictx->migration_info.empty()) { lderr(cct) << "cannot move migrating image to trash" << dendl; + ictx->snap_lock.put_read(); ictx->state->close(); return -EINVAL; } - - r = disable_mirroring(ictx); - if (r < 0) { - ictx->state->close(); - return r; - } + ictx->snap_lock.put_read(); if (check_for_watchers) { std::list t_watchers; @@ -197,6 +184,12 @@ int Trash::move_by_id(librados::IoCtx &io_ctx, rbd_trash_image_source_t sourc } } + r = disable_mirroring(ictx); + if (r < 0) { + ictx->state->close(); + return r; + } + ictx->state->close(); } @@ -209,13 +202,13 @@ int Trash::move_by_id(librados::IoCtx &io_ctx, rbd_trash_image_source_t sourc trash_image_spec.state = cls::rbd::TRASH_IMAGE_STATE_MOVING; C_SaferCond ctx; - auto req = trash::MoveRequest::create(io_ctx, id, trash_image_spec, + auto req = trash::MoveRequest::create(io_ctx, image_id, trash_image_spec, &ctx); req->send(); r = ctx.wait(); trash_image_spec.state = cls::rbd::TRASH_IMAGE_STATE_NORMAL; - int ret = cls_client::trash_state_set(&io_ctx, id, + int ret = cls_client::trash_state_set(&io_ctx, image_id, trash_image_spec.state, cls::rbd::TRASH_IMAGE_STATE_MOVING); if (ret < 0 && ret != -EOPNOTSUPP) { @@ -228,7 +221,7 @@ int Trash::move_by_id(librados::IoCtx &io_ctx, rbd_trash_image_source_t sourc } C_SaferCond notify_ctx; - TrashWatcher::notify_image_added(io_ctx, id, trash_image_spec, + TrashWatcher::notify_image_added(io_ctx, image_id, trash_image_spec, ¬ify_ctx); r = notify_ctx.wait(); if (r < 0) { @@ -244,20 +237,31 @@ int Trash::move(librados::IoCtx &io_ctx, rbd_trash_image_source_t source, const std::string &image_name, uint64_t delay, bool check_for_watchers) { CephContext *cct((CephContext *)io_ctx.cct()); - ldout(cct, 20) << &io_ctx << " " << image_name - << dendl; + ldout(cct, 20) << &io_ctx << " name=" << image_name << dendl; // try to get image id from the directory std::string image_id; int r = cls_client::dir_get_id(&io_ctx, RBD_DIRECTORY, image_name, &image_id); - if (r < 0 && r != -ENOENT) { + if (r == -ENOENT) { + r = io_ctx.stat(util::old_header_name(image_name), nullptr, nullptr); + if (r == 0) { + // cannot move V1 image to trash + ldout(cct, 10) << "cannot move v1 image to trash" << dendl; + return -EOPNOTSUPP; + } + + // image doesn't exist -- perhaps already in the trash since removing + // from the directory is the last step + return -ENOENT; + } else if (r < 0) { lderr(cct) << "failed to retrieve image id: " << cpp_strerror(r) << dendl; return r; } - return Trash::move_by_id(io_ctx, source, image_id, image_name, delay, - check_for_watchers); + ceph_assert(!image_name.empty() && !image_id.empty()); + return Trash::move(io_ctx, source, image_name, image_id, delay, + check_for_watchers); } template @@ -328,7 +332,7 @@ int Trash::purge(IoCtx& io_ctx, time_t expire_ts, ldout(cct, 20) << &io_ctx << dendl; std::vector trash_entries; - int r = librbd::api::Trash<>::list(io_ctx, trash_entries); + int r = librbd::api::Trash::list(io_ctx, trash_entries); if (r < 0) { return r; } @@ -422,7 +426,7 @@ int Trash::purge(IoCtx& io_ctx, time_t expire_ts, (pool_percent_used - threshold)); for (const auto &it : img->second) { - auto ictx = new ImageCtx("", it, nullptr, io_ctx, false); + auto ictx = new I("", it, nullptr, io_ctx, false); r = ictx->state->open(OPEN_FLAG_SKIP_OPEN_PARENT); if (r == -ENOENT) { continue; @@ -479,7 +483,7 @@ int Trash::purge(IoCtx& io_ctx, time_t expire_ts, NoOpProgressContext remove_pctx; uint64_t list_size = to_be_removed.size(), i = 0; for (const auto &entry_id : to_be_removed) { - r = librbd::api::Trash<>::remove(io_ctx, entry_id, true, remove_pctx); + r = librbd::api::Trash::remove(io_ctx, entry_id, true, remove_pctx); if (r < 0) { if (r == -ENOTEMPTY) { ldout(cct, 5) << "image has snapshots - these must be deleted " @@ -540,7 +544,16 @@ int Trash::remove(IoCtx &io_ctx, const std::string &image_id, bool force, return r; } - r = Image::remove(io_ctx, "", image_id, prog_ctx, false, true); + ThreadPool *thread_pool; + ContextWQ *op_work_queue; + ImageCtx::get_thread_pool_instance(cct, &thread_pool, &op_work_queue); + + C_SaferCond cond; + auto req = librbd::image::RemoveRequest::create( + io_ctx, "", image_id, force, true, prog_ctx, op_work_queue, &cond); + req->send(); + + r = cond.wait(); if (r < 0) { lderr(cct) << "error removing image " << image_id << ", which is pending deletion" << dendl; diff --git a/src/librbd/api/Trash.h b/src/librbd/api/Trash.h index 2c128b03209..65f1ece4822 100644 --- a/src/librbd/api/Trash.h +++ b/src/librbd/api/Trash.h @@ -21,12 +21,12 @@ namespace api { template struct Trash { - static int move_by_id(librados::IoCtx &io_ctx, rbd_trash_image_source_t source, - const std::string &image_id, const std::string &image_name, - uint64_t delay, bool check_for_watchers=false); static int move(librados::IoCtx &io_ctx, rbd_trash_image_source_t source, const std::string &image_name, uint64_t delay, - bool check_for_watchers=false); + bool check_for_watchers); + static int move(librados::IoCtx &io_ctx, rbd_trash_image_source_t source, + const std::string &image_name, const std::string &image_id, + uint64_t delay, bool check_for_watchers); static int get(librados::IoCtx &io_ctx, const std::string &id, trash_image_info_t *info); static int list(librados::IoCtx &io_ctx, diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc index 46c10381f81..ef962bb1ddb 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -574,7 +574,7 @@ namespace librbd { TracepointProvider::initialize(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::api::Image<>::remove(io_ctx, name, "", prog_ctx); + int r = librbd::api::Image<>::remove(io_ctx, name, prog_ctx); tracepoint(librbd, remove_exit, r); return r; } @@ -584,7 +584,7 @@ namespace librbd { { TracepointProvider::initialize(get_cct(io_ctx)); tracepoint(librbd, remove_enter, io_ctx.get_pool_name().c_str(), io_ctx.get_id(), name); - int r = librbd::api::Image<>::remove(io_ctx, name, "", pctx); + int r = librbd::api::Image<>::remove(io_ctx, name, pctx); tracepoint(librbd, remove_exit, r); return r; } @@ -594,7 +594,7 @@ namespace librbd { tracepoint(librbd, trash_move_enter, io_ctx.get_pool_name().c_str(), io_ctx.get_id(), name); int r = librbd::api::Trash<>::move(io_ctx, RBD_TRASH_IMAGE_SOURCE_USER, - name, delay); + name, delay, false); tracepoint(librbd, trash_move_exit, r); return r; } @@ -3145,7 +3145,7 @@ extern "C" int rbd_remove(rados_ioctx_t p, const char *name) TracepointProvider::initialize(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::api::Image<>::remove(io_ctx, name, "", prog_ctx); + int r = librbd::api::Image<>::remove(io_ctx, name, prog_ctx); tracepoint(librbd, remove_exit, r); return r; } @@ -3158,7 +3158,7 @@ extern "C" int rbd_remove_with_progress(rados_ioctx_t p, const char *name, TracepointProvider::initialize(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::api::Image<>::remove(io_ctx, name, "", prog_ctx); + int r = librbd::api::Image<>::remove(io_ctx, name, prog_ctx); tracepoint(librbd, remove_exit, r); return r; } @@ -3171,7 +3171,7 @@ extern "C" int rbd_trash_move(rados_ioctx_t p, const char *name, tracepoint(librbd, trash_move_enter, io_ctx.get_pool_name().c_str(), io_ctx.get_id(), name); int r = librbd::api::Trash<>::move(io_ctx, RBD_TRASH_IMAGE_SOURCE_USER, name, - delay); + delay, false); tracepoint(librbd, trash_move_exit, r); return r; } diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index c6404f931cd..414303b036b 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -672,7 +672,6 @@ TEST_F(TestMockImageRefreshRequest, SuccessChild) { librbd::ImageCtx *ictx; librbd::ImageCtx *ictx2 = nullptr; std::string clone_name = get_temp_image_name(); - std::string image_id; ASSERT_EQ(0, open_image(m_image_name, &ictx)); ASSERT_EQ(0, snap_create(*ictx, "snap")); @@ -683,7 +682,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessChild) { } librbd::NoOpProgressContext no_op; - ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, "", image_id, no_op)); + ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, clone_name, no_op)); ASSERT_EQ(0, ictx->operations->snap_unprotect(cls::rbd::UserSnapshotNamespace(), "snap")); }; @@ -692,7 +691,6 @@ TEST_F(TestMockImageRefreshRequest, SuccessChild) { clone_name.c_str(), ictx->features, &order, 0, 0)); ASSERT_EQ(0, open_image(clone_name, &ictx2)); - image_id = ictx2->id; MockRefreshImageCtx mock_image_ctx(*ictx2); MockRefreshParentRequest *mock_refresh_parent_request = new MockRefreshParentRequest(); @@ -727,7 +725,6 @@ TEST_F(TestMockImageRefreshRequest, SuccessChildDontOpenParent) { librbd::ImageCtx *ictx; librbd::ImageCtx *ictx2 = nullptr; - std::string image_id; std::string clone_name = get_temp_image_name(); ASSERT_EQ(0, open_image(m_image_name, &ictx)); @@ -739,7 +736,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessChildDontOpenParent) { } librbd::NoOpProgressContext no_op; - ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, "", image_id, no_op)); + ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, clone_name, no_op)); ASSERT_EQ(0, ictx->operations->snap_unprotect(cls::rbd::UserSnapshotNamespace(), "snap")); }; @@ -748,7 +745,6 @@ TEST_F(TestMockImageRefreshRequest, SuccessChildDontOpenParent) { clone_name.c_str(), ictx->features, &order, 0, 0)); ASSERT_EQ(0, open_image(clone_name, &ictx2)); - image_id = ictx2->id; MockRefreshImageCtx mock_image_ctx(*ictx2); MockExclusiveLock mock_exclusive_lock; diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc index 6d5209306c3..8d24a608db6 100644 --- a/src/test/librbd/test_internal.cc +++ b/src/test/librbd/test_internal.cc @@ -359,7 +359,7 @@ TEST_F(TestInternal, FlattenFailsToLockImage) { parent->unlock_image(); } librbd::NoOpProgressContext no_op; - ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, clone_name, "", no_op)); + ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, clone_name, no_op)); } BOOST_SCOPE_EXIT_END; ASSERT_EQ(0, open_image(clone_name, &ictx2)); @@ -923,7 +923,7 @@ TEST_F(TestInternal, WriteFullCopyup) { } librbd::NoOpProgressContext remove_no_op; - ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, clone_name, "", + ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, clone_name, remove_no_op)); } BOOST_SCOPE_EXIT_END; @@ -958,20 +958,6 @@ TEST_F(TestInternal, WriteFullCopyup) { 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::api::Image<>::remove(m_ioctx, "", image_id, - remove_no_op)); -} - static int iterate_cb(uint64_t off, size_t len, int exists, void *arg) { interval_set *diff = static_cast *>(arg); @@ -1462,7 +1448,7 @@ TEST_F(TestInternal, SparsifyClone) { BOOST_SCOPE_EXIT_ALL(this, &ictx, clone_name) { close_image(ictx); librbd::NoOpProgressContext no_op; - EXPECT_EQ(0, librbd::api::Image<>::remove(m_ioctx, clone_name, "", no_op)); + EXPECT_EQ(0, librbd::api::Image<>::remove(m_ioctx, clone_name, no_op)); }; ASSERT_EQ(0, ictx->operations->resize((1 << ictx->order) * 20, true, no_op)); diff --git a/src/test/librbd/test_mirroring.cc b/src/test/librbd/test_mirroring.cc index 71306ba5157..1aa72da5177 100644 --- a/src/test/librbd/test_mirroring.cc +++ b/src/test/librbd/test_mirroring.cc @@ -718,17 +718,14 @@ TEST_F(TestMirroring, RemoveBootstrapped) ASSERT_EQ(0, m_rbd.mirror_mode_set(m_ioctx, RBD_MIRROR_MODE_POOL)); uint64_t features = RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING; - //uint64_t features = 0; int order = 20; ASSERT_EQ(0, m_rbd.create2(m_ioctx, image_name.c_str(), 4096, features, &order)); librbd::Image image; ASSERT_EQ(0, m_rbd.open(m_ioctx, image, image_name.c_str())); - std::string image_id; - ASSERT_EQ(0, image.get_id(&image_id)); librbd::NoOpProgressContext no_op; - ASSERT_EQ(-EBUSY, librbd::api::Image<>::remove(m_ioctx, "", image_id, no_op)); + ASSERT_EQ(-EBUSY, librbd::api::Image<>::remove(m_ioctx, image_name, no_op)); // simulate the image is open by rbd-mirror bootstrap uint64_t handle; @@ -749,7 +746,7 @@ TEST_F(TestMirroring, RemoveBootstrapped) ASSERT_EQ(0, m_ioctx.create(RBD_MIRRORING, false)); ASSERT_EQ(0, m_ioctx.watch2(RBD_MIRRORING, &handle, &watcher)); // now remove should succeed - ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, "", image_id, no_op)); + ASSERT_EQ(0, librbd::api::Image<>::remove(m_ioctx, image_name, no_op)); ASSERT_EQ(0, m_ioctx.unwatch2(handle)); ASSERT_TRUE(watcher.m_notified); ASSERT_EQ(0, image.close()); diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc index 5970440d041..ea64d038713 100644 --- a/src/test/rbd_mirror/test_ImageDeleter.cc +++ b/src/test/rbd_mirror/test_ImageDeleter.cc @@ -97,23 +97,21 @@ public: ASSERT_EQ(0, ctx.wait()); } - void remove_image(bool force=false) { - if (!force) { - cls::rbd::MirrorImage mirror_image; - int r = cls_client::mirror_image_get(&m_local_io_ctx, m_local_image_id, - &mirror_image); - EXPECT_EQ(1, r == 0 || r == -ENOENT); - if (r != -ENOENT) { - mirror_image.state = MirrorImageState::MIRROR_IMAGE_STATE_ENABLED; - EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, - m_local_image_id, - mirror_image)); - } - promote_image(); + void remove_image() { + cls::rbd::MirrorImage mirror_image; + int r = cls_client::mirror_image_get(&m_local_io_ctx, m_local_image_id, + &mirror_image); + EXPECT_EQ(1, r == 0 || r == -ENOENT); + if (r != -ENOENT) { + mirror_image.state = MirrorImageState::MIRROR_IMAGE_STATE_ENABLED; + EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, + m_local_image_id, + mirror_image)); } + promote_image(); + NoOpProgressContext ctx; - int r = librbd::api::Image<>::remove(m_local_io_ctx, m_image_name, "", ctx, - force); + r = librbd::api::Image<>::remove(m_local_io_ctx, m_image_name, ctx); EXPECT_EQ(1, r == 0 || r == -ENOENT); } -- 2.39.5