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 <dillaman@redhat.com>

diff --git a/src/cls/rbd/cls_rbd_types.cc b/src/cls/rbd/cls_rbd_types.cc
index cf36de94805ac5737ba03a682006dc3df2d277e9..a331b37c23b0832b661e43967be40acf38a8186d 100644 (file)
--- a/src/cls/rbd/cls_rbd_types.cc
+++ b/src/cls/rbd/cls_rbd_types.cc
@@ -1208,6 +1208,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<uint32_t>(migration_state) << ")";
     break;

diff --git a/src/cls/rbd/cls_rbd_types.h b/src/cls/rbd/cls_rbd_types.h
index d244592f2595155de0941590e07bb338fb11d53d..c6bfad032c199a2f0acb602cb99fef382bfca46a 100644 (file)
--- 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, ceph::buffer::list& bl) {

diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h
index 68b4f698009f490cdd0d792dfe911be2c8e13b64..ad48e8863b47077920e3dabf6a9f4480b987630e 100644 (file)
--- a/src/include/rbd/librbd.h
+++ b/src/include/rbd/librbd.h
@@ -328,6 +328,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 ec071225888089539ed29410c8a1b4df92ea1bd4..b2871bf1cf7cbcd943f7c7a7ebcf22539ec0b670 100644 (file)
--- a/src/librbd/api/Migration.cc
+++ b/src/librbd/api/Migration.cc
@@ -792,8 +792,36 @@ int Migration<I>::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<obj_watch_t> 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<I>::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;
@@ -801,12 +829,6 @@ int Migration<I>::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<librbd::snap_info_t> snaps;
     r = Snapshot<I>::list(dst_image_ctx, snaps);
     if (r < 0) {
@@ -853,7 +875,7 @@ int Migration<I>::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 c1ccde4000ec2c9a572b5ee2b8ccdf843b320cd8..251c89bfebf36d2cf607105bc6c11278320a8880 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -124,13 +124,29 @@ Context *RefreshRequest<I>::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:
@@ -1306,8 +1322,14 @@ void RefreshRequest<I>::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;
       }
@@ -1318,7 +1340,7 @@ void RefreshRequest<I>::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;
     }
@@ -1422,7 +1444,13 @@ template <typename I>
 int RefreshRequest<I>::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;
@@ -1441,17 +1469,24 @@ int RefreshRequest<I>::get_parent_info(uint64_t snap_id,
 }
 
 template <typename I>
-bool RefreshRequest<I>::get_migration_info(ParentImageInfo *parent_md,
-                                           MigrationInfo *migration_info) {
+int RefreshRequest<I>::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;
@@ -1488,10 +1523,11 @@ bool RefreshRequest<I>::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 4e314c7e6980c635ef632c7172d5d7f72fa1c427..6970a9b45626cd8e62759335cc45add3372f8084 100644 (file)
--- 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 fe1ec95e99c7260b31f9675e9d9983a99023c1a2..6fd4783711b343299e4e3de06d01962388277622 100644 (file)
--- a/src/pybind/rbd/rbd.pyx
+++ b/src/pybind/rbd/rbd.pyx
@@ -278,6 +278,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
@@ -797,6 +798,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 73ae1483cb08d1a35cbbe02f802e0354d0c5d0ad..641418a6fce6ff0cea51dd4c5283c07b0a368f1b 100644 (file)
--- a/src/test/librbd/test_Migration.cc
+++ b/src/test/librbd/test_Migration.cc
@@ -1112,6 +1112,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 a1e9e4644822195a29bf2cb2cb794818f7e55417..db9b69bf6df1c2d8ae967185ab19ee8469e89f1b 100644 (file)
--- a/src/tools/rbd/action/Status.cc
+++ b/src/tools/rbd/action/Status.cc
@@ -119,6 +119,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";
       }