From: Jason Dillaman Date: Tue, 19 Feb 2019 14:20:51 +0000 (-0500) Subject: librbd: include in-progress image removals in image list X-Git-Tag: v14.1.0~1^2~5 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=4e18584bee6c95041588689129fa989dad0ed47e;p=ceph.git librbd: include in-progress image removals in image list Exclude them from the trash list to prevent double-counting them. This mimics the previous behavior for handling interrupted image removals. Signed-off-by: Jason Dillaman --- diff --git a/src/cls/rbd/cls_rbd_types.h b/src/cls/rbd/cls_rbd_types.h index 18a9eeff01d1..bbfc85870b21 100644 --- a/src/cls/rbd/cls_rbd_types.h +++ b/src/cls/rbd/cls_rbd_types.h @@ -559,6 +559,7 @@ enum TrashImageSource { TRASH_IMAGE_SOURCE_USER = 0, TRASH_IMAGE_SOURCE_MIRRORING = 1, TRASH_IMAGE_SOURCE_MIGRATION = 2, + TRASH_IMAGE_SOURCE_REMOVING = 3, }; inline std::ostream& operator<<(std::ostream& os, @@ -573,6 +574,9 @@ inline std::ostream& operator<<(std::ostream& os, case TRASH_IMAGE_SOURCE_MIGRATION: os << "migration"; break; + case TRASH_IMAGE_SOURCE_REMOVING: + os << "removing"; + break; default: os << "unknown (" << static_cast(source) << ")"; break; diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h index 149e59dcc9a0..1adffe781cf6 100644 --- a/src/include/rbd/librbd.h +++ b/src/include/rbd/librbd.h @@ -238,7 +238,7 @@ typedef enum { RBD_TRASH_IMAGE_SOURCE_USER = 0, RBD_TRASH_IMAGE_SOURCE_MIRRORING = 1, RBD_TRASH_IMAGE_SOURCE_MIGRATION = 2, - RBD_TRASH_IMAGE_SOURCE_REMOVE = 3 + RBD_TRASH_IMAGE_SOURCE_REMOVING = 3 } rbd_trash_image_source_t; typedef struct { diff --git a/src/librbd/api/Image.cc b/src/librbd/api/Image.cc index 1e586cc775de..c77b6005aee6 100644 --- a/src/librbd/api/Image.cc +++ b/src/librbd/api/Image.cc @@ -93,7 +93,7 @@ int Image::list_images(librados::IoCtx& io_ctx, return r; } - // old format images are in a tmap + // V1 format images are in a tmap if (bl.length()) { auto p = bl.cbegin(); bufferlist header; @@ -106,6 +106,7 @@ int Image::list_images(librados::IoCtx& io_ctx, } } + // V2 format images std::map image_names_to_ids; r = list_images_v2(io_ctx, &image_names_to_ids); if (r < 0) { @@ -118,6 +119,22 @@ int Image::list_images(librados::IoCtx& io_ctx, .name = img_pair.first}); } + // include V2 images in a partially removed state + std::vector trash_images; + r = Trash::list(io_ctx, trash_images, false); + if (r < 0 && r != -EOPNOTSUPP) { + lderr(cct) << "error listing trash images: " << cpp_strerror(r) << dendl; + return r; + } + + for (const auto& trash_image : trash_images) { + if (trash_image.source == RBD_TRASH_IMAGE_SOURCE_REMOVING) { + images->push_back({.id = trash_image.id, + .name = trash_image.name}); + + } + } + return 0; } @@ -438,7 +455,7 @@ int Image::list_descendants( } std::vector trash_images; - r = Trash::list(child_io_ctx, trash_images); + r = Trash::list(child_io_ctx, trash_images, false); if (r < 0 && r != -EOPNOTSUPP) { lderr(cct) << "error listing trash images: " << cpp_strerror(r) << dendl; @@ -446,7 +463,10 @@ int Image::list_descendants( } for (auto& it : trash_images) { - child_image_id_to_info[it.id] = {it.name, true}; + child_image_id_to_info.insert({ + it.id, + {it.name, + it.source == RBD_TRASH_IMAGE_SOURCE_REMOVING ? false : true}}); } } @@ -689,13 +709,13 @@ int Image::remove(IoCtx& io_ctx, const std::string &image_name, 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); + r = Trash::list(io_ctx, trash_entries, false); 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) { + entry.source == RBD_TRASH_IMAGE_SOURCE_REMOVING) { return Trash::remove(io_ctx, entry.id, true, prog_ctx); } } @@ -712,7 +732,8 @@ int Image::remove(IoCtx& io_ctx, const std::string &image_name, ConfigProxy config(cct->_conf); Config::apply_pool_overrides(io_ctx, &config); - rbd_trash_image_source_t trash_image_source = RBD_TRASH_IMAGE_SOURCE_REMOVE; + rbd_trash_image_source_t trash_image_source = + RBD_TRASH_IMAGE_SOURCE_REMOVING; uint64_t expire_seconds = 0; bool check_watchers = true; if (config.get_val("rbd_move_to_trash_on_remove")) { @@ -726,7 +747,7 @@ int Image::remove(IoCtx& io_ctx, const std::string &image_name, 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) { + if (trash_image_source == RBD_TRASH_IMAGE_SOURCE_REMOVING) { // proceed with attempting to immediately remove the image r = Trash::remove(io_ctx, image_id, true, prog_ctx); } diff --git a/src/librbd/api/Migration.cc b/src/librbd/api/Migration.cc index 3c7df40bee81..b21b8d454b88 100644 --- a/src/librbd/api/Migration.cc +++ b/src/librbd/api/Migration.cc @@ -166,7 +166,7 @@ int trash_search(librados::IoCtx &io_ctx, rbd_trash_image_source_t source, const std::string &image_name, std::string *image_id) { std::vector entries; - int r = Trash<>::list(io_ctx, entries); + int r = Trash<>::list(io_ctx, entries, false); if (r < 0) { return r; } diff --git a/src/librbd/api/Pool.cc b/src/librbd/api/Pool.cc index 533106d0e611..f3dd357faa25 100644 --- a/src/librbd/api/Pool.cc +++ b/src/librbd/api/Pool.cc @@ -281,6 +281,12 @@ int Pool::get_stats(librados::IoCtx& io_ctx, StatOptions* stat_options) { uint64_t* max_provisioned_bytes; uint64_t* snapshot_count; + std::vector trash_entries; + int r = Trash::list(io_ctx, trash_entries, false); + if (r < 0 && r != -EOPNOTSUPP) { + return r; + } + get_pool_stat_option_value( stat_options, RBD_POOL_STAT_OPTION_IMAGES, &image_count); get_pool_stat_option_value( @@ -300,10 +306,15 @@ int Pool::get_stats(librados::IoCtx& io_ctx, StatOptions* stat_options) { } std::vector image_ids; - image_ids.reserve(images.size()); + image_ids.reserve(images.size() + trash_entries.size()); for (auto& it : images) { image_ids.push_back(std::move(it.second)); } + for (auto& it : trash_entries) { + if (it.source == RBD_TRASH_IMAGE_SOURCE_REMOVING) { + image_ids.push_back(std::move(it.id)); + } + } r = get_pool_stats(io_ctx, config, image_ids, image_count, provisioned_bytes, max_provisioned_bytes, @@ -325,15 +336,13 @@ int Pool::get_stats(librados::IoCtx& io_ctx, StatOptions* stat_options) { stat_options, RBD_POOL_STAT_OPTION_TRASH_SNAPSHOTS, &snapshot_count); if (image_count != nullptr || provisioned_bytes != nullptr || max_provisioned_bytes != nullptr || snapshot_count != nullptr) { - std::vector trash_entries; - int r = Trash::list(io_ctx, trash_entries); - if (r < 0 && r != -EOPNOTSUPP) { - return r; - } std::vector image_ids; image_ids.reserve(trash_entries.size()); for (auto& it : trash_entries) { + if (it.source == RBD_TRASH_IMAGE_SOURCE_REMOVING) { + continue; + } image_ids.push_back(std::move(it.id)); } diff --git a/src/librbd/api/Trash.cc b/src/librbd/api/Trash.cc index 6f81b8c83e03..8acbe34edd0d 100644 --- a/src/librbd/api/Trash.cc +++ b/src/librbd/api/Trash.cc @@ -288,7 +288,8 @@ int Trash::get(IoCtx &io_ctx, const std::string &id, } template -int Trash::list(IoCtx &io_ctx, vector &entries) { +int Trash::list(IoCtx &io_ctx, vector &entries, + bool exclude_user_remove_source) { CephContext *cct((CephContext *)io_ctx.cct()); ldout(cct, 20) << "trash_list " << &io_ctx << dendl; @@ -314,6 +315,10 @@ int Trash::list(IoCtx &io_ctx, vector &entries) { for (const auto &entry : trash_entries) { rbd_trash_image_source_t source = static_cast(entry.second.source); + if (exclude_user_remove_source && + source == RBD_TRASH_IMAGE_SOURCE_REMOVING) { + continue; + } entries.push_back({entry.first, entry.second.name, source, entry.second.deletion_time.sec(), entry.second.deferment_end_time.sec()}); @@ -332,7 +337,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, true); if (r < 0) { return r; } diff --git a/src/librbd/api/Trash.h b/src/librbd/api/Trash.h index 65f1ece48224..c671e2c0fcbc 100644 --- a/src/librbd/api/Trash.h +++ b/src/librbd/api/Trash.h @@ -30,7 +30,8 @@ struct Trash { static int get(librados::IoCtx &io_ctx, const std::string &id, trash_image_info_t *info); static int list(librados::IoCtx &io_ctx, - std::vector &entries); + std::vector &entries, + bool exclude_user_remove_source); static int purge(IoCtx& io_ctx, time_t expire_ts, float threshold, ProgressContext& pctx); static int remove(librados::IoCtx &io_ctx, const std::string &image_id, diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc index ef962bb1ddb6..039e4eb28f40 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -607,7 +607,7 @@ namespace librbd { TracepointProvider::initialize(get_cct(io_ctx)); tracepoint(librbd, trash_list_enter, io_ctx.get_pool_name().c_str(), io_ctx.get_id()); - int r = librbd::api::Trash<>::list(io_ctx, entries); + int r = librbd::api::Trash<>::list(io_ctx, entries, true); #ifdef WITH_LTTNG if (r >= 0) { for (const auto& entry : entries) { @@ -3206,7 +3206,7 @@ extern "C" int rbd_trash_list(rados_ioctx_t p, rbd_trash_image_info_t *entries, memset(entries, 0, sizeof(*entries) * *num_entries); vector cpp_entries; - int r = librbd::api::Trash<>::list(io_ctx, cpp_entries); + int r = librbd::api::Trash<>::list(io_ctx, cpp_entries, true); if (r < 0) { tracepoint(librbd, trash_list_exit, r, *num_entries); return r; diff --git a/src/pybind/rbd/rbd.pyx b/src/pybind/rbd/rbd.pyx index d0b79b64cafc..452ef3b7c340 100644 --- a/src/pybind/rbd/rbd.pyx +++ b/src/pybind/rbd/rbd.pyx @@ -188,7 +188,7 @@ cdef extern from "rbd/librbd.h" nogil: _RBD_TRASH_IMAGE_SOURCE_USER "RBD_TRASH_IMAGE_SOURCE_USER", _RBD_TRASH_IMAGE_SOURCE_MIRRORING "RBD_TRASH_IMAGE_SOURCE_MIRRORING", _RBD_TRASH_IMAGE_SOURCE_MIGRATION "RBD_TRASH_IMAGE_SOURCE_MIGRATION" - _RBD_TRASH_IMAGE_SOURCE_REMOVE "RBD_TRASH_IMAGE_SOURCE_REMOVE" + _RBD_TRASH_IMAGE_SOURCE_REMOVING "RBD_TRASH_IMAGE_SOURCE_REMOVING" ctypedef struct rbd_trash_image_info_t: char *id @@ -1341,7 +1341,7 @@ class RBD(object): if ret != 0: raise make_ex(ret, 'error retrieving image from trash') - __source_string = ['USER', 'MIRRORING', 'MIGRATION'] + __source_string = ['USER', 'MIRRORING', 'MIGRATION', 'REMOVING'] info = { 'id' : decode_cstr(c_info.id), 'name' : decode_cstr(c_info.name), diff --git a/src/test/librbd/CMakeLists.txt b/src/test/librbd/CMakeLists.txt index 53570db52634..3295afe8d7a1 100644 --- a/src/test/librbd/CMakeLists.txt +++ b/src/test/librbd/CMakeLists.txt @@ -18,6 +18,7 @@ set(librbd_test test_MirroringWatcher.cc test_ObjectMap.cc test_Operations.cc + test_Trash.cc journal/test_Entries.cc journal/test_Replay.cc) add_library(rbd_test STATIC ${librbd_test}) diff --git a/src/test/librbd/test_Trash.cc b/src/test/librbd/test_Trash.cc new file mode 100644 index 000000000000..e7a1e7f87334 --- /dev/null +++ b/src/test/librbd/test_Trash.cc @@ -0,0 +1,92 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "cls/rbd/cls_rbd_client.h" +#include "cls/rbd/cls_rbd_types.h" +#include "test/librbd/test_fixture.h" +#include "test/librbd/test_support.h" +#include "librbd/api/Trash.h" +#include +#include + +void register_test_trash() { +} + +namespace librbd { + +static bool operator==(const trash_image_info_t& lhs, + const trash_image_info_t& rhs) { + return (lhs.id == rhs.id && + lhs.name == rhs.name && + lhs.source == rhs.source); +} + +static bool operator==(const image_spec_t& lhs, + const image_spec_t& rhs) { + return (lhs.id == rhs.id && lhs.name == rhs.name); +} + +class TestTrash : public TestFixture { +public: + + TestTrash() {} +}; + +TEST_F(TestTrash, UserRemovingSource) { + REQUIRE_FORMAT_V2(); + + auto compare_lambda = [](const trash_image_info_t& lhs, + const trash_image_info_t& rhs) { + if (lhs.id != rhs.id) { + return lhs.id < rhs.id; + } else if (lhs.name != rhs.name) { + return lhs.name < rhs.name; + } + return lhs.source < rhs.source; + }; + typedef std::set TrashEntries; + + librbd::RBD rbd; + librbd::Image image; + auto image_name1 = m_image_name; + std::string image_id1; + ASSERT_EQ(0, rbd.open(m_ioctx, image, image_name1.c_str())); + ASSERT_EQ(0, image.get_id(&image_id1)); + ASSERT_EQ(0, image.close()); + + auto image_name2 = get_temp_image_name(); + ASSERT_EQ(0, create_image_pp(m_rbd, m_ioctx, image_name2, m_image_size)); + + std::string image_id2; + ASSERT_EQ(0, rbd.open(m_ioctx, image, image_name2.c_str())); + ASSERT_EQ(0, image.get_id(&image_id2)); + ASSERT_EQ(0, image.close()); + + ASSERT_EQ(0, api::Trash<>::move(m_ioctx, RBD_TRASH_IMAGE_SOURCE_USER, + image_name1, image_id1, 0, false)); + ASSERT_EQ(0, api::Trash<>::move(m_ioctx, RBD_TRASH_IMAGE_SOURCE_REMOVING, + image_name2, image_id2, 0, true)); + + TrashEntries trash_entries{compare_lambda}; + TrashEntries expected_trash_entries{compare_lambda}; + + std::vector entries; + ASSERT_EQ(0, api::Trash<>::list(m_ioctx, entries, true)); + trash_entries.insert(entries.begin(), entries.end()); + + expected_trash_entries = { + {.id = image_id1, + .name = image_name1, + .source = RBD_TRASH_IMAGE_SOURCE_USER}, + }; + ASSERT_EQ(expected_trash_entries, trash_entries); + + std::vector expected_images = { + {.id = image_id2, .name = image_name2} + }; + std::vector images; + ASSERT_EQ(0, rbd.list2(m_ioctx, &images)); + ASSERT_EQ(expected_images, images); +} + +} // namespace librbd diff --git a/src/test/librbd/test_main.cc b/src/test/librbd/test_main.cc index 17b974b8e661..4b66203bd302 100644 --- a/src/test/librbd/test_main.cc +++ b/src/test/librbd/test_main.cc @@ -22,6 +22,7 @@ extern void register_test_mirroring(); extern void register_test_mirroring_watcher(); extern void register_test_object_map(); extern void register_test_operations(); +extern void register_test_trash(); #endif // TEST_LIBRBD_INTERNALS int main(int argc, char **argv) @@ -41,6 +42,7 @@ int main(int argc, char **argv) register_test_mirroring_watcher(); register_test_object_map(); register_test_operations(); + register_test_trash(); #endif // TEST_LIBRBD_INTERNALS ::testing::InitGoogleTest(&argc, argv); diff --git a/src/tools/rbd/action/Trash.cc b/src/tools/rbd/action/Trash.cc index f932f73a2427..327b20ba703b 100644 --- a/src/tools/rbd/action/Trash.cc +++ b/src/tools/rbd/action/Trash.cc @@ -266,8 +266,8 @@ int do_list(librbd::RBD &rbd, librados::IoCtx& io_ctx, bool long_flag, case RBD_TRASH_IMAGE_SOURCE_MIGRATION: del_source = "MIGRATION"; break; - case RBD_TRASH_IMAGE_SOURCE_REMOVE: - del_source = "REMOVE"; + case RBD_TRASH_IMAGE_SOURCE_REMOVING: + del_source = "REMOVING"; break; }