luminous: rbd: librbd: object map improperly flagged as invalidated

src/librbd/ImageCtx.cc

@@ -656,18 +656,20 @@ struct C_InvalidateCache : public Context {
     return -ENOENT;
   }
 
-  int ImageCtx::test_flags(uint64_t flags, bool *flags_set) const
+  int ImageCtx::test_flags(librados::snap_t in_snap_id,
+                           uint64_t flags, bool *flags_set) const
   {
     RWLock::RLocker l(snap_lock);
-    return test_flags(flags, snap_lock, flags_set);
+    return test_flags(in_snap_id, flags, snap_lock, flags_set);
   }
 
-  int ImageCtx::test_flags(uint64_t flags, const RWLock &in_snap_lock,
+  int ImageCtx::test_flags(librados::snap_t in_snap_id,
+                           uint64_t flags, const RWLock &in_snap_lock,
                            bool *flags_set) const
   {
     assert(snap_lock.is_locked());
     uint64_t snap_flags;
-    int r = get_flags(snap_id, &snap_flags);
+    int r = get_flags(in_snap_id, &snap_flags);
     if (r < 0) {
       return r;
     }

src/librbd/ImageCtx.h

@@ -274,8 +274,10 @@ namespace librbd {
     bool test_features(uint64_t test_features,
                        const RWLock &in_snap_lock) const;
     int get_flags(librados::snap_t in_snap_id, uint64_t *flags) const;
-    int test_flags(uint64_t test_flags, bool *flags_set) const;
-    int test_flags(uint64_t test_flags, const RWLock &in_snap_lock,
+    int test_flags(librados::snap_t in_snap_id,
+                   uint64_t test_flags, bool *flags_set) const;
+    int test_flags(librados::snap_t in_snap_id,
+                   uint64_t test_flags, const RWLock &in_snap_lock,
                    bool *flags_set) const;
     int update_flags(librados::snap_t in_snap_id, uint64_t flag, bool enabled);
 

src/librbd/ObjectMap.cc

@@ -96,7 +96,8 @@ bool ObjectMap<I>::object_may_exist(uint64_t object_no) const
   }
 
   bool flags_set;
-  int r = m_image_ctx.test_flags(RBD_FLAG_OBJECT_MAP_INVALID,
+  int r = m_image_ctx.test_flags(m_image_ctx.snap_id,
+                                 RBD_FLAG_OBJECT_MAP_INVALID,
                                  m_image_ctx.snap_lock, &flags_set);
   if (r < 0 || flags_set) {
     return true;
@@ -258,7 +259,7 @@ void ObjectMap<I>::detained_aio_update(UpdateOperation &&op) {
       handle_detained_aio_update(cell, r, on_finish);
     });
   aio_update(CEPH_NOSNAP, op.start_object_no, op.end_object_no, op.new_state,
-             op.current_state, op.parent_trace, ctx);
+             op.current_state, op.parent_trace, op.ignore_enoent, ctx);
 }
 
 template <typename I>
@@ -286,13 +287,14 @@ void ObjectMap<I>::aio_update(uint64_t snap_id, uint64_t start_object_no,
                               uint64_t end_object_no, uint8_t new_state,
                               const boost::optional<uint8_t> &current_state,
                               const ZTracer::Trace &parent_trace,
-                              Context *on_finish) {
+                              bool ignore_enoent, Context *on_finish) {
   assert(m_image_ctx.snap_lock.is_locked());
   assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0);
   assert(m_image_ctx.image_watcher != nullptr);
   assert(m_image_ctx.exclusive_lock == nullptr ||
          m_image_ctx.exclusive_lock->is_lock_owner());
-  assert(snap_id != CEPH_NOSNAP || m_image_ctx.object_map_lock.is_wlocked());
+  assert(snap_id != CEPH_NOSNAP ||
+         m_image_ctx.object_map_lock.is_wlocked());
   assert(start_object_no < end_object_no);
 
   CephContext *cct = m_image_ctx.cct;
@@ -324,7 +326,7 @@ void ObjectMap<I>::aio_update(uint64_t snap_id, uint64_t start_object_no,
 
   auto req = object_map::UpdateRequest<I>::create(
     m_image_ctx, &m_object_map, snap_id, start_object_no, end_object_no,
-    new_state, current_state, parent_trace, on_finish);
+    new_state, current_state, parent_trace, ignore_enoent, on_finish);
   req->send();
 }
 

src/librbd/ObjectMap.h

@@ -56,17 +56,19 @@ class ObjectMap {
   template <typename T, void(T::*MF)(int) = &T::complete>
   bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint8_t new_state,
                   const boost::optional<uint8_t> &current_state,
-                  const ZTracer::Trace &parent_trace, T *callback_object) {
+                  const ZTracer::Trace &parent_trace, bool ignore_enoent,
+                  T *callback_object) {
     return aio_update<T, MF>(snap_id, start_object_no, start_object_no + 1,
                              new_state, current_state, parent_trace,
-                             callback_object);
+                             ignore_enoent, callback_object);
   }
 
   template <typename T, void(T::*MF)(int) = &T::complete>
   bool aio_update(uint64_t snap_id, uint64_t start_object_no,
                   uint64_t end_object_no, uint8_t new_state,
                   const boost::optional<uint8_t> &current_state,
-                  const ZTracer::Trace &parent_trace, T *callback_object) {
+                  const ZTracer::Trace &parent_trace, bool ignore_enoent,
+                  T *callback_object) {
     assert(start_object_no < end_object_no);
     if (snap_id == CEPH_NOSNAP) {
       auto it = m_object_map.begin() + start_object_no;
@@ -83,12 +85,13 @@ class ObjectMap {
 
       UpdateOperation update_operation(start_object_no, end_object_no,
                                        new_state, current_state, parent_trace,
+                                       ignore_enoent,
                                        util::create_context_callback<T, MF>(
                                          callback_object));
       detained_aio_update(std::move(update_operation));
     } else {
       aio_update(snap_id, start_object_no, end_object_no, new_state,
-                 current_state, parent_trace,
+                 current_state, parent_trace, ignore_enoent,
                  util::create_context_callback<T, MF>(callback_object));
     }
     return true;
@@ -105,15 +108,18 @@ class ObjectMap {
     uint8_t new_state;
     boost::optional<uint8_t> current_state;
     ZTracer::Trace parent_trace;
+    bool ignore_enoent;
     Context *on_finish;
 
     UpdateOperation(uint64_t start_object_no, uint64_t end_object_no,
                     uint8_t new_state,
                     const boost::optional<uint8_t> &current_state,
-                    const ZTracer::Trace &parent_trace, Context *on_finish)
+                    const ZTracer::Trace &parent_trace,
+                    bool ignore_enoent, Context *on_finish)
       : start_object_no(start_object_no), end_object_no(end_object_no),
         new_state(new_state), current_state(current_state),
-        parent_trace(parent_trace), on_finish(on_finish) {
+        parent_trace(parent_trace), ignore_enoent(ignore_enoent),
+        on_finish(on_finish) {
     }
   };
 
@@ -132,7 +138,8 @@ class ObjectMap {
   void aio_update(uint64_t snap_id, uint64_t start_object_no,
                   uint64_t end_object_no, uint8_t new_state,
                   const boost::optional<uint8_t> &current_state,
-                  const ZTracer::Trace &parent_trace, Context *on_finish);
+                  const ZTracer::Trace &parent_trace, bool ignore_enoent,
+                  Context *on_finish);
   bool update_required(const ceph::BitVector<2>::Iterator &it,
                        uint8_t new_state);
 

src/librbd/io/CopyupRequest.cc

@@ -49,7 +49,7 @@ class UpdateObjectMap : public C_AsyncObjectThrottle<> {
       assert(m_image_ctx.exclusive_lock->is_lock_owner());
       assert(m_image_ctx.object_map != nullptr);
       bool sent = m_image_ctx.object_map->aio_update<Context>(
-        CEPH_NOSNAP, m_object_no, OBJECT_EXISTS, {}, m_trace, this);
+        CEPH_NOSNAP, m_object_no, OBJECT_EXISTS, {}, m_trace, false, this);
       return (sent ? 0 : 1);
     }
 
@@ -66,7 +66,7 @@ class UpdateObjectMap : public C_AsyncObjectThrottle<> {
     }
 
     bool sent = m_image_ctx.object_map->aio_update<Context>(
-      snap_id, m_object_no, state, {}, m_trace, this);
+      snap_id, m_object_no, state, {}, m_trace, true, this);
     assert(sent);
     return 0;
   }
