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>
(cherry picked from commit a8a633396a4105991c9643c2b39391621934c26d)

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index a999f6f79b0d178018e9d8f50e5f6cdea0b7a243..07bc843537a18842e9451b85625b96084c4a5f15 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1719,17 +1719,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 c03ce647154e3bf81b28c59a5f94a17aac15411b..5101a7e4b2aa646a8b4715fe384e8f899b1d7cbb 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -3652,10 +3652,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 cd59dd711fbdc48c6c7a105ed1224d047b850761..fcd98ec767f0b976157643ea25b253021f676a69 100644 (file)
--- a/src/test/librbd/test_mirroring.cc
+++ b/src/test/librbd/test_mirroring.cc
@@ -464,7 +464,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);
 }
@@ -473,7 +473,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);
 }
@@ -482,7 +482,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);
 }