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 <dillaman@redhat.com>

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 57ea9597036e905009a5b15c9c4f06dc64ea101d..7b73921831e9b0275d37bf2debb31f4d036ea712 100644 (file)
--- 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 89ae38bd9196b3352bff65e116accb579db8e07b..a15fabe48821978b108fe32ba214c5d27f2f0329 100644 (file)
--- 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 d4d70420007aa17b7f95a1b058f706ea995c2ca1..041f287410efb84d1e59db5891daa6577792dc66 100644 (file)
--- a/src/librbd/ObjectMap.cc
+++ b/src/librbd/ObjectMap.cc
@@ -96,7 +96,8 @@ bool ObjectMap<I>::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<I>::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 9a4e7a51c97b60e847667efddf16d75c682ad47c..a5e3a8bd993c1ccfaef55daea63e435740529533 100644 (file)
--- 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 3bd1c2123db82be0f4765077b556cebdad83f4b9..dcb0661fbd1b2f3c77eda48ebe7d8fed8b8f1d7d 100644 (file)
--- a/src/librbd/operation/SnapshotRemoveRequest.cc
+++ b/src/librbd/operation/SnapshotRemoveRequest.cc
@@ -267,7 +267,6 @@ void SnapshotRemoveRequest<I>::release_snap_id() {
   ldout(cct, 5) << "snap_name=" << m_snap_name << ", "
                 << "snap_id=" << m_snap_id << dendl;
 
-
   auto aio_comp = create_rados_callback<
     SnapshotRemoveRequest<I>,
     &SnapshotRemoveRequest<I>::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 038725b748d6f044424b6927922ea4a939495361..f4b2ed43889a4edc1fb086ffce170cc2a75c9794 100644 (file)
--- 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 e7fff9482a36ccfed5b1b46f5b27955d632ebb02..19d89d10f030f588e5bed7ff2bc279b87c3e5593 100644 (file)
--- 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 0f14e855097a03d817e6257a45969583870f0466..1f9148faa07f865d849e730552869838678916d5 100644 (file)
--- 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 5dfb0171fb68e51c8d3c3028581fc930b076c9de..a42e201634faed9ef99d5cc3caaf698248a0869a 100644 (file)
--- 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 ef556df435a33fd0fc55ea0d9e017238772f9928..f5e34c860b2374e3be3a14bb9953560208e193cd 100644 (file)
--- 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 a9e3229b538dd5fe3281e2bbe0d7a87245bd400a..f59a8f452bbc474282a68199d86c4fabfc4cf35a 100644 (file)
--- 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);