From: Jason Dillaman Date: Tue, 4 Aug 2020 21:09:57 +0000 (-0400) Subject: librbd: track in-progress migration aborting operation X-Git-Tag: v14.2.12~68^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=423556aab73bb6549022c24b1f56f12ccb2f6031;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) (cherry picked from commit 2bc5229717326298ca964333fc07a38c5e48701e) Conflicts: src/librbd/image/RefreshRequest.cc: lock and read-only flags refactor --- diff --git a/src/cls/rbd/cls_rbd_types.cc b/src/cls/rbd/cls_rbd_types.cc index 1d7f932e9709..0d2c9c754540 100644 --- a/src/cls/rbd/cls_rbd_types.cc +++ b/src/cls/rbd/cls_rbd_types.cc @@ -801,6 +801,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 bbfc85870b21..073006b3b993 100644 --- a/src/cls/rbd/cls_rbd_types.h +++ b/src/cls/rbd/cls_rbd_types.h @@ -700,6 +700,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 3d27e394eee2..522a6fb6d482 100644 --- a/src/include/rbd/librbd.h +++ b/src/include/rbd/librbd.h @@ -270,6 +270,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 d885948cfba4..813eeeeca249 100644 --- a/src/librbd/api/Migration.cc +++ b/src/librbd/api/Migration.cc @@ -777,8 +777,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; @@ -786,12 +814,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 = snap_list(dst_image_ctx, snaps); if (r < 0) { @@ -839,7 +861,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 ad3c1bb86430..3cc1b79ec382 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -130,13 +130,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_image_ctx.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: @@ -1327,8 +1343,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; } @@ -1437,7 +1459,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; @@ -1456,17 +1484,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; @@ -1503,10 +1538,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 dc1d86b33cc4..0a0f1e8d709c 100644 --- a/src/librbd/image/RefreshRequest.h +++ b/src/librbd/image/RefreshRequest.h @@ -257,8 +257,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 3ce8ed4463b9..264c91fbc27e 100644 --- a/src/pybind/rbd/rbd.pyx +++ b/src/pybind/rbd/rbd.pyx @@ -240,6 +240,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 @@ -710,6 +711,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 d6c8fac62c0c..164a5ba9cfb5 100644 --- a/src/test/librbd/test_Migration.cc +++ b/src/test/librbd/test_Migration.cc @@ -1102,6 +1102,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 75afdd774311..0a599e7f0e04 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"; }