rbd: interlock object-map/fast-diff features together

Since we only support Jewel and later releases, which both support
object-map and fast-diff, enabling/disabling object-map should always
enable/disable fast-diff.

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

diff --git a/qa/workunits/rbd/qemu_dynamic_features.sh b/qa/workunits/rbd/qemu_dynamic_features.sh
index 7045f5f09f55e81542fa62537ab5ead2b6f6582c..7ee5868d8094accdd594c90231386ab945ca274d 100755 (executable)
--- a/qa/workunits/rbd/qemu_dynamic_features.sh
+++ b/qa/workunits/rbd/qemu_dynamic_features.sh
@@ -28,7 +28,6 @@ while is_qemu_running ; do
   rbd feature enable ${IMAGE_NAME} exclusive-lock || break
   rbd feature enable ${IMAGE_NAME} journaling || break
   rbd feature enable ${IMAGE_NAME} object-map || break
-  rbd feature enable ${IMAGE_NAME} fast-diff || break
   if is_qemu_running ; then
     sleep 60
   fi

diff --git a/src/include/rbd/features.h b/src/include/rbd/features.h
index 074d744a7936edd2d6254802fcdb8b3be9758648..24ae5fcfc10fcad5951f130202a6dd09304762ac 100644 (file)
--- a/src/include/rbd/features.h
+++ b/src/include/rbd/features.h
@@ -71,6 +71,7 @@
 /// features that will be implicitly enabled
 #define RBD_FEATURES_IMPLICIT_ENABLE  (RBD_FEATURE_STRIPINGV2 | \
                                        RBD_FEATURE_DATA_POOL  | \
+                                       RBD_FEATURE_FAST_DIFF  | \
                                        RBD_FEATURE_OPERATIONS)
 
 /// features that cannot be controlled by the user

diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc
index 0e3ba2ac881d024070c493d4d8530dc0c55889a9..abd70c7063b1b35c088ce54ef9f1dc3f1967d260 100644 (file)
--- a/src/librbd/Operations.cc
+++ b/src/librbd/Operations.cc
@@ -1318,6 +1318,17 @@ int Operations<I>::update_features(uint64_t features, bool enabled) {
     lderr(cct) << "cannot update immutable features" << dendl;
     return -EINVAL;
   }
