From e675400df7f8ba796d60e9bac4234857f0cb1392 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 6 Oct 2015 12:31:59 -0400 Subject: [PATCH] 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 --- src/librbd/ObjectMap.cc | 27 +++++++++++++-------------- src/librbd/ObjectMap.h | 2 +- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index db17daa577d71..d94780711d530 100644 --- 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 115023306944f..797307f2820dd 100644 --- 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); }; -- 2.39.5