librbd: permit deep-flatten to be dynamically disabled

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc
index 706a5de531ffd09a278b4b1cce970648e6944d03..ee54c3b79693384c38caabd8bf567bcf2a00b237 100644 (file)
--- a/src/cls/rbd/cls_rbd.cc
+++ b/src/cls/rbd/cls_rbd.cc
@@ -380,12 +380,6 @@ int set_features(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return -EINVAL;
   }
 
-  if ((mask & RBD_FEATURES_MUTABLE) != mask) {
-    CLS_ERR("Attempting to set immutable feature: %" PRIu64,
-            mask & ~RBD_FEATURES_MUTABLE);
-    return -EINVAL;
-  }
-
   // check that features exists to make sure this is a header object
   // that was created correctly
   uint64_t orig_features = 0;
@@ -396,6 +390,22 @@ int set_features(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return r;
   }
 
+  uint64_t changed_features = (orig_features ^ features) & mask;
+  uint64_t enabled_features = changed_features & features;
+  if ((enabled_features & RBD_FEATURES_MUTABLE) != enabled_features) {
+    CLS_ERR("Attempting to enable immutable feature: %" PRIu64,
+            enabled_features & ~RBD_FEATURES_MUTABLE);
+    return -EINVAL;
+  }
+
+  uint64_t disabled_features = changed_features & orig_features;
+  uint64_t disable_mask = (RBD_FEATURES_MUTABLE | RBD_FEATURES_DISABLE_ONLY);
+  if ((disabled_features & disable_mask) != disabled_features) {
+       CLS_ERR("Attempting to disable immutable feature: %" PRIu64,
+               enabled_features & ~disable_mask);
+       return -EINVAL;
+  }
+
   features = (orig_features & ~mask) | (features & mask);
   CLS_LOG(10, "set_features features=%" PRIu64 " orig_features=%" PRIu64,
           features, orig_features);

diff --git a/src/include/rbd/features.h b/src/include/rbd/features.h
index a78e6fc67624c3cc994be04ebaa9193e94f6ac68..3e3509d4691af4e335c42cbd62341281a0f2867e 100644 (file)
--- a/src/include/rbd/features.h
+++ b/src/include/rbd/features.h
@@ -36,6 +36,9 @@
                                          RBD_FEATURE_FAST_DIFF      | \
                                          RBD_FEATURE_JOURNALING)
 
+/// features that may be dynamically disabled
+#define RBD_FEATURES_DISABLE_ONLY       (RBD_FEATURE_DEEP_FLATTEN)
+
 /// features that only work when used with a single client
 /// using the image for writes
 #define RBD_FEATURES_SINGLE_CLIENT (RBD_FEATURE_EXCLUSIVE_LOCK | \

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 1d76a2e5beacb69dbc030e8258638229395c14e8..905e91759606c95e6aa0df484ab088dbd74aa82c 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1317,7 +1317,10 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
         return r;
       }
 
