From 1f2eef7f1fbb842c63aab3e8561d700ecf7a09ea Mon Sep 17 00:00:00 2001 From: songweibin Date: Thu, 13 Dec 2018 10:37:14 +0800 Subject: [PATCH] librbd: disallow trash restoring when image being migrated Signed-off-by: songweibin --- qa/workunits/rbd/cli_generic.sh | 8 ++++++++ src/librbd/api/Migration.cc | 4 ++-- src/librbd/api/Trash.cc | 10 +++++++++- src/librbd/api/Trash.h | 3 ++- src/librbd/librbd.cc | 6 ++++-- src/test/librbd/test_librbd.cc | 8 ++++++++ 6 files changed, 33 insertions(+), 6 deletions(-) diff --git a/qa/workunits/rbd/cli_generic.sh b/qa/workunits/rbd/cli_generic.sh index d2066d9d944..dc6cdb39c4c 100755 --- 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 32c13df3786..a7a6eb32221 100644 --- a/src/librbd/api/Migration.cc +++ b/src/librbd/api/Migration.cc @@ -1109,8 +1109,8 @@ template int Migration::v2_relink_src_image() { ldout(m_cct, 10) << dendl; - int r = Trash::restore(m_src_io_ctx, m_src_image_ctx->id, - m_src_image_ctx->name); + int r = Trash::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 c8c40a1eefe..f2e78efe413 100644 --- a/src/librbd/api/Trash.cc +++ b/src/librbd/api/Trash.cc @@ -243,7 +243,8 @@ int Trash::remove(IoCtx &io_ctx, const std::string &image_id, bool force, } template -int Trash::restore(librados::IoCtx &io_ctx, const std::string &image_id, +int Trash::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::restore(librados::IoCtx &io_ctx, const std::string &image_id, return r; } + if (trash_spec.source != static_cast(source)) { + lderr(cct) << "Current trash source: " << trash_spec.source + << " does not match expected: " + << static_cast(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 cf7217de809..34f69f498cb 100644 --- a/src/librbd/api/Trash.h +++ b/src/librbd/api/Trash.h @@ -29,7 +29,8 @@ struct Trash { std::vector &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 817b464274c..12ef1e0b034 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -643,7 +643,8 @@ namespace librbd { TracepointProvider::initialize(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(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 bbc4777268c..74f4d7d4e9a 100644 --- 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())); -- 2.39.5