librbd/migration: don't instantiate NativeFormat, handle it via dispatch

Trying to shoehorn NativeFormat under FormatInterface doesn't really
work.  It fundamentally doesn't fit in:

- Unlike for RawFormat and QCOWFormat, src_image_ctx for NativeFormat
  is not dummy -- it's an ImageCtx for a real RBD image.  Pre-creating
  it in OpenSourceImageRequest with the expectation that placeholder
  values would be overridden later forces NativeFormat to reach into
  ImageCtx guts, duplicating the logic in the constructor.  This also
  necessitates calling snap_set() in a separate step, since snap_id
  isn't known at the time ImageCtx is created.

- Unlike for RawFormat and QCOWFormat, get_image_size() and
  get_snapshots() implementations for NativeFormat are dummy.

- read() and list_snaps() implementations for NativeFormat are
  inconsistent: read() passes through io::ImageDispatch layer, but
  list_snaps() doesn't.  Both can be passing through, meaning that in
  essence these are also dummy.

All of this is with today's code.  Additional complications arise with
planned support for migrating from external clusters where src_image_ctx
would require more invasive patching to "move" to an IoCtx belonging to
an external cluster's CephContext and also with other work.

With the above in mind, NativeFormat actually consists of:

1. Code that parses the "type: native" source spec
2. Code that patches ImageCtx, working around the fact that it's
   pre-created in OpenSourceImageRequest
3. A bunch of dummy implementations for FormatInterface

With this change, (1) is wrapped into a static method that also creates
ImageCtx after all required parameters are known and (2) and (3) go away
entirely.  NativeFormat no longer implements FormatInterface and doesn't
get instantiated at all.

Signed-off-by: Ilya Dryomov <[email protected]>

Author: idryomov

src/librbd/migration/ImageDispatch.cc

