librbd/object_map: decouple object map processing in DiffRequest

In preparation for potentially using in-memory object map, decouple
object map processing from loading object maps and place the logic in
prepare_for_object_map() and process_object_map().

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit dabb677ba5923f347c5b4b81b6a86214699a52bf)

diff --git a/src/librbd/object_map/DiffRequest.cc b/src/librbd/object_map/DiffRequest.cc
index abb5353b0c1867cb675b0fafecfdfcdb1f859805..7da6152058b0ec1f521fbdf2af80246612c89885 100644 (file)
--- a/src/librbd/object_map/DiffRequest.cc
+++ b/src/librbd/object_map/DiffRequest.cc
@@ -44,88 +44,17 @@ bool DiffRequest<I>::is_diff_iterate() const {
 }
 
 template <typename I>
-void DiffRequest<I>::send() {
-  auto cct = m_image_ctx->cct;
-
-  if (m_snap_id_start == CEPH_NOSNAP || m_snap_id_start > m_snap_id_end) {
-    lderr(cct) << "invalid start/end snap ids: "
-               << "snap_id_start=" << m_snap_id_start << ", "
-               << "snap_id_end=" << m_snap_id_end << dendl;
-    finish(-EINVAL);
-    return;
-  }
-  if (m_start_object_no == UINT64_MAX || m_start_object_no > m_end_object_no ||
-      (m_start_object_no != 0 && m_end_object_no == UINT64_MAX)) {
-    lderr(cct) << "invalid start/end object numbers: "
-               << "start_object_no=" << m_start_object_no << ", "
-               << "end_object_no=" << m_end_object_no << dendl;
-    finish(-EINVAL);
-    return;
-  }
-
-  m_object_diff_state->clear();
-
-  if (m_snap_id_start == m_snap_id_end) {
-    // no delta between the same snapshot
-    finish(0);
-    return;
-  }
-  if (m_start_object_no == m_end_object_no) {
-    // no objects in the provided range (half-open)
-    finish(0);
-    return;
-  }
-
-  // collect all the snap ids in the provided range (inclusive) unless
-  // this is diff-iterate against the beginning of time, in which case
-  // only the end version matters
-  std::shared_lock image_locker{m_image_ctx->image_lock};
-  if (!is_diff_iterate() || m_snap_id_start != 0) {
-    if (m_snap_id_start != 0) {
-      m_snap_ids.insert(m_snap_id_start);
-    }
-    auto snap_info_it = m_image_ctx->snap_info.upper_bound(m_snap_id_start);
-    auto snap_info_it_end = m_image_ctx->snap_info.lower_bound(m_snap_id_end);
-    for (; snap_info_it != snap_info_it_end; ++snap_info_it) {
-      m_snap_ids.insert(snap_info_it->first);
-    }
-  }
-  m_snap_ids.insert(m_snap_id_end);
-
-  load_object_map(&image_locker);
-}
-
-template <typename I>
-void DiffRequest<I>::load_object_map(
-    std::shared_lock<ceph::shared_mutex>* image_locker) {
+int DiffRequest<I>::prepare_for_object_map() {
   ceph_assert(ceph_mutex_is_locked(m_image_ctx->image_lock));
 
-  if (m_snap_ids.empty()) {
-    image_locker->unlock();
-
-    finish(0);
-    return;
-  }
-
-  m_current_snap_id = *m_snap_ids.begin();
-  m_snap_ids.erase(m_current_snap_id);
-
   auto cct = m_image_ctx->cct;
-  ldout(cct, 10) << "snap_id=" << m_current_snap_id << dendl;
+  ldout(cct, 20) << "snap_id=" << m_current_snap_id << dendl;
 
   if ((m_image_ctx->features & RBD_FEATURE_FAST_DIFF) == 0) {
-    image_locker->unlock();
-
     ldout(cct, 10) << "fast-diff feature not enabled" << dendl;
-    finish(-EINVAL);
-    return;
+    return -EINVAL;
   }
 
-  // ignore ENOENT with intermediate snapshots since deleted
-  // snaps will get merged with later snapshots
-  m_ignore_enoent = (m_current_snap_id != m_snap_id_start &&
-                     m_current_snap_id != m_snap_id_end);
-
   if (m_current_snap_id == CEPH_NOSNAP) {
     m_current_size = m_image_ctx->size;
   } else {
@@ -133,86 +62,35 @@ void DiffRequest<I>::load_object_map(
     if (snap_it == m_image_ctx->snap_info.end()) {
       ldout(cct, 10) << "snapshot " << m_current_snap_id << " does not exist"
                      << dendl;
-      if (!m_ignore_enoent) {
-        image_locker->unlock();
-
-        finish(-ENOENT);
-        return;
-      }
-
-      load_object_map(image_locker);
-      return;
+      return -ENOENT;
     }
-
     m_current_size = snap_it->second.size;
   }
 
-  uint64_t flags = 0;
+  uint64_t flags;
   int r = m_image_ctx->get_flags(m_current_snap_id, &flags);
-  if (r < 0) {
-    image_locker->unlock();
-
-    lderr(cct) << "failed to retrieve image flags: " << cpp_strerror(r)
-               << dendl;
-    finish(r);
-    return;
-  }
-  image_locker->unlock();
+  ceph_assert(r == 0);
 
   if ((flags & RBD_FLAG_FAST_DIFF_INVALID) != 0) {
     ldout(cct, 1) << "cannot perform fast diff on invalid object map"
                   << dendl;
-    finish(-EINVAL);
-    return;
+    return -EINVAL;
   }
 
-  std::string oid(ObjectMap<>::object_map_name(m_image_ctx->id,
-                                               m_current_snap_id));
-
-  librados::ObjectReadOperation op;
-  cls_client::object_map_load_start(&op);
-
-  m_out_bl.clear();
-  auto aio_comp = create_rados_callback<
-    DiffRequest<I>, &DiffRequest<I>::handle_load_object_map>(this);
-  r = m_image_ctx->md_ctx.aio_operate(oid, aio_comp, &op, &m_out_bl);
-  ceph_assert(r == 0);
-  aio_comp->release();
+  return 0;
 }
 
 template <typename I>
-void DiffRequest<I>::handle_load_object_map(int r) {
+int DiffRequest<I>::process_object_map(const BitVector<2>& object_map) {
   auto cct = m_image_ctx->cct;
-  ldout(cct, 10) << "r=" << r << dendl;
-
-  BitVector<2> object_map;
-  std::string oid(ObjectMap<>::object_map_name(m_image_ctx->id,
-                                               m_current_snap_id));
-
-  if (r == 0) {
-    auto bl_it = m_out_bl.cbegin();
-    r = cls_client::object_map_load_finish(&bl_it, &object_map);
-  }
-  if (r == -ENOENT && m_ignore_enoent) {
-    ldout(cct, 10) << "object map " << oid << " does not exist" << dendl;
-
-    std::shared_lock image_locker{m_image_ctx->image_lock};
-    load_object_map(&image_locker);
-    return;
-  } else if (r < 0) {
-    lderr(cct) << "failed to load object map: " << oid << dendl;
-    finish(r);
-    return;
-  }
-  ldout(cct, 20) << "loaded object map " << oid << dendl;
+  ldout(cct, 20) << "snap_id=" << m_current_snap_id << dendl;
 
   uint64_t num_objs = Striper::get_num_objects(m_image_ctx->layout,
                                                m_current_size);
   if (object_map.size() < num_objs) {
     ldout(cct, 1) << "object map too small: "
                   << object_map.size() << " < " << num_objs << dendl;
-    finish(-EINVAL);
-    return;
+    return -EINVAL;
   }
 
   uint64_t start_object_no, end_object_no;
@@ -326,6 +204,137 @@ void DiffRequest<I>::handle_load_object_map(int r) {
   ldout(cct, 20) << "computed resize diffs" << dendl;
 
   ceph_assert(diff_it == m_object_diff_state->end());
+  return 0;
+}
+
+template <typename I>
+void DiffRequest<I>::send() {
+  auto cct = m_image_ctx->cct;
+
+  if (m_snap_id_start == CEPH_NOSNAP || m_snap_id_start > m_snap_id_end) {
+    lderr(cct) << "invalid start/end snap ids: "
+               << "snap_id_start=" << m_snap_id_start << ", "
+               << "snap_id_end=" << m_snap_id_end << dendl;
+    finish(-EINVAL);
+    return;
+  }
+  if (m_start_object_no == UINT64_MAX || m_start_object_no > m_end_object_no ||
+      (m_start_object_no != 0 && m_end_object_no == UINT64_MAX)) {
+    lderr(cct) << "invalid start/end object numbers: "
+               << "start_object_no=" << m_start_object_no << ", "
+               << "end_object_no=" << m_end_object_no << dendl;
+    finish(-EINVAL);
+    return;
+  }
+
+  m_object_diff_state->clear();
+
+  if (m_snap_id_start == m_snap_id_end) {
+    // no delta between the same snapshot
+    finish(0);
+    return;
+  }
+  if (m_start_object_no == m_end_object_no) {
+    // no objects in the provided range (half-open)
+    finish(0);
+    return;
+  }
+
+  // collect all the snap ids in the provided range (inclusive) unless
+  // this is diff-iterate against the beginning of time, in which case
+  // only the end version matters
+  std::shared_lock image_locker{m_image_ctx->image_lock};
+  if (!is_diff_iterate() || m_snap_id_start != 0) {
+    if (m_snap_id_start != 0) {
+      m_snap_ids.insert(m_snap_id_start);
+    }
+    auto snap_info_it = m_image_ctx->snap_info.upper_bound(m_snap_id_start);
+    auto snap_info_it_end = m_image_ctx->snap_info.lower_bound(m_snap_id_end);
+    for (; snap_info_it != snap_info_it_end; ++snap_info_it) {
+      m_snap_ids.insert(snap_info_it->first);
+    }
+  }
+  m_snap_ids.insert(m_snap_id_end);
+
+  load_object_map(&image_locker);
+}
+
+template <typename I>
+void DiffRequest<I>::load_object_map(
+    std::shared_lock<ceph::shared_mutex>* image_locker) {
+  ceph_assert(ceph_mutex_is_locked(m_image_ctx->image_lock));
+
+  if (m_snap_ids.empty()) {
+    image_locker->unlock();
+
+    finish(0);
+    return;
+  }
+
+  m_current_snap_id = *m_snap_ids.begin();
+  m_snap_ids.erase(m_current_snap_id);
+
+  auto cct = m_image_ctx->cct;
+  ldout(cct, 10) << "snap_id=" << m_current_snap_id << dendl;
+
+  // ignore ENOENT with intermediate snapshots since deleted
+  // snaps will get merged with later snapshots
+  m_ignore_enoent = (m_current_snap_id != m_snap_id_start &&
+                     m_current_snap_id != m_snap_id_end);
+
+  int r = prepare_for_object_map();
+  if (r == -ENOENT && m_ignore_enoent) {
+    load_object_map(image_locker);
+    return;
+  } else if (r < 0) {
+    image_locker->unlock();
+
+    finish(r);
+    return;
+  }
+  image_locker->unlock();
+
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx->id,
+                                               m_current_snap_id));
+
+  librados::ObjectReadOperation op;
+  cls_client::object_map_load_start(&op);
+
+  m_out_bl.clear();
+  auto aio_comp = create_rados_callback<
+    DiffRequest<I>, &DiffRequest<I>::handle_load_object_map>(this);
+  r = m_image_ctx->md_ctx.aio_operate(oid, aio_comp, &op, &m_out_bl);
+  ceph_assert(r == 0);
+  aio_comp->release();
+}
+
+template <typename I>
+void DiffRequest<I>::handle_load_object_map(int r) {
+  auto cct = m_image_ctx->cct;
+  ldout(cct, 10) << "r=" << r << dendl;
+
+  BitVector<2> object_map;
+  std::string oid(ObjectMap<>::object_map_name(m_image_ctx->id,
+                                               m_current_snap_id));
+
+  if (r == 0) {
+    auto bl_it = m_out_bl.cbegin();
+    r = cls_client::object_map_load_finish(&bl_it, &object_map);
+  }
+  if (r == -ENOENT && m_ignore_enoent) {
+    ldout(cct, 10) << "object map " << oid << " does not exist" << dendl;
+  } else if (r < 0) {
+    lderr(cct) << "failed to load object map: " << oid << dendl;
+    finish(r);
+    return;
+  } else {
+    r = process_object_map(object_map);
+    if (r < 0) {
+      finish(r);
+      return;
+    }
+  }
+
   std::shared_lock image_locker{m_image_ctx->image_lock};
   load_object_map(&image_locker);
 }

diff --git a/src/librbd/object_map/DiffRequest.h b/src/librbd/object_map/DiffRequest.h
index 84211c6fc4a4842fc296720ae260b465cc205044..740f4e02a11742b279fb12c3a5eb5ed9041907fc 100644 (file)
--- a/src/librbd/object_map/DiffRequest.h
+++ b/src/librbd/object_map/DiffRequest.h
@@ -72,6 +72,9 @@ private:
 
   bool is_diff_iterate() const;
 
+  int prepare_for_object_map();
+  int process_object_map(const BitVector<2>& object_map);
+
   void load_object_map(std::shared_lock<ceph::shared_mutex>* image_locker);
   void handle_load_object_map(int r);