librbd: invalidate object map on error even w/o holding lock

If there is a deep bug that prevents the object map from owning
the lock before objects are updated, flag the object map as invalid
regardless.

Fixes: #13372
Backport: hammer
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc
index db17daa577d71b19602a124715b62bf9e6fa7426..d94780711d5308f4503e0b83aad55c14f9982e2d 100644 (file)
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -183,7 +183,7 @@ void ObjectMap::refresh(uint64_t snap_id)
   if (r == -EINVAL) {
     // object map is corrupt on-disk -- clear it and properly size it
     // so future IO can keep the object map in sync
-    invalidate(snap_id);
+    invalidate(snap_id, false);
 
     librados::ObjectWriteOperation op;
     if (snap_id == CEPH_NOSNAP) {
@@ -202,7 +202,7 @@ void ObjectMap::refresh(uint64_t snap_id)
   if (r < 0) {
     lderr(cct) << "error refreshing object map: " << cpp_strerror(r)
                << dendl;
-    invalidate(snap_id);
+    invalidate(snap_id, false);
     m_object_map.clear();
     return;
   }
@@ -213,7 +213,7 @@ void ObjectMap::refresh(uint64_t snap_id)
   if (m_object_map.size() < num_objs) {
     lderr(cct) << "object map smaller than current object count: "
                << m_object_map.size() << " != " << num_objs << dendl;
-    invalidate(snap_id);
+    invalidate(snap_id, false);
 
     // correct the size issue so future IO can keep the object map in sync
     librados::ObjectWriteOperation op;
@@ -262,7 +262,7 @@ void ObjectMap::rollback(uint64_t snap_id) {
   if (r < 0) {
     lderr(cct) << "unable to load snapshot object map '" << snap_oid << "': "
               << cpp_strerror(r) << dendl;
-    invalidate(snap_id);
+    invalidate(snap_id, false);
     return;
   }
 
@@ -274,7 +274,7 @@ void ObjectMap::rollback(uint64_t snap_id) {
   if (r < 0) {
     lderr(cct) << "unable to rollback object map: " << cpp_strerror(r)
               << dendl;
-    invalidate(CEPH_NOSNAP);
+    invalidate(CEPH_NOSNAP, true);
   }
 }
 
@@ -298,7 +298,7 @@ void ObjectMap::snapshot_add(uint64_t snap_id) {
   if (r < 0) {
     lderr(cct) << "unable to load object map: " << cpp_strerror(r)
               << dendl;
-    invalidate(CEPH_NOSNAP);
+    invalidate(CEPH_NOSNAP, false);
     return;
   }
 
@@ -307,7 +307,7 @@ void ObjectMap::snapshot_add(uint64_t snap_id) {
   if (r < 0) {
     lderr(cct) << "unable to snapshot object map '" << snap_oid << "': "
               << cpp_strerror(r) << dendl;
-    invalidate(snap_id);
+    invalidate(snap_id, false);
     return;
   }
 
@@ -319,7 +319,7 @@ void ObjectMap::snapshot_add(uint64_t snap_id) {
     if (r < 0) {
       lderr(cct) << "unable to snapshot object map: " << cpp_strerror(r)
                  << dendl;
-      invalidate(CEPH_NOSNAP);
+      invalidate(CEPH_NOSNAP, true);
       return;
     }
 
@@ -363,7 +363,7 @@ int ObjectMap::snapshot_remove(uint64_t snap_id) {
       uint64_t flags;
       m_image_ctx.get_flags(snap_id, &flags);
       if ((flags & RBD_FLAG_OBJECT_MAP_INVALID) != 0) {
-        invalidate(next_snap_id);
+        invalidate(next_snap_id, true);
         r = -EINVAL;
       }
     }
@@ -381,7 +381,7 @@ int ObjectMap::snapshot_remove(uint64_t snap_id) {
       if (r < 0) {
         lderr(cct) << "unable to remove object map snapshot: "
                    << cpp_strerror(r) << dendl;
-        invalidate(next_snap_id);
+        invalidate(next_snap_id, true);
       }
     }
 
@@ -496,7 +496,7 @@ void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no,
   req->send();
 }
 
-void ObjectMap::invalidate(uint64_t snap_id) {
+void ObjectMap::invalidate(uint64_t snap_id, bool force) {
   assert(m_image_ctx.snap_lock.is_wlocked());
   assert(m_image_ctx.object_map_lock.is_wlocked());
   uint64_t flags;
@@ -527,7 +527,7 @@ void ObjectMap::invalidate(uint64_t snap_id) {
   }
 
   librados::ObjectWriteOperation op;
-  if (snap_id == CEPH_NOSNAP) {
+  if (snap_id == CEPH_NOSNAP && !force) {
     m_image_ctx.image_watcher->assert_header_locked(&op);
   }
   cls_client::set_flags(&op, snap_id, flags, flags);
@@ -560,7 +560,7 @@ bool ObjectMap::Request::should_complete(int r) {
   case STATE_REQUEST:
     if (r == -EBUSY) {
       lderr(cct) << "object map lock not owned by client" << dendl;
-      return true;
+      return invalidate();
     } else if (r < 0) {
       lderr(cct) << "failed to update object map: " << cpp_strerror(r)
                 << dendl;
@@ -610,7 +610,6 @@ bool ObjectMap::Request::invalidate() {
   m_image_ctx.flags |= flags;
 
   librados::ObjectWriteOperation op;
-  m_image_ctx.image_watcher->assert_header_locked(&op);
   cls_client::set_flags(&op, CEPH_NOSNAP, flags, flags);
 
   librados::AioCompletion *rados_completion = create_callback_completion();

diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h
index 115023306944fa3371294cec0e94238cb9aaf5d9..797307f2820dd7bcc760e07d7ca6b484887f1aa8 100644 (file)
--- a/src/librbd/ObjectMap.h
+++ b/src/librbd/ObjectMap.h
@@ -143,7 +143,7 @@ private:
   uint64_t m_snap_id;
   bool m_enabled;
 
-  void invalidate(uint64_t snap_id);
+  void invalidate(uint64_t snap_id, bool force);
   void resize(uint64_t num_objs, uint8_t default_state);
 };