-      if ((features & RBD_FEATURES_MUTABLE) != features) {
+      uint64_t disable_mask = (RBD_FEATURES_MUTABLE |
+                               RBD_FEATURES_DISABLE_ONLY);
+      if ((enabled && (features & RBD_FEATURES_MUTABLE) != features) ||
+          (!enabled && (features & disable_mask) != features)) {
         lderr(cct) << "cannot update immutable features" << dendl;
         return -EINVAL;
       } else if (features == 0) {
@@ -1390,7 +1393,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
             lderr(cct) << "cannot disable exclusive lock" << dendl;
             return -EINVAL;
           }
-          features_mask |= RBD_FEATURE_OBJECT_MAP;
+          features_mask |= (RBD_FEATURE_OBJECT_MAP |
+                            RBD_FEATURE_JOURNALING);
         }
         if ((features & RBD_FEATURE_OBJECT_MAP) != 0) {
           if ((new_features & RBD_FEATURE_FAST_DIFF) != 0) {

diff --git a/src/test/cli/rbd/help.t b/src/test/cli/rbd/help.t
index 800fc45a5f5c25005bf74496a4c2f95db0cf36c6..341bf5242a0195b8fe0cc403d632c8679c37e27b 100644 (file)
--- a/src/test/cli/rbd/help.t
+++ b/src/test/cli/rbd/help.t
@@ -149,8 +149,8 @@
     --order arg               object order [12 <= order <= 25]
     --object-size arg         object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg       image features
-                              [layering(+), striping(+), exclusive-lock(*),
-                              object-map(*), fast-diff(*), deep-flatten,
+                              [layering(+), striping, exclusive-lock(+*),
+                              object-map(+*), fast-diff(+*), deep-flatten(+-),
                               journaling(*)]
     --image-shared            shared image
     --stripe-unit arg         stripe unit
@@ -161,6 +161,7 @@
   
   Image Features:
     (*) supports enabling/disabling on existing images
+    (-) supports disabling-only on existing images
     (+) enabled by default for new images if features not specified
   
   rbd help copy
@@ -192,8 +193,8 @@
     --order arg                  object order [12 <= order <= 25]
     --object-size arg            object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg          image features
-                                 [layering(+), striping(+), exclusive-lock(*),
-                                 object-map(*), fast-diff(*), deep-flatten,
+                                 [layering(+), striping, exclusive-lock(+*),
+                                 object-map(+*), fast-diff(+*), deep-flatten(+-),
                                  journaling(*)]
     --image-shared               shared image
     --stripe-unit arg            stripe unit
@@ -205,6 +206,7 @@
   
   Image Features:
     (*) supports enabling/disabling on existing images
+    (-) supports disabling-only on existing images
     (+) enabled by default for new images if features not specified
   
   rbd help create
@@ -234,8 +236,8 @@
     --order arg               object order [12 <= order <= 25]
     --object-size arg         object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg       image features
-                              [layering(+), striping(+), exclusive-lock(*),
-                              object-map(*), fast-diff(*), deep-flatten,
+                              [layering(+), striping, exclusive-lock(+*),
+                              object-map(+*), fast-diff(+*), deep-flatten(+-),
                               journaling(*)]
     --image-shared            shared image
     --stripe-unit arg         stripe unit
@@ -247,6 +249,7 @@
   
   Image Features:
     (*) supports enabling/disabling on existing images
+    (-) supports disabling-only on existing images
     (+) enabled by default for new images if features not specified
   
   rbd help diff
@@ -479,8 +482,8 @@
     --order arg               object order [12 <= order <= 25]
     --object-size arg         object size in B/K/M [4K <= object size <= 32M]
     --image-feature arg       image features
-                              [layering(+), striping(+), exclusive-lock(*),
-                              object-map(*), fast-diff(*), deep-flatten,
+                              [layering(+), striping, exclusive-lock(+*),
+                              object-map(+*), fast-diff(+*), deep-flatten(+-),
                               journaling(*)]
     --image-shared            shared image
     --stripe-unit arg         stripe unit
@@ -494,6 +497,7 @@
   
   Image Features:
     (*) supports enabling/disabling on existing images
+    (-) supports disabling-only on existing images
     (+) enabled by default for new images if features not specified
   
   rbd help import-diff

diff --git a/src/test/cls_rbd/test_cls_rbd.cc b/src/test/cls_rbd/test_cls_rbd.cc
index 19d77255941bbb5f26af9645a5e6a66f8c8258a7..8c405685d8219b58c2acdc32fb0a4fe3e3c474a0 100644 (file)
--- a/src/test/cls_rbd/test_cls_rbd.cc
+++ b/src/test/cls_rbd/test_cls_rbd.cc
@@ -1259,7 +1259,8 @@ TEST_F(TestClsRbd, set_features)
   ASSERT_EQ(0, _rados.ioctx_create(_pool_name.c_str(), ioctx));
 
   string oid = get_temp_image_name();
-  ASSERT_EQ(0, create_image(&ioctx, oid, 0, 22, 0, oid));
+  uint64_t base_features = RBD_FEATURE_LAYERING | RBD_FEATURE_DEEP_FLATTEN;
+  ASSERT_EQ(0, create_image(&ioctx, oid, 0, 22, base_features, oid));
 
   uint64_t features = RBD_FEATURES_MUTABLE;
   uint64_t mask = RBD_FEATURES_MUTABLE;
@@ -1268,7 +1269,7 @@ TEST_F(TestClsRbd, set_features)
   uint64_t actual_features;
   ASSERT_EQ(0, get_features(&ioctx, oid, CEPH_NOSNAP, &actual_features));
 
-  uint64_t expected_features = RBD_FEATURES_MUTABLE;
+  uint64_t expected_features = RBD_FEATURES_MUTABLE | base_features;
   ASSERT_EQ(expected_features, actual_features);
 
   features = 0;
@@ -1277,11 +1278,15 @@ TEST_F(TestClsRbd, set_features)
 
   ASSERT_EQ(0, get_features(&ioctx, oid, CEPH_NOSNAP, &actual_features));
 
-  expected_features = RBD_FEATURES_MUTABLE & ~RBD_FEATURE_OBJECT_MAP;
+  expected_features = (RBD_FEATURES_MUTABLE | base_features) &
+                      ~RBD_FEATURE_OBJECT_MAP;
   ASSERT_EQ(expected_features, actual_features);
 
-  mask = RBD_FEATURE_LAYERING;
-  ASSERT_EQ(-EINVAL, set_features(&ioctx, oid, features, mask));
+  ASSERT_EQ(0, set_features(&ioctx, oid, 0, RBD_FEATURE_DEEP_FLATTEN));
+  ASSERT_EQ(-EINVAL, set_features(&ioctx, oid, RBD_FEATURE_DEEP_FLATTEN,
+                                  RBD_FEATURE_DEEP_FLATTEN));
+
+  ASSERT_EQ(-EINVAL, set_features(&ioctx, oid, 0, RBD_FEATURE_LAYERING));
 }
 
 TEST_F(TestClsRbd, mirror) {

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index a1589a4ffb991d003528e6b1d11f4ae50cb155a7..47c1cab30efb4127b3544b5eab7d1118561556ac 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -3520,6 +3520,9 @@ TEST_F(TestLibRBD, UpdateFeatures)
     return;
   }
 
+  uint64_t features;
+  ASSERT_EQ(0, image.features(&features));
+
   // must provide a single feature
   ASSERT_EQ(-EINVAL, image.update_features(0, true));
 
@@ -3565,6 +3568,11 @@ TEST_F(TestLibRBD, UpdateFeatures)
   ASSERT_EQ(0U, flags);
 
   ASSERT_EQ(0, image.update_features(RBD_FEATURE_EXCLUSIVE_LOCK, false));
+
+  if ((features & RBD_FEATURE_DEEP_FLATTEN) != 0) {
+    ASSERT_EQ(0, image.update_features(RBD_FEATURE_DEEP_FLATTEN, false));
+  }
+  ASSERT_EQ(-EINVAL, image.update_features(RBD_FEATURE_DEEP_FLATTEN, true));
 }
 
 TEST_F(TestLibRBD, RebuildObjectMap)

diff --git a/src/tools/rbd/ArgumentTypes.cc b/src/tools/rbd/ArgumentTypes.cc
index 698a643a7fc243074f9fa55bbaf5824879b6c7f8..fac5c7d34a414859dc888ba376d4628697722863 100644 (file)
--- a/src/tools/rbd/ArgumentTypes.cc
+++ b/src/tools/rbd/ArgumentTypes.cc
@@ -288,12 +288,14 @@ std::string get_short_features_help(bool append_suffix) {
 
     std::string suffix;
     if (append_suffix) {
-      if ((pair.first & RBD_FEATURES_MUTABLE) != 0) {
-        suffix += "*";
-      }
       if ((pair.first & g_conf->rbd_default_features) != 0) {
         suffix += "+";
       }
+      if ((pair.first & RBD_FEATURES_MUTABLE) != 0) {
+        suffix += "*";
+      } else if ((pair.first & RBD_FEATURES_DISABLE_ONLY) != 0) {
+        suffix += "-";
+      }
       if (!suffix.empty()) {
         suffix = "(" + suffix + ")";
       }
@@ -308,6 +310,7 @@ std::string get_long_features_help() {
   std::ostringstream oss;
   oss << "Image Features:" << std::endl
       << "  (*) supports enabling/disabling on existing images" << std::endl
+      << "  (-) supports disabling-only on existing images" << std::endl
       << "  (+) enabled by default for new images if features not specified"
       << std::endl;
   return oss.str();