From: Jason Dillaman Date: Fri, 14 Sep 2018 13:59:35 +0000 (-0400) Subject: librbd: test_flags helper should require snap id parameter X-Git-Tag: v13.2.3~137^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=645e18de350ea739418c7623bbce8669c91da70a;p=ceph.git librbd: test_flags helper should require snap id parameter The HEAD and snapshots have potentially different flag states since object maps get invalidated per revision. Signed-off-by: Jason Dillaman (cherry picked from commit 862082792d9c2ff23823e46937b7de9a42830cfd) Conflicts: src/librbd/ObjectMap.cc: trivial resolution src/test/librbd/test_Migration.cc: DNE --- diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 83a92ea94642..ec09e64b55af 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -544,18 +544,20 @@ public: 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; } diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index 1b2953b4bba0..819eda609b98 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -275,8 +275,10 @@ namespace librbd { bool test_op_features(uint64_t op_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); diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index 0a94f5a86e12..b91f42d3cb8b 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -96,7 +96,8 @@ bool ObjectMap::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; diff --git a/src/librbd/object_map/Request.cc b/src/librbd/object_map/Request.cc index 9a4e7a51c97b..a5e3a8bd993c 100644 --- a/src/librbd/object_map/Request.cc +++ b/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; } diff --git a/src/librbd/operation/SnapshotRemoveRequest.cc b/src/librbd/operation/SnapshotRemoveRequest.cc index 7dd256ea7877..49cca2a4bbfb 100644 --- a/src/librbd/operation/SnapshotRemoveRequest.cc +++ b/src/librbd/operation/SnapshotRemoveRequest.cc @@ -267,7 +267,6 @@ void SnapshotRemoveRequest::release_snap_id() { ldout(cct, 5) << "snap_name=" << m_snap_name << ", " << "snap_id=" << m_snap_id << dendl; - auto aio_comp = create_rados_callback< SnapshotRemoveRequest, &SnapshotRemoveRequest::handle_release_snap_id>(this); diff --git a/src/test/librbd/object_map/test_mock_InvalidateRequest.cc b/src/test/librbd/object_map/test_mock_InvalidateRequest.cc index 038725b748d6..f4b2ed43889a 100644 --- a/src/test/librbd/object_map/test_mock_InvalidateRequest.cc +++ b/src/test/librbd/object_map/test_mock_InvalidateRequest.cc @@ -28,7 +28,8 @@ TEST_F(TestMockObjectMapInvalidateRequest, UpdatesInMemoryFlag) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); bool flags_set; - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, + RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); ASSERT_FALSE(flags_set); C_SaferCond cond_ctx; @@ -45,7 +46,8 @@ TEST_F(TestMockObjectMapInvalidateRequest, UpdatesInMemoryFlag) { } ASSERT_EQ(0, cond_ctx.wait()); - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, + RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); ASSERT_TRUE(flags_set); } diff --git a/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc index e7fff9482a36..19d89d10f030 100644 --- a/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc +++ b/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc @@ -105,7 +105,8 @@ TEST_F(TestMockObjectMapSnapshotRollbackRequest, ReadMapError) { ASSERT_NE(0U, flags & RBD_FLAG_OBJECT_MAP_INVALID); } bool flags_set; - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, + RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); ASSERT_TRUE(flags_set); expect_unlock_exclusive_lock(*ictx); } @@ -136,7 +137,8 @@ TEST_F(TestMockObjectMapSnapshotRollbackRequest, WriteMapError) { ASSERT_EQ(0U, flags & RBD_FLAG_OBJECT_MAP_INVALID); } bool flags_set; - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, + RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); ASSERT_TRUE(flags_set); expect_unlock_exclusive_lock(*ictx); } diff --git a/src/test/librbd/test_DeepCopy.cc b/src/test/librbd/test_DeepCopy.cc index 0f14e855097a..1f9148faa07f 100644 --- a/src/test/librbd/test_DeepCopy.cc +++ b/src/test/librbd/test_DeepCopy.cc @@ -87,8 +87,10 @@ struct TestDeepCopy : public TestFixture { if (m_dst_ictx->test_features(RBD_FEATURE_LAYERING)) { bool flags_set; - EXPECT_EQ(0, m_dst_ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, - &flags_set)); + RWLock::RLocker dst_locker(m_dst_ictx->snap_lock); + EXPECT_EQ(0, m_dst_ictx->test_flags(m_dst_ictx->snap_id, + RBD_FLAG_OBJECT_MAP_INVALID, + m_dst_ictx->snap_lock, &flags_set)); EXPECT_FALSE(flags_set); } diff --git a/src/test/librbd/test_ObjectMap.cc b/src/test/librbd/test_ObjectMap.cc index ef556df435a3..f5e34c860b23 100644 --- a/src/test/librbd/test_ObjectMap.cc +++ b/src/test/librbd/test_ObjectMap.cc @@ -31,7 +31,8 @@ TEST_F(TestObjectMap, RefreshInvalidatesWhenCorrupt) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); bool flags_set; - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_FALSE(flags_set); C_SaferCond lock_ctx; @@ -47,7 +48,8 @@ TEST_F(TestObjectMap, RefreshInvalidatesWhenCorrupt) { ASSERT_EQ(0, ictx->md_ctx.write_full(oid, bl)); ASSERT_EQ(0, when_open_object_map(ictx)); - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_TRUE(flags_set); } @@ -57,7 +59,8 @@ TEST_F(TestObjectMap, RefreshInvalidatesWhenTooSmall) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); bool flags_set; - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_FALSE(flags_set); C_SaferCond lock_ctx; @@ -74,7 +77,8 @@ TEST_F(TestObjectMap, RefreshInvalidatesWhenTooSmall) { ASSERT_EQ(0, ictx->md_ctx.operate(oid, &op)); ASSERT_EQ(0, when_open_object_map(ictx)); - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_TRUE(flags_set); } @@ -84,7 +88,8 @@ TEST_F(TestObjectMap, InvalidateFlagOnDisk) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); bool flags_set; - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_FALSE(flags_set); C_SaferCond lock_ctx; @@ -100,11 +105,13 @@ TEST_F(TestObjectMap, InvalidateFlagOnDisk) { ASSERT_EQ(0, ictx->md_ctx.write_full(oid, bl)); ASSERT_EQ(0, when_open_object_map(ictx)); - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_TRUE(flags_set); ASSERT_EQ(0, open_image(m_image_name, &ictx)); - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_TRUE(flags_set); } @@ -114,7 +121,8 @@ TEST_F(TestObjectMap, AcquireLockInvalidatesWhenTooSmall) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); bool flags_set; - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_FALSE(flags_set); librados::ObjectWriteOperation op; @@ -130,12 +138,13 @@ TEST_F(TestObjectMap, AcquireLockInvalidatesWhenTooSmall) { } ASSERT_EQ(0, lock_ctx.wait()); - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, + &flags_set)); ASSERT_TRUE(flags_set); // Test the flag is stored on disk ASSERT_EQ(0, ictx->state->refresh()); - ASSERT_EQ(0, ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, + ASSERT_EQ(0, ictx->test_flags(CEPH_NOSNAP, RBD_FLAG_OBJECT_MAP_INVALID, &flags_set)); ASSERT_TRUE(flags_set); } diff --git a/src/test/rbd_mirror/test_ImageSync.cc b/src/test/rbd_mirror/test_ImageSync.cc index 5ef2cbe77a3c..4cb094c2fabd 100644 --- a/src/test/rbd_mirror/test_ImageSync.cc +++ b/src/test/rbd_mirror/test_ImageSync.cc @@ -319,7 +319,8 @@ TEST_F(TestImageSync, SnapshotStress) { local_size = m_local_image_ctx->get_image_size( m_local_image_ctx->snap_id); bool flags_set; - ASSERT_EQ(0, m_local_image_ctx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, + ASSERT_EQ(0, m_local_image_ctx->test_flags(m_local_image_ctx->snap_id, + RBD_FLAG_OBJECT_MAP_INVALID, m_local_image_ctx->snap_lock, &flags_set)); ASSERT_FALSE(flags_set);