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>
(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 1d7f932e9709ca706480c45eb8b33fdffa194b7c..0d2c9c75454023f8136e1481f655c5cb80d8cbd6 100644 (file)
--- 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<uint32_t>(migration_state) << ")";
     break;

diff --git a/src/cls/rbd/cls_rbd_types.h b/src/cls/rbd/cls_rbd_types.h
index bbfc85870b21b0519298fbe4f0f9d67d60774560..073006b3b99323cea8460b61a4cc85b528e82e45 100644 (file)
--- 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 3d27e394eee263256c58d2138c7f2fbc6d31b07b..522a6fb6d4827f5adffb5da46bbcd43ec463ad02 100644 (file)
--- 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 d885948cfba401d2efb5a2fb6bb8dadc2d1752cf..813eeeeca249f24e29fe57845b80b987cde6cf9e 100644 (file)
--- a/src/librbd/api/Migration.cc
+++ b/src/librbd/api/Migration.cc
@@ -777,8 +777,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;
@@ -786,12 +814,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 = snap_list(dst_image_ctx, snaps);
     if (r < 0) {
@@ -839,7 +861,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 ad3c1bb8643091fbd91c6921301b3eee0c1e5b45..3cc1b79ec3821c3fc1cd37bbda3ec8974a4441af 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -130,13 +130,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_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<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;
         }
@@ -1437,7 +1459,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;
@@ -1456,17 +1484,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;
@@ -1503,10 +1538,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 dc1d86b33cc4ab16f590c45836c3171ad7c0fe97..0a0f1e8d709c87957627bb1f5029c34e0ccae0b2 100644 (file)
--- 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 3ce8ed4463b9d659ef7ec391676fa003d3804c40..264c91fbc27ede18ee36c017c7a4db98cd6975db 100644 (file)
--- 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 d6c8fac62c0c798f1a2ec6be36e6ede07b4376e6..164a5ba9cfb58339572ab60358240c4c2463d524 100644 (file)
--- 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 75afdd7743117558a82457fbc8598e8cdc1b3b0b..0a599e7f0e04d1f8bb714165554b045c2e816d4c 100644 (file)
--- 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";
       }