librbd: disallow trash restoring when image being migrated

Signed-off-by: songweibin <song.weibin@zte.com.cn>

diff --git a/qa/workunits/rbd/cli_generic.sh b/qa/workunits/rbd/cli_generic.sh
index d2066d9d9446d804fb8e8325f3fc92e9d291d179..dc6cdb39c4c591d5ff68e982a100092807b76191 100755 (executable)
--- a/qa/workunits/rbd/cli_generic.sh
+++ b/qa/workunits/rbd/cli_generic.sh
@@ -743,6 +743,14 @@ test_migration() {
     rbd migration execute test1
     rbd migration commit test1
 
+    # testing trash
+    rbd migration prepare test1
+    expect_fail rbd trash mv test1
+    ID=`rbd trash ls -a | cut -d ' ' -f 1`
+    expect_fail rbd trash rm $ID
+    expect_fail rbd trash restore $ID
+    rbd migration abort test1
+
     for format in 1 2; do
         # Abort migration after successful prepare
         rbd create -s 128M --image-format ${format} test2

diff --git a/src/librbd/api/Migration.cc b/src/librbd/api/Migration.cc
index 32c13df3786d9a52dfd4b1a916f41625b3f95246..a7a6eb322215930fea0d3b474b58ec1b64d7fb2c 100644 (file)
--- a/src/librbd/api/Migration.cc
+++ b/src/librbd/api/Migration.cc
@@ -1109,8 +1109,8 @@ template <typename I>
 int Migration<I>::v2_relink_src_image() {
   ldout(m_cct, 10) << dendl;
 
-  int r = Trash<I>::restore(m_src_io_ctx, m_src_image_ctx->id,
-                            m_src_image_ctx->name);
+  int r = Trash<I>::restore(m_src_io_ctx, RBD_TRASH_IMAGE_SOURCE_MIGRATION,
+                            m_src_image_ctx->id, m_src_image_ctx->name);
   if (r < 0) {
     lderr(m_cct) << "failed restoring image from trash: " << cpp_strerror(r)
                  << dendl;

diff --git a/src/librbd/api/Trash.cc b/src/librbd/api/Trash.cc
index c8c40a1eefebf61473f4d7fff8055e00f4ed11c4..f2e78efe4133a4892dbdd2c251a49fb2233a63a8 100644 (file)
--- a/src/librbd/api/Trash.cc
+++ b/src/librbd/api/Trash.cc
@@ -243,7 +243,8 @@ int Trash<I>::remove(IoCtx &io_ctx, const std::string &image_id, bool force,
 }
 
 template <typename I>
-int Trash<I>::restore(librados::IoCtx &io_ctx, const std::string &image_id,
+int Trash<I>::restore(librados::IoCtx &io_ctx, rbd_trash_image_source_t source,
+                      const std::string &image_id,
                       const std::string &image_new_name) {
   CephContext *cct((CephContext *)io_ctx.cct());
   ldout(cct, 20) << "trash_restore " << &io_ctx << " " << image_id << " "
@@ -257,6 +258,13 @@ int Trash<I>::restore(librados::IoCtx &io_ctx, const std::string &image_id,
     return r;
   }
 
+  if (trash_spec.source !=  static_cast<cls::rbd::TrashImageSource>(source)) {
+    lderr(cct) << "Current trash source: " << trash_spec.source
+               << " does not match expected: "
+               << static_cast<cls::rbd::TrashImageSource>(source) << dendl;
+    return -EINVAL;
+  }
+
   std::string image_name = image_new_name;
   if (trash_spec.state != cls::rbd::TRASH_IMAGE_STATE_NORMAL &&
       trash_spec.state != cls::rbd::TRASH_IMAGE_STATE_RESTORING) {

diff --git a/src/librbd/api/Trash.h b/src/librbd/api/Trash.h
index cf7217de8098e1c716de88eb986fb87496ba07ff..34f69f498cb5c44ad0d87344a8507c93ed298861 100644 (file)
--- a/src/librbd/api/Trash.h
+++ b/src/librbd/api/Trash.h
@@ -29,7 +29,8 @@ struct Trash {
                   std::vector<trash_image_info_t> &entries);
   static int remove(librados::IoCtx &io_ctx, const std::string &image_id,
                     bool force, ProgressContext& prog_ctx);
-  static int restore(librados::IoCtx &io_ctx, const std::string &image_id,
+  static int restore(librados::IoCtx &io_ctx, rbd_trash_image_source_t source,
+                     const std::string &image_id,
                      const std::string &image_new_name);
 
 };

diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index 817b464274c25d37cf68266ffcfb45cd2688e710..12ef1e0b03462b5358ab7babe78e675798d1c61c 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -643,7 +643,8 @@ namespace librbd {
     TracepointProvider::initialize<tracepoint_traits>(get_cct(io_ctx));
     tracepoint(librbd, trash_undelete_enter, io_ctx.get_pool_name().c_str(),
                io_ctx.get_id(), id, name);
-    int r = librbd::api::Trash<>::restore(io_ctx, id, name);
+    int r = librbd::api::Trash<>::restore(io_ctx, RBD_TRASH_IMAGE_SOURCE_USER,
+                                          id, name);
     tracepoint(librbd, trash_undelete_exit, r);
     return r;
   }
@@ -3216,7 +3217,8 @@ extern "C" int rbd_trash_restore(rados_ioctx_t p, const char *id,
   TracepointProvider::initialize<tracepoint_traits>(get_cct(io_ctx));
   tracepoint(librbd, trash_undelete_enter, io_ctx.get_pool_name().c_str(),
              io_ctx.get_id(), id, name);
-  int r = librbd::api::Trash<>::restore(io_ctx, id, name);
+  int r = librbd::api::Trash<>::restore(io_ctx, RBD_TRASH_IMAGE_SOURCE_USER,
+                                        id, name);
   tracepoint(librbd, trash_undelete_exit, r);
   return r;
 }

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index bbc4777268c6878cd6bea3a2dabe44c5f9d72c4c..74f4d7d4e9a280f85a5018c4160336e01d056f98 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -6920,6 +6920,8 @@ TEST_F(TestLibRBD, Migration) {
     ASSERT_EQ(status.source_image_id, string());
   } else {
     ASSERT_NE(status.source_image_id, string());
+    ASSERT_EQ(-EROFS, rbd_trash_remove(ioctx, status.source_image_id, false));
+    ASSERT_EQ(-EINVAL, rbd_trash_restore(ioctx, status.source_image_id, name.c_str()));
   }
   ASSERT_EQ(status.dest_pool_id, rados_ioctx_get_id(ioctx));
   ASSERT_EQ(status.dest_image_name, name);
@@ -6928,6 +6930,7 @@ TEST_F(TestLibRBD, Migration) {
   rbd_migration_status_cleanup(&status);
 
   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()));
 
@@ -6944,6 +6947,7 @@ TEST_F(TestLibRBD, Migration) {
                                      new_name.c_str(), image_options));
 
   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()));
 
@@ -6980,6 +6984,8 @@ TEST_F(TestLibRBD, MigrationPP) {
     ASSERT_EQ(status.source_image_id, "");
   } else {
     ASSERT_NE(status.source_image_id, "");
+    ASSERT_EQ(-EROFS, rbd.trash_remove(ioctx, status.source_image_id.c_str(), false));
+    ASSERT_EQ(-EINVAL, rbd.trash_restore(ioctx, status.source_image_id.c_str(), name.c_str()));
   }
   ASSERT_EQ(status.dest_pool_id, ioctx.get_id());
   ASSERT_EQ(status.dest_image_name, name);
@@ -6987,6 +6993,7 @@ TEST_F(TestLibRBD, MigrationPP) {
   ASSERT_EQ(status.state, RBD_IMAGE_MIGRATION_STATE_PREPARED);
 
   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()));
 
@@ -7002,6 +7009,7 @@ TEST_F(TestLibRBD, MigrationPP) {
                                      new_name.c_str(), image_options));
 
   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()));