librbd: support deleting image by id instead of name

The rbd-mirror daemon will use this API to delete images instead
of attempting to use the local image name.

Fixes: http://tracker.ceph.com/issues/16227
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 87b32d15914207f61595c1c943817d983faceacd)

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 406c2caa33d9bcd0c20bfa189a6938f5fcd17f61..193fd36b5e618c3001aabe7233dcd3140c2a7589 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1554,7 +1554,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;
@@ -2008,16 +2008,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;
@@ -2026,6 +2030,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();
@@ -2120,7 +2125,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) {
@@ -2133,12 +2138,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);
@@ -2165,7 +2178,7 @@ 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;
@@ -2173,7 +2186,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
       }
 
       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: "

diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index 3fd068691fe0db0212d2fa270dfa82174afb3ea1..48330ecd9885d4e7111b547062273980b36aac98 100644 (file)
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -134,8 +134,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_is_protected(ImageCtx *ictx, const char *snap_name,

diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index b71d5e8c3f6461216ce33099154f5f9eb4ab2edc..2d6d80c7f185601e52e620824812c59616a602e2 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -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;
   }
@@ -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;
   }
@@ -1702,7 +1702,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;
 }
@@ -1715,7 +1715,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;
 }

diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc
index 05034919b149fda74b6b428ca137c2b29dff4472..6ff22d36e77636dc189bc436ea10ad7b5c11f6b1 100644 (file)
--- a/src/test/librbd/image/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/image/test_mock_RefreshRequest.cc
@@ -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"));
   };
 

diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index 407e036255f55b58370be1629d459f5b157d7a21..c6ac977d7325041de91bb2f92352ffb61c07aa31 100644 (file)
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -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));
@@ -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));
@@ -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));
+}

diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc
index ef2200e4c5f340944e7fc82a9e0fe1091246b977..5480316e338f4b6531637c467062ca004f6f4a29 100644 (file)
--- a/src/test/rbd_mirror/test_ImageDeleter.cc
+++ b/src/test/rbd_mirror/test_ImageDeleter.cc
@@ -102,7 +102,7 @@ public:
       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);
   }
 

diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc
index c9d30d33567d3c337a5f4a37a4098cacfce26062..6af2537ba4cf42f5ec0ea066cc5f2c92ca4d7b3c 100644 (file)
--- a/src/tools/rbd_mirror/ImageDeleter.cc
+++ b/src/tools/rbd_mirror/ImageDeleter.cc
@@ -407,8 +407,7 @@ bool ImageDeleter::process_image_delete() {
   }
 
   librbd::NoOpProgressContext ctx;
-  r = librbd::remove(ioctx, m_active_delete->local_image_name.c_str(), ctx,
-                     true);
+  r = librbd::remove(ioctx, "", m_active_delete->local_image_id, ctx, true);
   if (r < 0 && r != -ENOENT) {
     derr << "error removing image " << m_active_delete->local_image_name
          << " from local pool: " << cpp_strerror(r) << dendl;