From: Jason Dillaman Date: Tue, 4 Aug 2020 21:09:57 +0000 (-0400) Subject: librbd: track in-progress migration aborting operation X-Git-Tag: v15.2.9~122^2~111^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2bc5229717326298ca964333fc07a38c5e48701e;p=ceph.git librbd: track in-progress migration aborting operation We want to prevent the destination image from being used while an abort is in-progress. Test that the image has no watchers prior to permitting the abort, switch the migration state to ABORTING, and treat the image as read-only if the migration state is ABORTING. Signed-off-by: Jason Dillaman (cherry picked from commit d848b4f1c083bd9f3e6eac3e186c9e7df2e22004) --- diff --git a/src/cls/rbd/cls_rbd_types.cc b/src/cls/rbd/cls_rbd_types.cc index 5f3c40b049f2f..b6a02496bcb5f 100644 --- a/src/cls/rbd/cls_rbd_types.cc +++ b/src/cls/rbd/cls_rbd_types.cc @@ -1201,6 +1201,9 @@ std::ostream& operator<<(std::ostream& os, case MIGRATION_STATE_EXECUTED: os << "executed"; break; + case MIGRATION_STATE_ABORTING: + os << "aborting"; + break; default: os << "unknown (" << static_cast(migration_state) << ")"; break; diff --git a/src/cls/rbd/cls_rbd_types.h b/src/cls/rbd/cls_rbd_types.h index 5ea37caf7d45c..ab77735f8f906 100644 --- a/src/cls/rbd/cls_rbd_types.h +++ b/src/cls/rbd/cls_rbd_types.h @@ -926,6 +926,7 @@ enum MigrationState { MIGRATION_STATE_PREPARED = 2, MIGRATION_STATE_EXECUTING = 3, MIGRATION_STATE_EXECUTED = 4, + MIGRATION_STATE_ABORTING = 5, }; inline void encode(const MigrationState &state, bufferlist& bl) { diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h index c06c6017118ef..495e904a697fc 100644 --- a/src/include/rbd/librbd.h +++ b/src/include/rbd/librbd.h @@ -325,6 +325,7 @@ typedef enum { RBD_IMAGE_MIGRATION_STATE_PREPARED = 2, RBD_IMAGE_MIGRATION_STATE_EXECUTING = 3, RBD_IMAGE_MIGRATION_STATE_EXECUTED = 4, + RBD_IMAGE_MIGRATION_STATE_ABORTING = 5, } rbd_image_migration_state_t; typedef struct { diff --git a/src/librbd/api/Migration.cc b/src/librbd/api/Migration.cc index a5c2241829393..a2cdabe1d589d 100644 --- a/src/librbd/api/Migration.cc +++ b/src/librbd/api/Migration.cc @@ -791,8 +791,36 @@ int Migration::abort() { ldout(m_cct, 1) << "failed to open destination image: " << cpp_strerror(r) << dendl; } else { - ldout(m_cct, 10) << "relinking children" << dendl; + BOOST_SCOPE_EXIT_TPL(&dst_image_ctx) { + if (dst_image_ctx != nullptr) { + dst_image_ctx->state->close(); + } + } BOOST_SCOPE_EXIT_END; + + std::list watchers; + int flags = librbd::image::LIST_WATCHERS_FILTER_OUT_MY_INSTANCE | + librbd::image::LIST_WATCHERS_FILTER_OUT_MIRROR_INSTANCES; + C_SaferCond on_list_watchers; + auto list_watchers_request = librbd::image::ListWatchersRequest::create( + *dst_image_ctx, flags, &watchers, &on_list_watchers); + list_watchers_request->send(); + r = on_list_watchers.wait(); + if (r < 0) { + lderr(m_cct) << "failed listing watchers:" << cpp_strerror(r) << dendl; + return r; + } + if (!watchers.empty()) { + lderr(m_cct) << "image has watchers - cannot abort migration" << dendl; + return -EBUSY; + } + // ensure destination image is now read-only + r = set_state(cls::rbd::MIGRATION_STATE_ABORTING, ""); + if (r < 0) { + return r; + } + + ldout(m_cct, 10) << "relinking children" << dendl; r = relink_children(dst_image_ctx, m_src_image_ctx); if (r < 0) { return r; @@ -800,12 +828,6 @@ int Migration::abort() { ldout(m_cct, 10) << "removing dst image snapshots" << dendl; - BOOST_SCOPE_EXIT_TPL(&dst_image_ctx) { - if (dst_image_ctx != nullptr) { - dst_image_ctx->state->close(); - } - } BOOST_SCOPE_EXIT_END; - std::vector snaps; r = Snapshot::list(dst_image_ctx, snaps); if (r < 0) { @@ -853,7 +875,7 @@ int Migration::abort() { << m_dst_io_ctx.get_pool_name() << "/" << m_dst_image_name << " (" << m_dst_image_id << ")': " << cpp_strerror(r) << dendl; - // not fatal + return r; } } diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index 560b22ee6106b..590242c353e06 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -124,13 +124,29 @@ Context *RefreshRequest::handle_get_migration_header(int *result) { case cls::rbd::MIGRATION_HEADER_TYPE_DST: ldout(cct, 1) << this << " " << __func__ << ": migrating from: " << m_migration_spec << dendl; - if (m_migration_spec.state != cls::rbd::MIGRATION_STATE_PREPARED && - m_migration_spec.state != cls::rbd::MIGRATION_STATE_EXECUTING && - m_migration_spec.state != cls::rbd::MIGRATION_STATE_EXECUTED) { + switch (m_migration_spec.state) { + case cls::rbd::MIGRATION_STATE_PREPARING: ldout(cct, 5) << this << " " << __func__ << ": current migration state: " << m_migration_spec.state << ", retrying" << dendl; send(); return nullptr; + case cls::rbd::MIGRATION_STATE_PREPARED: + case cls::rbd::MIGRATION_STATE_EXECUTING: + case cls::rbd::MIGRATION_STATE_EXECUTED: + break; + case cls::rbd::MIGRATION_STATE_ABORTING: + if (!m_read_only) { + lderr(cct) << this << " " << __func__ << ": migration is being aborted" + << dendl; + *result = -EROFS; + return m_on_finish; + } + break; + default: + lderr(cct) << this << " " << __func__ << ": migration is in an " + << "unexpected state" << dendl; + *result = -EINVAL; + return m_on_finish; } break; default: @@ -1301,8 +1317,14 @@ void RefreshRequest::apply() { m_image_ctx.operations_disabled = ( (m_op_features & ~RBD_OPERATION_FEATURES_ALL) != 0ULL); m_image_ctx.group_spec = m_group_spec; - if (get_migration_info(&m_image_ctx.parent_md, - &m_image_ctx.migration_info)) { + + bool migration_info_valid; + int r = get_migration_info(&m_image_ctx.parent_md, + &m_image_ctx.migration_info, + &migration_info_valid); + ceph_assert(r == 0); // validated in refresh parent step + + if (migration_info_valid) { for (auto it : m_image_ctx.migration_info.snap_map) { migration_reverse_snap_seq[it.second.front()] = it.first; } @@ -1313,7 +1335,7 @@ void RefreshRequest::apply() { librados::Rados rados(m_image_ctx.md_ctx); int8_t require_osd_release; - int r = rados.get_min_compatible_osd(&require_osd_release); + r = rados.get_min_compatible_osd(&require_osd_release); if (r == 0 && require_osd_release >= CEPH_RELEASE_OCTOPUS) { m_image_ctx.enable_sparse_copyup = true; } @@ -1417,7 +1439,13 @@ template int RefreshRequest::get_parent_info(uint64_t snap_id, ParentImageInfo *parent_md, MigrationInfo *migration_info) { - if (get_migration_info(parent_md, migration_info)) { + bool migration_info_valid; + int r = get_migration_info(parent_md, migration_info, &migration_info_valid); + if (r < 0) { + return r; + } + + if (migration_info_valid) { return 0; } else if (snap_id == CEPH_NOSNAP) { *parent_md = m_parent_md; @@ -1436,17 +1464,24 @@ int RefreshRequest::get_parent_info(uint64_t snap_id, } template -bool RefreshRequest::get_migration_info(ParentImageInfo *parent_md, - MigrationInfo *migration_info) { +int RefreshRequest::get_migration_info(ParentImageInfo *parent_md, + MigrationInfo *migration_info, + bool* migration_info_valid) { + CephContext *cct = m_image_ctx.cct; if (m_migration_spec.header_type != cls::rbd::MIGRATION_HEADER_TYPE_DST || (m_migration_spec.state != cls::rbd::MIGRATION_STATE_PREPARED && - m_migration_spec.state != cls::rbd::MIGRATION_STATE_EXECUTING)) { - ceph_assert(m_migration_spec.header_type == - cls::rbd::MIGRATION_HEADER_TYPE_SRC || - m_migration_spec.pool_id == -1 || - m_migration_spec.state == cls::rbd::MIGRATION_STATE_EXECUTED); + m_migration_spec.state != cls::rbd::MIGRATION_STATE_EXECUTING && + m_migration_spec.state != cls::rbd::MIGRATION_STATE_ABORTING)) { + if (m_migration_spec.header_type != cls::rbd::MIGRATION_HEADER_TYPE_SRC && + m_migration_spec.pool_id != -1 && + m_migration_spec.state != cls::rbd::MIGRATION_STATE_EXECUTED) { + lderr(cct) << this << " " << __func__ << ": invalid migration spec" + << dendl; + return -EINVAL; + } - return false; + *migration_info_valid = false; + return 0; } parent_md->spec.pool_id = m_migration_spec.pool_id; @@ -1483,10 +1518,11 @@ bool RefreshRequest::get_migration_info(ParentImageInfo *parent_md, *migration_info = {m_migration_spec.pool_id, m_migration_spec.pool_namespace, m_migration_spec.image_name, m_migration_spec.image_id, {}, overlap, m_migration_spec.flatten}; + *migration_info_valid = true; deep_copy::util::compute_snap_map(m_image_ctx.cct, 0, CEPH_NOSNAP, {}, snap_seqs, &migration_info->snap_map); - return true; + return 0; } } // namespace image diff --git a/src/librbd/image/RefreshRequest.h b/src/librbd/image/RefreshRequest.h index 4e314c7e6980c..6970a9b45626c 100644 --- a/src/librbd/image/RefreshRequest.h +++ b/src/librbd/image/RefreshRequest.h @@ -258,8 +258,9 @@ private: void apply(); int get_parent_info(uint64_t snap_id, ParentImageInfo *parent_md, MigrationInfo *migration_info); - bool get_migration_info(ParentImageInfo *parent_md, - MigrationInfo *migration_info); + int get_migration_info(ParentImageInfo *parent_md, + MigrationInfo *migration_info, + bool* migration_info_valid); }; } // namespace image diff --git a/src/pybind/rbd/rbd.pyx b/src/pybind/rbd/rbd.pyx index 72da0ee9fbb35..70d032b478913 100644 --- a/src/pybind/rbd/rbd.pyx +++ b/src/pybind/rbd/rbd.pyx @@ -273,6 +273,7 @@ cdef extern from "rbd/librbd.h" nogil: _RBD_IMAGE_MIGRATION_STATE_PREPARED "RBD_IMAGE_MIGRATION_STATE_PREPARED" _RBD_IMAGE_MIGRATION_STATE_EXECUTING "RBD_IMAGE_MIGRATION_STATE_EXECUTING" _RBD_IMAGE_MIGRATION_STATE_EXECUTED "RBD_IMAGE_MIGRATION_STATE_EXECUTED" + _RBD_IMAGE_MIGRATION_STATE_ABORTING "RBD_IMAGE_MIGRATION_STATE_ABORTING" ctypedef struct rbd_image_migration_status_t: int64_t source_pool_id @@ -790,6 +791,7 @@ RBD_IMAGE_MIGRATION_STATE_PREPARING = _RBD_IMAGE_MIGRATION_STATE_PREPARING RBD_IMAGE_MIGRATION_STATE_PREPARED = _RBD_IMAGE_MIGRATION_STATE_PREPARED RBD_IMAGE_MIGRATION_STATE_EXECUTING = _RBD_IMAGE_MIGRATION_STATE_EXECUTING RBD_IMAGE_MIGRATION_STATE_EXECUTED = _RBD_IMAGE_MIGRATION_STATE_EXECUTED +RBD_IMAGE_MIGRATION_STATE_ABORTING = _RBD_IMAGE_MIGRATION_STATE_ABORTING RBD_CONFIG_SOURCE_CONFIG = _RBD_CONFIG_SOURCE_CONFIG RBD_CONFIG_SOURCE_POOL = _RBD_CONFIG_SOURCE_POOL diff --git a/src/test/librbd/test_Migration.cc b/src/test/librbd/test_Migration.cc index 19512036ad365..38fdabe52ecfd 100644 --- a/src/test/librbd/test_Migration.cc +++ b/src/test/librbd/test_Migration.cc @@ -1107,6 +1107,15 @@ TEST_F(TestMigration, SnapTrimBeforePrepare) migration_commit(m_ioctx, m_image_name); } +TEST_F(TestMigration, AbortInUseImage) { + migration_prepare(m_ioctx, m_image_name); + migration_status(RBD_IMAGE_MIGRATION_STATE_PREPARED); + + librbd::NoOpProgressContext no_op; + EXPECT_EQ(-EBUSY, librbd::api::Migration<>::abort(m_ioctx, m_ictx->name, + no_op)); +} + TEST_F(TestMigration, CloneV1Parent) { const uint32_t CLONE_FORMAT = 1; diff --git a/src/tools/rbd/action/Status.cc b/src/tools/rbd/action/Status.cc index 75afdd7743117..0a599e7f0e04d 100644 --- a/src/tools/rbd/action/Status.cc +++ b/src/tools/rbd/action/Status.cc @@ -79,6 +79,9 @@ static int do_show_status(librados::IoCtx& io_ctx, const std::string &image_name case RBD_IMAGE_MIGRATION_STATE_EXECUTED: migration_state = "executed"; break; + case RBD_IMAGE_MIGRATION_STATE_ABORTING: + migration_state = "aborting"; + break; default: migration_state = "unknown"; }