From b7bb92d729505555201925f0d30280c6b1fdfc45 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 (cherry picked from commit 862082792d9c2ff23823e46937b7de9a42830cfd) Conflicts: src/librbd/ObjectMap.cc: trivial resolution src/librbd/operation/SnapshotRemoveRequest.cc: trivial resolution src/test/librbd/test_DeepCopy.cc: DNE src/test/librbd/test_Migration.cc: DNE --- src/librbd/ImageCtx.cc | 10 ++++--- src/librbd/ImageCtx.h | 6 ++-- src/librbd/ObjectMap.cc | 3 +- src/librbd/object_map/Request.cc | 3 +- .../object_map/test_mock_InvalidateRequest.cc | 6 ++-- .../test_mock_SnapshotRollbackRequest.cc | 6 ++-- src/test/librbd/test_ObjectMap.cc | 29 ++++++++++++------- src/test/rbd_mirror/test_ImageSync.cc | 3 +- 8 files changed, 43 insertions(+), 23 deletions(-) diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index e92a17702012..8f52c9609d33 100644 --- a/src/librbd/ImageCtx.cc +++ b/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; } diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index dc77af6d48d0..a78a71f67dec 100644 --- a/src/librbd/ImageCtx.h +++ b/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); diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index cfc15df629bc..ed2c9ecbca0c 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 82aaa7d199d6..4da6ba0dfff7 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/test/librbd/object_map/test_mock_InvalidateRequest.cc b/src/test/librbd/object_map/test_mock_InvalidateRequest.cc index e43129f19250..ce00e8d6f410 100644 --- a/src/test/librbd/object_map/test_mock_InvalidateRequest.cc +++ b/src/test/librbd/object_map/test_mock_InvalidateRequest.cc @@ -27,7 +27,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; @@ -44,7 +45,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_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 d0401ef0d605..89b223fb591e 100644 --- a/src/test/rbd_mirror/test_ImageSync.cc +++ b/src/test/rbd_mirror/test_ImageSync.cc @@ -296,7 +296,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.47.3