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 <dillaman@redhat.com>

diff --git a/src/cls/rbd/cls_rbd_types.h b/src/cls/rbd/cls_rbd_types.h
index 18a9eeff01d169ed8ec4dc3e02ff45917869c162..bbfc85870b21b0519298fbe4f0f9d67d60774560 100644 (file)
--- 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<uint32_t>(source) << ")";
     break;

diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h
index 149e59dcc9a03fe262a48a9b9c25c3c07bb8e07e..1adffe781cf68e80e618be2e1c075007f1764881 100644 (file)
--- 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 1e586cc775def569b44cbf8adddcb60d0b219202..c77b6005aee615fb1a0fb1a0e78d510e0dbddad1 100644 (file)
--- a/src/librbd/api/Image.cc
+++ b/src/librbd/api/Image.cc
@@ -93,7 +93,7 @@ int Image<I>::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<I>::list_images(librados::IoCtx& io_ctx,
     }
   }
 
+  // V2 format images
   std::map<std::string, std::string> image_names_to_ids;
   r = list_images_v2(io_ctx, &image_names_to_ids);
   if (r < 0) {
@@ -118,6 +119,22 @@ int Image<I>::list_images(librados::IoCtx& io_ctx,
                        .name = img_pair.first});
   }
 
+  // include V2 images in a partially removed state
+  std::vector<librbd::trash_image_info_t> trash_images;
+  r = Trash<I>::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<I>::list_descendants(
       }
 
       std::vector<librbd::trash_image_info_t> trash_images;
-      r = Trash<I>::list(child_io_ctx, trash_images);
+      r = Trash<I>::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<I>::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<I>::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_image_info_t> trash_entries;
-    r = Trash<I>::list(io_ctx, trash_entries);
+    r = Trash<I>::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<I>::remove(io_ctx, entry.id, true, prog_ctx);
         }
       }
@@ -712,7 +732,8 @@ int Image<I>::remove(IoCtx& io_ctx, const std::string &image_name,
     ConfigProxy config(cct->_conf);
     Config<I>::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<bool>("rbd_move_to_trash_on_remove")) {
@@ -726,7 +747,7 @@ int Image<I>::remove(IoCtx& io_ctx, const std::string &image_name,
     r = Trash<I>::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<I>::remove(io_ctx, image_id, true, prog_ctx);
       }

diff --git a/src/librbd/api/Migration.cc b/src/librbd/api/Migration.cc
index 3c7df40bee814934496ca7ef0a46e05d89888291..b21b8d454b8812ccc0f75c97d3636ae4e396c90b 100644 (file)
--- 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<trash_image_info_t> 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 533106d0e611b0197214f8ca6c355c2ff5552f22..f3dd357faa255c8608d87a7c2abaf1a24e04d87e 100644 (file)
--- a/src/librbd/api/Pool.cc
+++ b/src/librbd/api/Pool.cc
@@ -281,6 +281,12 @@ int Pool<I>::get_stats(librados::IoCtx& io_ctx, StatOptions* stat_options) {
   uint64_t* max_provisioned_bytes;
   uint64_t* snapshot_count;
 
+  std::vector<trash_image_info_t> trash_entries;
+  int r = Trash<I>::list(io_ctx, trash_entries, false);
+  if (r < 0 && r != -EOPNOTSUPP) {
+    return r;
+  }
+
   get_pool_stat_option_value<I>(
     stat_options, RBD_POOL_STAT_OPTION_IMAGES, &image_count);
   get_pool_stat_option_value<I>(
@@ -300,10 +306,15 @@ int Pool<I>::get_stats(librados::IoCtx& io_ctx, StatOptions* stat_options) {
     }
 
     std::vector<std::string> 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<I>(io_ctx, config, image_ids, image_count,
                           provisioned_bytes, max_provisioned_bytes,
@@ -325,15 +336,13 @@ int Pool<I>::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_image_info_t> trash_entries;
-    int r = Trash<I>::list(io_ctx, trash_entries);
-    if (r < 0 && r != -EOPNOTSUPP) {
-      return r;
-    }
 
     std::vector<std::string> 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 6f81b8c83e03022c320b13cb8e62df71bfb613a3..8acbe34edd0db1152475668e61432657e8b35d13 100644 (file)
--- a/src/librbd/api/Trash.cc
+++ b/src/librbd/api/Trash.cc
@@ -288,7 +288,8 @@ int Trash<I>::get(IoCtx &io_ctx, const std::string &id,
 }
 
 template <typename I>
-int Trash<I>::list(IoCtx &io_ctx, vector<trash_image_info_t> &entries) {
+int Trash<I>::list(IoCtx &io_ctx, vector<trash_image_info_t> &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<I>::list(IoCtx &io_ctx, vector<trash_image_info_t> &entries) {
     for (const auto &entry : trash_entries) {
       rbd_trash_image_source_t source =
           static_cast<rbd_trash_image_source_t>(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<I>::purge(IoCtx& io_ctx, time_t expire_ts,
   ldout(cct, 20) << &io_ctx << dendl;
 
   std::vector<librbd::trash_image_info_t> trash_entries;
-  int r = librbd::api::Trash<I>::list(io_ctx, trash_entries);
+  int r = librbd::api::Trash<I>::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 65f1ece48224aa725f3e526d66e55fa1b721315c..c671e2c0fcbcf896e0cbbf09c701705dc8b6259a 100644 (file)
--- 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<trash_image_info_t> &entries);
+                  std::vector<trash_image_info_t> &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 ef962bb1ddb64c8d8188c2b687ad5779de69274a..039e4eb28f40a9c27244e11fc6c32123d75878b7 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -607,7 +607,7 @@ namespace librbd {
     TracepointProvider::initialize<tracepoint_traits>(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<librbd::trash_image_info_t> 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 d0b79b64cafc6d1f3268e8f6981b86e6ef2fbf3e..452ef3b7c340f8469c17708b2f84d82ceb64bf67 100644 (file)
--- 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 53570db52634201ee076afa0ce62b9d191581b81..3295afe8d7a183d11bc454fdc0bb6a79f117acec 100644 (file)
--- 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 (file)
index 0000000..e7a1e7f
--- /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 <set>
+#include <vector>
+
+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<trash_image_info_t, decltype(compare_lambda)> 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<trash_image_info_t> 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<image_spec_t> expected_images = {
+    {.id = image_id2, .name = image_name2}
+  };
+  std::vector<image_spec_t> 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 17b974b8e6612a0dd4c863b2208cf752a2a90efb..4b66203bd30278ae39b6f7d1ad5105eeeb625af9 100644 (file)
--- 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 f932f73a2427de5787419fed7527f814490c0f4e..327b20ba703b9a400f254c72b11c0b5a3179bfac 100644 (file)
--- 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;
     }