@@ -43,6 +43,12 @@ bool ImageDispatch<I>::read(
   auto cct = m_image_ctx->cct;
   ldout(cct, 20) << dendl;
 
+  // let io::ImageDispatch layer (IMAGE_DISPATCH_LAYER_CORE) handle
+  // native format
+  if (!m_format) {
+    return false;
+  }
+
   *dispatch_result = io::DISPATCH_RESULT_COMPLETE;
   return m_format->read(aio_comp, io_context->get_read_snap(),
                         std::move(image_extents), std::move(read_result),
@@ -132,6 +138,12 @@ bool ImageDispatch<I>::list_snaps(
   auto cct = m_image_ctx->cct;
   ldout(cct, 20) << dendl;
 
+  // let io::ImageDispatch layer (IMAGE_DISPATCH_LAYER_CORE) handle
+  // native format
+  if (!m_format) {
+    return false;
+  }
+
   *dispatch_result = io::DISPATCH_RESULT_COMPLETE;
 
   aio_comp->set_request_count(1);

src/librbd/migration/ImageDispatch.h

@@ -86,6 +86,8 @@ class ImageDispatch : public io::ImageDispatchInterface {
 
 private:
   ImageCtxT* m_image_ctx;
+
+  // empty (nullptr) for native format
   std::unique_ptr<FormatInterface> m_format;
 
   void fail_io(int r, io::AioCompletion* aio_comp,

src/librbd/migration/NativeFormat.cc

@@ -16,8 +16,8 @@
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
-#define dout_prefix *_dout << "librbd::migration::NativeFormat: " << this \
-                           << " " << __func__ << ": "
+#define dout_prefix *_dout << "librbd::migration::NativeFormat: " << __func__ \
+                           << ": "
 
 namespace librbd {
 namespace migration {
@@ -51,45 +51,47 @@ std::string NativeFormat<I>::build_source_spec(
 }
 
 template <typename I>
-NativeFormat<I>::NativeFormat(
-    I* image_ctx, const json_spirit::mObject& json_object, bool import_only)
-  : m_image_ctx(image_ctx), m_json_object(json_object),
-    m_import_only(import_only) {
+bool NativeFormat<I>::is_source_spec(
+    const json_spirit::mObject& source_spec_object) {
+  auto it = source_spec_object.find(TYPE_KEY);
+  return it != source_spec_object.end() &&
+         it->second.type() == json_spirit::str_type &&
+         it->second.get_str() == "native";
 }
 
 template <typename I>
-void NativeFormat<I>::open(Context* on_finish) {
-  auto cct = m_image_ctx->cct;
-  ldout(cct, 10) << dendl;
-
+int NativeFormat<I>::create_image_ctx(
+    librados::IoCtx& dst_io_ctx,
+    const json_spirit::mObject& source_spec_object,
+    bool import_only, uint64_t src_snap_id, I** src_image_ctx) {
+  auto cct = reinterpret_cast<CephContext*>(dst_io_ctx.cct());
   int64_t pool_id = -1;
   std::string pool_namespace;
   std::string image_name;
   std::string image_id;
+  std::string snap_name;
+  uint64_t snap_id = CEPH_NOSNAP;
 
-  if (auto it = m_json_object.find(POOL_NAME_KEY);
-      it != m_json_object.end()) {
+  if (auto it = source_spec_object.find(POOL_NAME_KEY);
+      it != source_spec_object.end()) {
     if (it->second.type() == json_spirit::str_type) {
-      librados::Rados rados(m_image_ctx->md_ctx);
+      librados::Rados rados(dst_io_ctx);
       pool_id = rados.pool_lookup(it->second.get_str().c_str());
       if (pool_id < 0) {
         lderr(cct) << "failed to lookup pool" << dendl;
-        on_finish->complete(static_cast<int>(pool_id));
-        return;
+        return static_cast<int>(pool_id);
       }
     } else {
       lderr(cct) << "invalid pool name" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
   }
 
-  if (auto it = m_json_object.find(POOL_ID_KEY);
-      it != m_json_object.end()) {
+  if (auto it = source_spec_object.find(POOL_ID_KEY);
+      it != source_spec_object.end()) {
     if (pool_id != -1) {
       lderr(cct) << "cannot specify both pool name and pool id" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
     if (it->second.type() == json_spirit::int_type) {
       pool_id = it->second.get_int64();
@@ -101,232 +103,107 @@ void NativeFormat<I>::open(Context* on_finish) {
     }
     if (pool_id == -1) {
       lderr(cct) << "invalid pool id" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
   }
 
   if (pool_id == -1) {
     lderr(cct) << "missing pool name or pool id" << dendl;
-    on_finish->complete(-EINVAL);
-    return;
+    return -EINVAL;
   }
 
-  if (auto it = m_json_object.find(POOL_NAMESPACE_KEY);
-      it != m_json_object.end()) {
+  if (auto it = source_spec_object.find(POOL_NAMESPACE_KEY);
+      it != source_spec_object.end()) {
     if (it->second.type() == json_spirit::str_type) {
       pool_namespace = it->second.get_str();
     } else {
       lderr(cct) << "invalid pool namespace" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
   }
 
-  if (auto it = m_json_object.find(IMAGE_NAME_KEY);
-      it != m_json_object.end()) {
+  if (auto it = source_spec_object.find(IMAGE_NAME_KEY);
+      it != source_spec_object.end()) {
     if (it->second.type() == json_spirit::str_type) {
       image_name = it->second.get_str();
     } else {
       lderr(cct) << "invalid image name" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
   } else {
     lderr(cct) << "missing image name" << dendl;
-    on_finish->complete(-EINVAL);
-    return;
+    return -EINVAL;
   }
 
-  if (auto it = m_json_object.find(IMAGE_ID_KEY);
-      it != m_json_object.end()) {
+  if (auto it = source_spec_object.find(IMAGE_ID_KEY);
+      it != source_spec_object.end()) {
     if (it->second.type() == json_spirit::str_type) {
       image_id = it->second.get_str();
     } else {
       lderr(cct) << "invalid image id" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
   }
 
-  if (auto it = m_json_object.find(SNAP_NAME_KEY);
-      it != m_json_object.end()) {
+  if (auto it = source_spec_object.find(SNAP_NAME_KEY);
+      it != source_spec_object.end()) {
     if (it->second.type() == json_spirit::str_type) {
-      m_snap_name = it->second.get_str();
+      snap_name = it->second.get_str();
     } else {
       lderr(cct) << "invalid snap name" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
   }
 
-  if (auto it = m_json_object.find(SNAP_ID_KEY);
-      it != m_json_object.end()) {
-    if (!m_snap_name.empty()) {
+  if (auto it = source_spec_object.find(SNAP_ID_KEY);
+      it != source_spec_object.end()) {
+    if (!snap_name.empty()) {
       lderr(cct) << "cannot specify both snap name and snap id" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
     if (it->second.type() == json_spirit::int_type) {
-      m_snap_id = it->second.get_uint64();
+      snap_id = it->second.get_uint64();
     } else if (it->second.type() == json_spirit::str_type) {
       try {
-        m_snap_id = boost::lexical_cast<uint64_t>(it->second.get_str());
+        snap_id = boost::lexical_cast<uint64_t>(it->second.get_str());
       } catch (boost::bad_lexical_cast&) {
       }
     }
-    if (m_snap_id == CEPH_NOSNAP) {
+    if (snap_id == CEPH_NOSNAP) {
       lderr(cct) << "invalid snap id" << dendl;
-      on_finish->complete(-EINVAL);
-      return;
+      return -EINVAL;
     }
   }
 
   // snapshot is required for import to keep source read-only
-  if (m_import_only && m_snap_name.empty() && m_snap_id == CEPH_NOSNAP) {
+  if (import_only && snap_name.empty() && snap_id == CEPH_NOSNAP) {
     lderr(cct) << "snapshot required for import" << dendl;
-    on_finish->complete(-EINVAL);
-    return;
-  }
-
-  // TODO add support for external clusters
-  librados::IoCtx io_ctx;
-  int r = util::create_ioctx(m_image_ctx->md_ctx, "source image",
-                             pool_id, pool_namespace, &io_ctx);
-  if (r < 0) {
-    on_finish->complete(r);
-    return;
-  }
-
-  m_image_ctx->md_ctx.dup(io_ctx);
-  m_image_ctx->data_ctx.dup(io_ctx);
-  m_image_ctx->name = image_name;
-
-  uint64_t flags = 0;
-  if (image_id.empty() && !m_import_only) {
-    flags |= OPEN_FLAG_OLD_FORMAT;
-  } else {
-    m_image_ctx->id = image_id;
+    return -EINVAL;
   }
 
-  if (m_image_ctx->child != nullptr) {
-    // set rados flags for reading the parent image
-    if (m_image_ctx->child->config.template get_val<bool>("rbd_balance_parent_reads")) {
-      m_image_ctx->set_read_flag(librados::OPERATION_BALANCE_READS);
-    } else if (m_image_ctx->child->config.template get_val<bool>("rbd_localize_parent_reads")) {
-      m_image_ctx->set_read_flag(librados::OPERATION_LOCALIZE_READS);
-    }
+  // import snapshot is used only for destination image HEAD
+  // otherwise, src_snap_id corresponds to destination image "opened at"
+  // snap_id
+  if (src_snap_id != CEPH_NOSNAP) {
+    snap_id = src_snap_id;
   }
 
-  // open the source RBD image
-  on_finish = new LambdaContext([this, on_finish](int r) {
-    handle_open(r, on_finish); });
-  m_image_ctx->state->open(flags, on_finish);
-}
-
-template <typename I>
-void NativeFormat<I>::handle_open(int r, Context* on_finish) {
-  auto cct = m_image_ctx->cct;
-  ldout(cct, 10) << "r=" << r << dendl;
-
+  // TODO add support for external clusters
+  librados::IoCtx src_io_ctx;
+  int r = util::create_ioctx(dst_io_ctx, "source image", pool_id,
+                             pool_namespace, &src_io_ctx);
   if (r < 0) {
-    lderr(cct) << "failed to open image: " << cpp_strerror(r) << dendl;
-    on_finish->complete(r);
-    return;
-  }
-
-  if (m_snap_id == CEPH_NOSNAP && m_snap_name.empty()) {
-    on_finish->complete(0);
-    return;
-  }
-
-  if (!m_snap_name.empty()) {
-    std::shared_lock image_locker{m_image_ctx->image_lock};
-    m_snap_id = m_image_ctx->get_snap_id(cls::rbd::UserSnapshotNamespace{},
-                                         m_snap_name);
-  }
-
-  if (m_snap_id == CEPH_NOSNAP) {
-    lderr(cct) << "failed to locate snapshot " << m_snap_name << dendl;
-    on_finish = new LambdaContext([on_finish](int) {
-      on_finish->complete(-ENOENT); });
-    m_image_ctx->state->close(on_finish);
-    return;
+    return r;
   }
 
-  on_finish = new LambdaContext([this, on_finish](int r) {
-    handle_snap_set(r, on_finish); });
-  m_image_ctx->state->snap_set(m_snap_id, on_finish);
-}
-
-template <typename I>
-void NativeFormat<I>::handle_snap_set(int r, Context* on_finish) {
-  auto cct = m_image_ctx->cct;
-  ldout(cct, 10) << "r=" << r << dendl;
-
-  if (r < 0) {
-    lderr(cct) << "failed to set snapshot " << m_snap_id << ": "
-               << cpp_strerror(r) << dendl;
-    on_finish = new LambdaContext([r, on_finish](int) {
-      on_finish->complete(r); });
-    m_image_ctx->state->close(on_finish);
-    return;
+  if (!snap_name.empty() && snap_id == CEPH_NOSNAP) {
+    *src_image_ctx = I::create(image_name, image_id, snap_name.c_str(),
+                               src_io_ctx, true);
+  } else {
+    *src_image_ctx = I::create(image_name, image_id, snap_id, src_io_ctx,
+                               true);
   }
-
-  on_finish->complete(0);
-}
-
-template <typename I>
-void NativeFormat<I>::close(Context* on_finish) {
-  auto cct = m_image_ctx->cct;
-  ldout(cct, 10) << dendl;
-
-  // the native librbd::image::CloseRequest handles all cleanup
-  on_finish->complete(0);
-}
-
-template <typename I>
-void NativeFormat<I>::get_snapshots(SnapInfos* snap_infos, Context* on_finish) {
-  auto cct = m_image_ctx->cct;
-  ldout(cct, 10) << dendl;
-
-  m_image_ctx->image_lock.lock_shared();
-  *snap_infos = m_image_ctx->snap_info;
-  m_image_ctx->image_lock.unlock_shared();
-
-  on_finish->complete(0);
-}
-
-template <typename I>
-void NativeFormat<I>::get_image_size(uint64_t snap_id, uint64_t* size,
-                                     Context* on_finish) {
-  auto cct = m_image_ctx->cct;
-  ldout(cct, 10) << dendl;
-
-  m_image_ctx->image_lock.lock_shared();
-  *size = m_image_ctx->get_image_size(snap_id);
-  m_image_ctx->image_lock.unlock_shared();
-
-
-  on_finish->complete(0);
-}
-
-template <typename I>
-void NativeFormat<I>::list_snaps(io::Extents&& image_extents,
-                                 io::SnapIds&& snap_ids, int list_snaps_flags,
-                                 io::SnapshotDelta* snapshot_delta,
-                                 const ZTracer::Trace &parent_trace,
-                                 Context* on_finish) {
-  auto cct = m_image_ctx->cct;
-  ldout(cct, 20) << "image_extents=" << image_extents << dendl;
-
-  auto aio_comp = io::AioCompletion::create_and_start(
-    on_finish, util::get_image_ctx(m_image_ctx), io::AIO_TYPE_GENERIC);
-  auto req = io::ImageDispatchSpec::create_list_snaps(
-    *m_image_ctx, io::IMAGE_DISPATCH_LAYER_MIGRATION, aio_comp,
-    std::move(image_extents), io::ImageArea::DATA, std::move(snap_ids),
-    list_snaps_flags, snapshot_delta, {});
-  req->send();
+  return 0;
 }
 
 } // namespace migration