From ed8397595785825d42b3a4ed0f3c4083ad29be0f Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 29 Feb 2016 13:16:42 -0500 Subject: [PATCH] librbd: permit deep-flatten to be dynamically disabled Signed-off-by: Jason Dillaman --- src/cls/rbd/cls_rbd.cc | 22 ++++++++++++++++------ src/include/rbd/features.h | 3 +++ src/librbd/internal.cc | 8 ++++++-- src/test/cli/rbd/help.t | 20 ++++++++++++-------- src/test/cls_rbd/test_cls_rbd.cc | 15 ++++++++++----- src/test/librbd/test_librbd.cc | 8 ++++++++ src/tools/rbd/ArgumentTypes.cc | 9 ++++++--- 7 files changed, 61 insertions(+), 24 deletions(-) diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc index 706a5de531ffd..ee54c3b796933 100644 --- 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 a78e6fc67624c..3e3509d4691af 100644 --- 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 1d76a2e5beacb..905e91759606c 100644 --- 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 800fc45a5f5c2..341bf5242a019 100644 --- 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 19d77255941bb..8c405685d8219 100644 --- 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 a1589a4ffb991..47c1cab30efb4 100644 --- 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 698a643a7fc24..fac5c7d34a414 100644 --- 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(); -- 2.39.5