rbd-mirror: use pool id + global image id as deletion primary key

Fixes: http://tracker.ceph.com/issues/16538
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc
index d96b6249078cb04ee7e9ad7641e6ecae9618444e..ef2200e4c5f340944e7fc82a9e0fe1091246b977 100644 (file)
--- a/src/test/rbd_mirror/test_ImageDeleter.cc
+++ b/src/test/rbd_mirror/test_ImageDeleter.cc
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());
@@ -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());

diff --git a/src/test/rbd_mirror/test_ImageReplayer.cc b/src/test/rbd_mirror/test_ImageReplayer.cc
index ca60340b64963576a476929fc126a60d21eb0879..fec75d1565ea133d62c5b29c518e8a74e83af000 100644 (file)
--- a/src/test/rbd_mirror/test_ImageReplayer.cc
+++ b/src/test/rbd_mirror/test_ImageReplayer.cc
@@ -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;
@@ -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;
@@ -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;

diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc
index e4ad55ae60f3ea1510848e496dc1ee0ca0d3b8f8..d69458ce88762793318e4881f9ac2c46c557bafe 100644 (file)
--- a/src/tools/rbd_mirror/ImageDeleter.cc
+++ b/src/tools/rbd_mirror/ImageDeleter.cc
@@ -192,18 +192,18 @@ void ImageDeleter::run() {
 }
 
 void ImageDeleter::schedule_image_delete(RadosRef local_rados,
-                                         uint64_t local_pool_id,
+                                         int64_t local_pool_id,
                                          const std::string& local_image_id,
                                          const std::string& local_image_name,
                                          const std::string& global_image_id) {
   dout(20) << "enter" << dendl;
 
-  Mutex::Locker l(m_delete_lock);
+  Mutex::Locker locker(m_delete_lock);
 
-  auto del_info = find_delete_info(local_image_name);
+  auto del_info = find_delete_info(local_pool_id, global_image_id);
   if (del_info != nullptr) {
-    dout(20) << "image " << local_image_name << " was already scheduled for "
-             << "deletion" << dendl;
+    dout(20) << "image " << local_image_name << " (" << global_image_id << ") "
+             << "was already scheduled for deletion" << dendl;
     return;
   }
 
@@ -213,7 +213,8 @@ void ImageDeleter::schedule_image_delete(RadosRef local_rados,
   m_delete_queue_cond.Signal();
 }
 
-void ImageDeleter::wait_for_scheduled_deletion(const std::string& image_name,
+void ImageDeleter::wait_for_scheduled_deletion(int64_t local_pool_id,
+                                               const std::string &global_image_id,
                                                Context *ctx,
                                                bool notify_on_failed_retry) {
 
@@ -221,8 +222,8 @@ void ImageDeleter::wait_for_scheduled_deletion(const std::string& image_name,
       m_work_queue->queue(ctx, r);
     });
 
-  Mutex::Locker l(m_delete_lock);
-  auto del_info = find_delete_info(image_name);
+  Mutex::Locker locker(m_delete_lock);
+  auto del_info = find_delete_info(local_pool_id, global_image_id);
   if (!del_info) {
     // image not scheduled for deletion
     ctx->complete(0);
@@ -236,9 +237,10 @@ void ImageDeleter::wait_for_scheduled_deletion(const std::string& image_name,
   (*del_info)->notify_on_failed_retry = notify_on_failed_retry;
 }
 
-void ImageDeleter::cancel_waiter(const std::string& image_name) {
+void ImageDeleter::cancel_waiter(int64_t local_pool_id,
+                                 const std::string &global_image_id) {
   Mutex::Locker locker(m_delete_lock);
-  auto del_info = find_delete_info(image_name);
+  auto del_info = find_delete_info(local_pool_id, global_image_id);
   if (!del_info) {
     return;
   }
@@ -506,21 +508,22 @@ void ImageDeleter::retry_failed_deletions() {
 }
 
 unique_ptr<ImageDeleter::DeleteInfo> const* ImageDeleter::find_delete_info(
-    const std::string& image_name) {
+    int64_t local_pool_id, const std::string &global_image_id) {
   assert(m_delete_lock.is_locked());
 
-  if (m_active_delete && m_active_delete->match(image_name)) {
+  if (m_active_delete && m_active_delete->match(local_pool_id,
+                                                global_image_id)) {
     return &m_active_delete;
   }
 
   for (const auto& del_info : m_delete_queue) {
-    if (del_info->match(image_name)) {
+    if (del_info->match(local_pool_id, global_image_id)) {
       return &del_info;
     }
   }
 
   for (const auto& del_info : m_failed_queue) {
-    if (del_info->match(image_name)) {
+    if (del_info->match(local_pool_id, global_image_id)) {
       return &del_info;
     }
   }

diff --git a/src/tools/rbd_mirror/ImageDeleter.h b/src/tools/rbd_mirror/ImageDeleter.h
index 3d994b1bbc94b80ad61b5cc9dd14040897245399..d9b0f2e74340522ca2f936c9c452d71a5bd4b9c3 100644 (file)
--- a/src/tools/rbd_mirror/ImageDeleter.h
+++ b/src/tools/rbd_mirror/ImageDeleter.h
@@ -44,14 +44,16 @@ public:
   ImageDeleter& operator=(const ImageDeleter&) = delete;
 
   void schedule_image_delete(RadosRef local_rados,
-                             uint64_t local_pool_id,
+                             int64_t local_pool_id,
                              const std::string& local_image_id,
                              const std::string& local_image_name,
                              const std::string& global_image_id);
-  void wait_for_scheduled_deletion(const std::string& image_name,
+  void wait_for_scheduled_deletion(int64_t local_pool_id,
+                                   const std::string &global_image_id,
                                    Context *ctx,
                                    bool notify_on_failed_retry=true);
-  void cancel_waiter(const std::string& image_name);
+  void cancel_waiter(int64_t local_pool_id,
+                     const std::string &global_image_id);
 
   void print_status(Formatter *f, std::stringstream *ss);
 
@@ -75,7 +77,7 @@ private:
 
   struct DeleteInfo {
     RadosRef local_rados;
-    uint64_t local_pool_id;
+    int64_t local_pool_id;
     std::string local_image_id;
     std::string local_image_name;
     std::string global_image_id;
@@ -84,7 +86,7 @@ private:
     bool notify_on_failed_retry;
     Context *on_delete;
 
-    DeleteInfo(RadosRef local_rados, uint64_t local_pool_id,
+    DeleteInfo(RadosRef local_rados, int64_t local_pool_id,
                const std::string& local_image_id,
                const std::string& local_image_name,
                const std::string& global_image_id) :
@@ -94,8 +96,9 @@ private:
       notify_on_failed_retry(true), on_delete(nullptr) {
     }
 
-    bool match(const std::string& image_name) {
-      return local_image_name == image_name;
+    bool match(int64_t local_pool_id, const std::string &global_image_id) {
+      return (this->local_pool_id == local_pool_id &&
+              this->global_image_id == global_image_id);
     }
     void notify(int r);
     void to_string(std::stringstream& ss);
@@ -133,7 +136,9 @@ private:
   void enqueue_failed_delete(int error_code);
   void retry_failed_deletions();
 
-  unique_ptr<DeleteInfo> const* find_delete_info(const std::string& image_name);
+  unique_ptr<DeleteInfo> const*
+  find_delete_info(int64_t local_pool_id, const std::string &global_image_id);
+
 };
 
 } // namespace mirror

diff --git a/src/tools/rbd_mirror/Replayer.cc b/src/tools/rbd_mirror/Replayer.cc
index 9cefd78edeaa006bc52354927cc93cdf86f32f47..322f8f48e00338a59449607d5a4a600caafc368d 100644 (file)
--- a/src/tools/rbd_mirror/Replayer.cc
+++ b/src/tools/rbd_mirror/Replayer.cc
@@ -738,7 +738,9 @@ void Replayer::start_image_replayer(unique_ptr<ImageReplayer<> > &image_replayer
           }
        }
     );
-    m_image_deleter->wait_for_scheduled_deletion(image_name.get(), ctx, false);
+
+    m_image_deleter->wait_for_scheduled_deletion(
+      m_local_pool_id, image_replayer->get_global_image_id(), ctx, false);
   }
 }
 
@@ -750,7 +752,9 @@ bool Replayer::stop_image_replayer(unique_ptr<ImageReplayer<> > &image_replayer)
 
   // TODO: check how long it is stopping and alert if it is too long.
   if (image_replayer->is_stopped()) {
-    m_image_deleter->cancel_waiter(image_replayer->get_local_image_name());
+    m_image_deleter->cancel_waiter(m_local_pool_id,
+                                   image_replayer->get_global_image_id());
+
     if (!m_stopping.read()) {
       dout(20) << "scheduling delete" << dendl;
       m_image_deleter->schedule_image_delete(