+
+  bool set_object_map = (features & RBD_FEATURE_OBJECT_MAP) == RBD_FEATURE_OBJECT_MAP;
+  bool set_fast_diff = (features & RBD_FEATURE_FAST_DIFF) == RBD_FEATURE_FAST_DIFF;
+  bool exist_fast_diff = (m_image_ctx.features & RBD_FEATURE_FAST_DIFF) != 0;
+  bool exist_object_map = (m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0;
+
+  if ((enabled && ((set_object_map && !exist_fast_diff) || (set_fast_diff && !exist_object_map)))
+      || (!enabled && (set_object_map && exist_fast_diff))) {
+    features |= (RBD_FEATURE_OBJECT_MAP | RBD_FEATURE_FAST_DIFF);
+  }
+
   if (features == 0) {
     lderr(cct) << "update requires at least one feature" << dendl;
     return -EINVAL;

diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc
index 695d320ce2e2f44e830e900e4baab3cd1b939322..1418ef38f3caaeb4a0bd32908c4351c7bba05e49 100644 (file)
--- a/src/librbd/image/CreateRequest.cc
+++ b/src/librbd/image/CreateRequest.cc
@@ -167,6 +167,10 @@ CreateRequest<I>::CreateRequest(IoCtx &ioctx, const std::string &image_name,
   m_features |= features_set;
   m_features &= ~features_clear;
 
+  if ((m_features & RBD_FEATURE_OBJECT_MAP) == RBD_FEATURE_OBJECT_MAP) {
+      m_features |= RBD_FEATURE_FAST_DIFF;
+  }
+
   if (image_options.get(RBD_IMAGE_OPTION_STRIPE_UNIT, &m_stripe_unit) != 0 ||
       m_stripe_unit == 0) {
     m_stripe_unit = m_cct->_conf->get_val<uint64_t>("rbd_default_stripe_unit");

diff --git a/src/librbd/operation/DisableFeaturesRequest.cc b/src/librbd/operation/DisableFeaturesRequest.cc
index a21efda115ff82f7856faa6be5cb14f8a090449a..3f1d338a4aed463da48e9c35275b0b7c650dbd31 100644 (file)
--- a/src/librbd/operation/DisableFeaturesRequest.cc
+++ b/src/librbd/operation/DisableFeaturesRequest.cc
@@ -193,12 +193,6 @@ Context *DisableFeaturesRequest<I>::handle_acquire_exclusive_lock(int *result) {
       m_disable_flags |= RBD_FLAG_FAST_DIFF_INVALID;
     }
     if ((m_features & RBD_FEATURE_OBJECT_MAP) != 0) {
-      if ((m_new_features & RBD_FEATURE_FAST_DIFF) != 0) {
-        lderr(cct) << "cannot disable object-map. fast-diff must be "
-                      "disabled before disabling object-map." << dendl;
-        *result = -EINVAL;
-        break;
-      }
       m_disable_flags |= RBD_FLAG_OBJECT_MAP_INVALID;
     }
   } while (false);

diff --git a/src/librbd/operation/EnableFeaturesRequest.cc b/src/librbd/operation/EnableFeaturesRequest.cc
index 0f7f7bbe29c0db6857f9fbc73b2a1d3df0b3d35b..b9d7420cd1adae869094f9a6c7db87af6503f22a 100644 (file)
--- a/src/librbd/operation/EnableFeaturesRequest.cc
+++ b/src/librbd/operation/EnableFeaturesRequest.cc
@@ -189,12 +189,6 @@ Context *EnableFeaturesRequest<I>::handle_get_mirror_mode(int *result) {
       m_features_mask |= RBD_FEATURE_EXCLUSIVE_LOCK;
     }
     if ((m_features & RBD_FEATURE_FAST_DIFF) != 0) {
-      if ((m_new_features & RBD_FEATURE_OBJECT_MAP) == 0) {
-       lderr(cct) << "cannot enable fast-diff. object-map must be "
-                      "enabled before enabling fast-diff." << dendl;
-       *result = -EINVAL;
-       break;
-      }
       m_enable_flags |= RBD_FLAG_FAST_DIFF_INVALID;
       m_features_mask |= (RBD_FEATURE_OBJECT_MAP | RBD_FEATURE_EXCLUSIVE_LOCK);
     }

diff --git a/src/test/cli/rbd/help.t b/src/test/cli/rbd/help.t
index c791ef12cb3bb93a6494cc5f8e67b2a1de888b62..387c03e8913a7a4b91f4f383e72d53a4049e9822 100644 (file)
--- a/src/test/cli/rbd/help.t
+++ b/src/test/cli/rbd/help.t
@@ -194,7 +194,7 @@
     --object-size arg         object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg       image features
                               [layering(+), exclusive-lock(+*), object-map(+*),
-                              fast-diff(+*), deep-flatten(+-), journaling(*)]
+                              deep-flatten(+-), journaling(*)]
     --image-shared            shared image
     --stripe-unit arg         stripe unit in B/K/M
     --stripe-count arg        stripe count
@@ -240,8 +240,7 @@
     --object-size arg            object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg          image features
                                  [layering(+), exclusive-lock(+*),
-                                 object-map(+*), fast-diff(+*), deep-flatten(+-),
-                                 journaling(*)]
+                                 object-map(+*), deep-flatten(+-), journaling(*)]
     --image-shared               shared image
     --stripe-unit arg            stripe unit in B/K/M
     --stripe-count arg           stripe count
@@ -286,7 +285,7 @@
     --object-size arg         object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg       image features
                               [layering(+), exclusive-lock(+*), object-map(+*),
-                              fast-diff(+*), deep-flatten(+-), journaling(*)]
+                              deep-flatten(+-), journaling(*)]
     --image-shared            shared image
     --stripe-unit arg         stripe unit in B/K/M
     --stripe-count arg        stripe count
@@ -335,8 +334,7 @@
     --object-size arg            object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg          image features
                                  [layering(+), exclusive-lock(+*),
-                                 object-map(+*), fast-diff(+*), deep-flatten(+-),
-                                 journaling(*)]
+                                 object-map(+*), deep-flatten(+-), journaling(*)]
     --image-shared               shared image
     --stripe-unit arg            stripe unit in B/K/M
     --stripe-count arg           stripe count
@@ -500,7 +498,7 @@
     <image-spec>         image specification
                          (example: [<pool-name>/]<image-name>)
     <features>           image features
-                         [exclusive-lock, object-map, fast-diff, journaling]
+                         [exclusive-lock, object-map, journaling]
   
   Optional arguments
     -p [ --pool ] arg    pool name
@@ -519,7 +517,7 @@
     <image-spec>              image specification
                               (example: [<pool-name>/]<image-name>)
     <features>                image features
