librbd: enable/disable of features is not allowed when already enabled/disabled

Fixes: http://tracker.ceph.com/issues/16079
Signed-off-by: Lu Shi <shi.lu@h3c.com>

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 13682df4182ab9a05d96130ea64d3fa32a740448..8ebe976f82efb0b375791312da2daca10881f135 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1726,17 +1726,21 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
       RWLock::WLocker snap_locker(ictx->snap_lock);
       uint64_t new_features;
       if (enabled) {
+       if ((features & ictx->features) != 0) {
+         lderr(cct) << "one or more requested features are already enabled" << dendl;
+         return -EINVAL;
+       }
         features &= ~ictx->features;
         new_features = ictx->features | features;
       } else {
+        if ((features & ~ictx->features) != 0) {
+         lderr(cct) << "one or more requested features are already disabled" << dendl;
+         return -EINVAL;
+        }
         features &= ictx->features;
         new_features = ictx->features & ~features;
       }
 
-      if (features == 0) {
-        return 0;
-      }
-
       bool enable_mirroring = false;
       uint64_t features_mask = features;
       uint64_t disable_flags = 0;

diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc
index 05034919b149fda74b6b428ca137c2b29dff4472..eff4b9dadad412e0106cc94c0da4dd10572ed7a0 100644 (file)
--- a/src/test/librbd/image/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/image/test_mock_RefreshRequest.cc
@@ -503,12 +503,22 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLock) {
     mock_image_ctx.journal = &mock_journal;
   }
 
-  ASSERT_EQ(0, update_features(ictx,
-                               RBD_FEATURE_EXCLUSIVE_LOCK |
-                               RBD_FEATURE_OBJECT_MAP |
-                               RBD_FEATURE_FAST_DIFF |
-                               RBD_FEATURE_JOURNALING, false));
+  if (ictx->test_features(RBD_FEATURE_JOURNALING)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING, false));
+  }
+
+  if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) {    
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_FAST_DIFF , false));
+  }
 
+  if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_OBJECT_MAP , false));
+  }
+  
+  if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) {       
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_EXCLUSIVE_LOCK , false));
+  }
+  
   expect_op_work_queue(mock_image_ctx);
   expect_test_features(mock_image_ctx);
 
@@ -539,11 +549,21 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLockWhileAcquiringLock) {
   MockExclusiveLock mock_exclusive_lock;
   mock_image_ctx.exclusive_lock = &mock_exclusive_lock;
 
-  ASSERT_EQ(0, update_features(ictx,
-                               RBD_FEATURE_EXCLUSIVE_LOCK |
-                               RBD_FEATURE_OBJECT_MAP |
-                               RBD_FEATURE_FAST_DIFF |
-                               RBD_FEATURE_JOURNALING, false));
+  if (ictx->test_features(RBD_FEATURE_JOURNALING)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING, false));
+  }
+
+  if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) {    
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_FAST_DIFF , false));
+  }
+
+  if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_OBJECT_MAP , false));
+  }
+  
+  if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) {       
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_EXCLUSIVE_LOCK , false));
+  }
 
   expect_op_work_queue(mock_image_ctx);
   expect_test_features(mock_image_ctx);
@@ -567,9 +587,13 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
-  ASSERT_EQ(0, update_features(ictx,
-                               RBD_FEATURE_OBJECT_MAP |
-                               RBD_FEATURE_FAST_DIFF, false));
+  if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) {    
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_FAST_DIFF , false));
+  }
+
+  if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_OBJECT_MAP , false));
+  }
 
   MockRefreshImageCtx mock_image_ctx(*ictx);
   MockRefreshParentRequest mock_refresh_parent_request;
@@ -603,9 +627,13 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithoutExclusiveLock) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
-  ASSERT_EQ(0, update_features(ictx,
-                               RBD_FEATURE_OBJECT_MAP |
-                               RBD_FEATURE_FAST_DIFF, false));
+  if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) {    
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_FAST_DIFF , false));
+  }
+
+  if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_OBJECT_MAP , false));
+  }
 
   MockRefreshImageCtx mock_image_ctx(*ictx);
   MockRefreshParentRequest mock_refresh_parent_request;
@@ -651,7 +679,9 @@ TEST_F(TestMockImageRefreshRequest, DisableJournal) {
   MockJournal *mock_journal = new MockJournal();
   mock_image_ctx.journal = mock_journal;
 
-  ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING, false));
+  if (ictx->test_features(RBD_FEATURE_JOURNALING)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING , false));
+  }
 
   expect_op_work_queue(mock_image_ctx);
   expect_test_features(mock_image_ctx);
@@ -679,7 +709,9 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithExclusiveLock) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
-  ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING, false));
+  if (ictx->test_features(RBD_FEATURE_JOURNALING)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING , false));
+  }
 
   MockRefreshImageCtx mock_image_ctx(*ictx);
   MockRefreshParentRequest mock_refresh_parent_request;
