From: Jason Dillaman <dillaman@redhat.com>
Date: Thu, 21 Feb 2019 03:32:38 +0000 (-0500)
Subject: librbd: execute image pre-removal validation before moving image to trash
X-Git-Tag: v14.1.0~1^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3a8553498cc5e197c4e5636458a0a15d37677abb;p=ceph.git

librbd: execute image pre-removal validation before moving image to trash

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

diff --git a/src/librbd/api/Image.cc b/src/librbd/api/Image.cc
index a999ed744b24..48bec35da0c2 100644
--- a/src/librbd/api/Image.cc
+++ b/src/librbd/api/Image.cc
@@ -15,8 +15,9 @@
 #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 "librbd/image/RemoveRequest.h"
+#include "librbd/image/PreRemoveRequest.h"
 #include <boost/scope_exit.hpp>
 
 #define dout_subsys ceph_subsys_rbd
@@ -56,6 +57,22 @@ bool compare(const librbd::linked_image_spec_t& lhs,
   return false;
 }
 
+template <typename I>
+int pre_remove_image(librados::IoCtx& io_ctx, const std::string& image_id) {
+  I *image_ctx = I::create("", image_id, nullptr, io_ctx, false);
+  int r = image_ctx->state->open(OPEN_FLAG_SKIP_OPEN_PARENT);
+  if (r < 0) {
+    return r;
+  }
+
+  C_SaferCond ctx;
+  auto req = image::PreRemoveRequest<I>::create(image_ctx, false, &ctx);
+  req->send();
+
+  r = ctx.wait();
+  image_ctx->state->close();
+  return r;
+}
 
 } // anonymous namespace
 
@@ -740,6 +757,13 @@ int Image<I>::remove(IoCtx& io_ctx, const std::string &image_name,
       trash_image_source = RBD_TRASH_IMAGE_SOURCE_USER;
       expire_seconds = config.get_val<uint64_t>(
         "rbd_move_to_trash_on_remove_expire_seconds");
+    } else {
+      // attempt to pre-validate the removal before moving to trash and
+      // removing
+      r = pre_remove_image<I>(io_ctx, image_id);
+      if (r < 0 && r != -ENOENT) {
+        return r;
+      }
     }
 
     r = Trash<I>::move(io_ctx, trash_image_source, image_name, image_id,
@@ -759,6 +783,8 @@ int Image<I>::remove(IoCtx& io_ctx, const std::string &image_name,
     } else if (r < 0 && r != -EOPNOTSUPP) {
       return r;
     }
+
+    // fall-through if trash isn't supported
   }
 
   ThreadPool *thread_pool;
diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index acf1a2227280..b87caec6dd32 100644
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -7206,7 +7206,7 @@ TEST_F(TestLibRBD, Migration) {
   ASSERT_EQ(status.state, RBD_IMAGE_MIGRATION_STATE_PREPARED);
   rbd_migration_status_cleanup(&status);
 
-  ASSERT_EQ(-EINVAL, rbd_remove(ioctx, name.c_str()));
+  ASSERT_EQ(-EBUSY, rbd_remove(ioctx, name.c_str()));
   ASSERT_EQ(-EINVAL, rbd_trash_move(ioctx, name.c_str(), 0));
 
   ASSERT_EQ(0, rbd_migration_execute(ioctx, name.c_str()));
@@ -7223,7 +7223,7 @@ TEST_F(TestLibRBD, Migration) {
   ASSERT_EQ(0, rbd_migration_prepare(ioctx, name.c_str(), ioctx,
                                      new_name.c_str(), image_options));
 
-  ASSERT_EQ(-EINVAL, rbd_remove(ioctx, new_name.c_str()));
+  ASSERT_EQ(-EBUSY, rbd_remove(ioctx, new_name.c_str()));
   ASSERT_EQ(-EINVAL, rbd_trash_move(ioctx, new_name.c_str(), 0));
 
   ASSERT_EQ(0, rbd_migration_abort(ioctx, name.c_str()));
@@ -7269,7 +7269,7 @@ TEST_F(TestLibRBD, MigrationPP) {
   ASSERT_NE(status.dest_image_id, "");
   ASSERT_EQ(status.state, RBD_IMAGE_MIGRATION_STATE_PREPARED);
 
-  ASSERT_EQ(-EINVAL, rbd.remove(ioctx, name.c_str()));
+  ASSERT_EQ(-EBUSY, rbd.remove(ioctx, name.c_str()));
   ASSERT_EQ(-EINVAL, rbd.trash_move(ioctx, name.c_str(), 0));
 
   ASSERT_EQ(0, rbd.migration_execute(ioctx, name.c_str()));
@@ -7285,7 +7285,7 @@ TEST_F(TestLibRBD, MigrationPP) {
   ASSERT_EQ(0, rbd.migration_prepare(ioctx, name.c_str(), ioctx,
                                      new_name.c_str(), image_options));
 
-  ASSERT_EQ(-EINVAL, rbd.remove(ioctx, new_name.c_str()));
+  ASSERT_EQ(-EBUSY, rbd.remove(ioctx, new_name.c_str()));
   ASSERT_EQ(-EINVAL, rbd.trash_move(ioctx, new_name.c_str(), 0));
 
   ASSERT_EQ(0, rbd.migration_abort(ioctx, name.c_str()));