From 862082792d9c2ff23823e46937b7de9a42830cfd Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 14 Sep 2018 09:59:35 -0400 Subject: [PATCH] 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 --- src/librbd/ImageCtx.cc | 10 ++++--- src/librbd/ImageCtx.h | 6 ++-- src/librbd/ObjectMap.cc | 6 ++-- src/librbd/object_map/Request.cc | 3 +- src/librbd/operation/SnapshotRemoveRequest.cc | 1 - .../object_map/test_mock_InvalidateRequest.cc | 6 ++-- .../test_mock_SnapshotRollbackRequest.cc | 6 ++-- src/test/librbd/test_DeepCopy.cc | 6 ++-- src/test/librbd/test_Migration.cc | 6 ++-- src/test/librbd/test_ObjectMap.cc | 29 ++++++++++++------- src/test/rbd_mirror/test_ImageSync.cc | 3 +- 11 files changed, 53 insertions(+), 29 deletions(-) diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 57ea9597036..7b73921831e 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -573,18 +573,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 { ceph_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 89ae38bd919..a15fabe4882 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -303,8 +303,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 d4d70420007..041f287410e 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; @@ -122,7 +123,8 @@ bool ObjectMap::object_may_not_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 9a4e7a51c97..a5e3a8bd993 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 3bd1c2123db..dcb0661fbd1 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 038725b748d..f4b2ed43889 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 e7fff9482a3..19d89d10f03 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 0f14e855097..1f9148faa07 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_Migration.cc b/src/test/librbd/test_Migration.cc index 5dfb0171fb6..a42e201634f 100644 --- a/src/test/librbd/test_Migration.cc +++ b/src/test/librbd/test_Migration.cc @@ -104,8 +104,10 @@ struct TestMigration : public TestFixture { if (dst_ictx->test_features(RBD_FEATURE_LAYERING)) { bool flags_set; - EXPECT_EQ(0, dst_ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID, - &flags_set)); + RWLock::RLocker dst_locker(dst_ictx->snap_lock); + EXPECT_EQ(0, dst_ictx->test_flags(dst_ictx->snap_id, + RBD_FLAG_OBJECT_MAP_INVALID, + 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 ef556df435a..f5e34c860b2 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 a9e3229b538..f59a8f452bb 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); -- 2.39.5