@@ -346,7 +346,7 @@ bool CopyupRequest<I>::send_object_map_head() {
       if (may_update && (new_state != current_state) &&
           m_ictx->object_map->aio_update<CopyupRequest>(
             CEPH_NOSNAP, m_object_no, new_state, current_state, m_trace,
-            this)) {
+            false, this)) {
         return false;
       }
     }

src/librbd/io/ObjectRequest.cc

@@ -485,7 +485,8 @@ void AbstractObjectWriteRequest<I>::pre_write_object_map_update() {
   if (image_ctx->object_map->template aio_update<
         AbstractObjectWriteRequest<I>,
         &AbstractObjectWriteRequest<I>::handle_pre_write_object_map_update>(
-          CEPH_NOSNAP, this->m_object_no, new_state, {}, this->m_trace, this)) {
+          CEPH_NOSNAP, this->m_object_no, new_state, {}, this->m_trace, false,
+          this)) {
     image_ctx->object_map_lock.put_write();
     image_ctx->snap_lock.put_read();
     return;
@@ -627,7 +628,7 @@ void AbstractObjectWriteRequest<I>::post_write_object_map_update() {
         AbstractObjectWriteRequest<I>,
         &AbstractObjectWriteRequest<I>::handle_post_write_object_map_update>(
           CEPH_NOSNAP, this->m_object_no, OBJECT_NONEXISTENT, OBJECT_PENDING,
-          this->m_trace, this)) {
+          this->m_trace, false, this)) {
     image_ctx->object_map_lock.put_write();
     image_ctx->snap_lock.put_read();
     return;

src/librbd/object_map/Request.cc

@@ -49,7 +49,8 @@ bool Request::should_complete(int r) {
 
 bool Request::invalidate() {
   bool flags_set;
-  int r = m_image_ctx.test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set);
+  int r = m_image_ctx.test_flags(m_snap_id, RBD_FLAG_OBJECT_MAP_INVALID,
+                                 &flags_set);
   if (r == 0 && flags_set) {
     return true;
   }

src/librbd/object_map/SnapshotCreateRequest.cc

@@ -76,7 +76,6 @@ bool SnapshotCreateRequest::should_complete(int r) {
 
 void SnapshotCreateRequest::send_read_map() {
   assert(m_image_ctx.snap_lock.is_locked());
-  assert(m_image_ctx.get_snap_info(m_snap_id) != NULL);
 
   CephContext *cct = m_image_ctx.cct;
   std::string oid(ObjectMap<>::object_map_name(m_image_ctx.id, CEPH_NOSNAP));
@@ -135,7 +134,7 @@ bool SnapshotCreateRequest::send_add_snapshot() {
 void SnapshotCreateRequest::update_object_map() {
   RWLock::WLocker snap_locker(m_image_ctx.snap_lock);
   RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock);
-  
+
   auto it = m_object_map.begin();
   auto end_it = m_object_map.end();
   for (; it != end_it; ++it) {