-                              [exclusive-lock, object-map, fast-diff, journaling]
+                              [exclusive-lock, object-map, journaling]
   
   Optional arguments
     -p [ --pool ] arg         pool name
@@ -822,7 +820,7 @@
     --object-size arg         object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg       image features
                               [layering(+), exclusive-lock(+*), object-map(+*),
-                              fast-diff(+*), deep-flatten(+-), journaling(*)]
+                              deep-flatten(+-), journaling(*)]
     --image-shared            shared image
     --stripe-unit arg         stripe unit in B/K/M
     --stripe-count arg        stripe count

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index a18f67e5a9d88c78060749509910c29164414a74..d4e17b6d54506da60822cea731320d11cc2a6871 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -5317,13 +5317,14 @@ TEST_F(TestLibRBD, UpdateFeatures)
   ASSERT_EQ(0, image.features(&features));
   ASSERT_NE(0U, features & RBD_FEATURE_EXCLUSIVE_LOCK);
 
-  // cannot enable fast diff w/o object map
-  ASSERT_EQ(-EINVAL, image.update_features(RBD_FEATURE_FAST_DIFF, true));
-  ASSERT_EQ(0, image.update_features(RBD_FEATURE_OBJECT_MAP, true));
+  // can enable fast diff w/o object map
+  ASSERT_EQ(0, image.update_features(RBD_FEATURE_FAST_DIFF, true));
+  ASSERT_EQ(-EINVAL, image.update_features(RBD_FEATURE_OBJECT_MAP, true));
   ASSERT_EQ(0, image.features(&features));
   ASSERT_NE(0U, features & RBD_FEATURE_OBJECT_MAP);
 
-  uint64_t expected_flags = RBD_FLAG_OBJECT_MAP_INVALID;
+  uint64_t expected_flags = RBD_FLAG_OBJECT_MAP_INVALID |
+         RBD_FLAG_FAST_DIFF_INVALID;
   uint64_t flags;
   ASSERT_EQ(0, image.get_flags(&flags));
   ASSERT_EQ(expected_flags, flags);
@@ -5332,29 +5333,23 @@ TEST_F(TestLibRBD, UpdateFeatures)
   ASSERT_EQ(0, image.features(&features));
   ASSERT_EQ(0U, features & RBD_FEATURE_OBJECT_MAP);
 
-  ASSERT_EQ(0, image.update_features(RBD_FEATURE_OBJECT_MAP |
-                                     RBD_FEATURE_FAST_DIFF |
-                                     RBD_FEATURE_JOURNALING, true));
-
-  expected_flags = RBD_FLAG_OBJECT_MAP_INVALID | RBD_FLAG_FAST_DIFF_INVALID;
-  ASSERT_EQ(0, image.get_flags(&flags));
-  ASSERT_EQ(expected_flags, flags);
-
-  // cannot disable object map w/ fast diff
-  ASSERT_EQ(-EINVAL, image.update_features(RBD_FEATURE_OBJECT_MAP, false));
-  ASSERT_EQ(0, image.update_features(RBD_FEATURE_FAST_DIFF, false));
+  // can disable object map w/ fast diff
+  ASSERT_EQ(0, image.update_features(RBD_FEATURE_OBJECT_MAP, true));
+  ASSERT_EQ(0, image.update_features(RBD_FEATURE_OBJECT_MAP, false));
+  ASSERT_EQ(-EINVAL, image.update_features(RBD_FEATURE_FAST_DIFF, false));
   ASSERT_EQ(0, image.features(&features));
   ASSERT_EQ(0U, features & RBD_FEATURE_FAST_DIFF);
 
-  expected_flags = RBD_FLAG_OBJECT_MAP_INVALID;
   ASSERT_EQ(0, image.get_flags(&flags));
-  ASSERT_EQ(expected_flags, flags);
+  ASSERT_EQ(0U, flags);
 
   // cannot disable exclusive lock w/ object map
+  ASSERT_EQ(0, image.update_features(RBD_FEATURE_OBJECT_MAP, true));
   ASSERT_EQ(-EINVAL, image.update_features(RBD_FEATURE_EXCLUSIVE_LOCK, false));
   ASSERT_EQ(0, image.update_features(RBD_FEATURE_OBJECT_MAP, false));
 
   // cannot disable exclusive lock w/ journaling
+  ASSERT_EQ(0, image.update_features(RBD_FEATURE_JOURNALING, true));
   ASSERT_EQ(-EINVAL, image.update_features(RBD_FEATURE_EXCLUSIVE_LOCK, false));
   ASSERT_EQ(0, image.update_features(RBD_FEATURE_JOURNALING, false));