@@ -713,7 +745,9 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithoutExclusiveLock) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
-  ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING, false));
+  if (ictx->test_features(RBD_FEATURE_JOURNALING)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING , false));
+  }
 
   MockRefreshImageCtx mock_image_ctx(*ictx);
   MockRefreshParentRequest mock_refresh_parent_request;
@@ -758,9 +792,13 @@ TEST_F(TestMockImageRefreshRequest, DisableObjectMap) {
     mock_image_ctx.journal = &mock_journal;
   }
 
-  ASSERT_EQ(0, update_features(ictx,
-                               RBD_FEATURE_OBJECT_MAP |
-                               RBD_FEATURE_FAST_DIFF, false));
+  if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) {    
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_FAST_DIFF , false));
+  }
+
+  if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_OBJECT_MAP , false));
+  }
 
   expect_op_work_queue(mock_image_ctx);
   expect_test_features(mock_image_ctx);
@@ -785,7 +823,9 @@ TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
-  ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING, false));
+  if (ictx->test_features(RBD_FEATURE_JOURNALING)) {   
+    ASSERT_EQ(0, update_features(ictx, RBD_FEATURE_JOURNALING , false));
+  }
 
   MockRefreshImageCtx mock_image_ctx(*ictx);
   MockRefreshParentRequest mock_refresh_parent_request;

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index 551d266196003390f074e13c90282009eca44d8e..2f1f783d408635587621b488f692af0917280f55 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -3511,10 +3511,14 @@ TEST_F(TestLibRBD, UpdateFeatures)
   // must provide a single feature
   ASSERT_EQ(-EINVAL, image.update_features(0, true));
 
-  ASSERT_EQ(0, image.update_features(RBD_FEATURE_EXCLUSIVE_LOCK |
-                                     RBD_FEATURE_OBJECT_MAP |
-                                     RBD_FEATURE_FAST_DIFF |
-                                     RBD_FEATURE_JOURNALING, false));
+  uint64_t disable_features;
+  disable_features = features & (RBD_FEATURE_EXCLUSIVE_LOCK |
+                                 RBD_FEATURE_OBJECT_MAP |
+                                 RBD_FEATURE_FAST_DIFF |
+                                 RBD_FEATURE_JOURNALING);
+  if (disable_features != 0) {
+    ASSERT_EQ(0, image.update_features(disable_features, false));
+  }
 
   // cannot enable object map nor journaling w/o exclusive lock
   ASSERT_EQ(-EINVAL, image.update_features(RBD_FEATURE_OBJECT_MAP, true));

diff --git a/src/test/librbd/test_mirroring.cc b/src/test/librbd/test_mirroring.cc
index 758fc456fdb34b59cab205cf5643256df33d75bf..912f098985b2e619a9eb840c3249e63c660a9738 100644 (file)
--- a/src/test/librbd/test_mirroring.cc
+++ b/src/test/librbd/test_mirroring.cc
@@ -463,7 +463,7 @@ TEST_F(TestMirroring, EnableJournaling_In_MirrorModeDisabled) {
   uint64_t init_features = 0;
   init_features |= RBD_FEATURE_OBJECT_MAP;
   init_features |= RBD_FEATURE_EXCLUSIVE_LOCK;
-  uint64_t features = init_features | RBD_FEATURE_JOURNALING;
+  uint64_t features = RBD_FEATURE_JOURNALING;
   check_mirroring_on_update_features(init_features, true, false, features, 0,
                       RBD_MIRROR_MODE_DISABLED, RBD_MIRROR_IMAGE_DISABLED);
 }
@@ -472,7 +472,7 @@ TEST_F(TestMirroring, EnableJournaling_In_MirrorModeImage) {
   uint64_t init_features = 0;
   init_features |= RBD_FEATURE_OBJECT_MAP;
   init_features |= RBD_FEATURE_EXCLUSIVE_LOCK;
-  uint64_t features = init_features | RBD_FEATURE_JOURNALING;
+  uint64_t features = RBD_FEATURE_JOURNALING;
   check_mirroring_on_update_features(init_features, true, false, features, 0,
                       RBD_MIRROR_MODE_IMAGE, RBD_MIRROR_IMAGE_DISABLED);
 }
@@ -481,7 +481,7 @@ TEST_F(TestMirroring, EnableJournaling_In_MirrorModePool) {
   uint64_t init_features = 0;
   init_features |= RBD_FEATURE_OBJECT_MAP;
   init_features |= RBD_FEATURE_EXCLUSIVE_LOCK;
-  uint64_t features = init_features | RBD_FEATURE_JOURNALING;
+  uint64_t features = RBD_FEATURE_JOURNALING;
   check_mirroring_on_update_features(init_features, true, false, features, 0,
                       RBD_MIRROR_MODE_POOL, RBD_MIRROR_IMAGE_ENABLED);
 }