librbd: default features should be negotiated with the OSD

Derived from f066ce8f80bce929edc209590efd47cce2196ae2

Signed-off-by: Mykola Golub <mgolub@mirantis.com>
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 9fecb1e1688f0b8acbcb282553ffd5c1d413f03a..04bfcb1a672c2f42dc4d89097b21aa491e4de405 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1053,7 +1053,8 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
                uint64_t stripe_count, uint8_t journal_order,
                 uint8_t journal_splay_width, const std::string &journal_pool,
                 const std::string &non_primary_global_image_id,
-                const std::string &primary_mirror_uuid)
+                const std::string &primary_mirror_uuid,
+                bool negotiate_features)
   {
     ostringstream bid_ss;
     uint32_t extra;
@@ -1104,6 +1105,19 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
       goto err_remove_id;
     }
 
+    if (negotiate_features) {
+      uint64_t all_features = 0;
+      r = cls_client::get_all_features(&io_ctx, RBD_DIRECTORY, &all_features);
+      if (r < 0) {
+       ldout(cct, 10) << "error retrieving server supported features set: "
+                      << cpp_strerror(r) << dendl;
+      } else if ((features & all_features) != features) {
+       features &= all_features;
+       ldout(cct, 10) << "limiting default features set to server supported: "
+                      << features << dendl;
+      }
+    }
+
     oss << RBD_DATA_PREFIX << id;
     header_oid = util::header_name(id);
     r = cls_client::create_image(&io_ctx, header_oid, size, order,
@@ -1243,10 +1257,19 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
   int create(librados::IoCtx& io_ctx, const char *imgname, uint64_t size,
             int *order)
   {
-    CephContext *cct = (CephContext *)io_ctx.cct();
-    bool old_format = cct->_conf->rbd_default_format == 1;
-    uint64_t features = old_format ? 0 : cct->_conf->rbd_default_features;
-    return create(io_ctx, imgname, size, old_format, features, order, 0, 0);
+    uint64_t order_ = *order;
+    ImageOptions opts;
+
+    int r = opts.set(RBD_IMAGE_OPTION_ORDER, order_);
+    assert(r == 0);
+
+    r = create(io_ctx, imgname, size, opts, "", "");
+
+    int r1 = opts.get(RBD_IMAGE_OPTION_ORDER, &order_);
+    assert(r1 == 0);
+    *order = order_;
+
+    return r;
   }
 
   int create(IoCtx& io_ctx, const char *imgname, uint64_t size,
@@ -1294,10 +1317,13 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
     opts.get(RBD_IMAGE_OPTION_FORMAT, &format);
     bool old_format = format == 1;
 
-    uint64_t features;
-    if (opts.get(RBD_IMAGE_OPTION_FEATURES, &features) != 0) {
-      features = old_format ? 0 : cct->_conf->rbd_default_features;
+    uint64_t features = 0;
+    bool negotiate_features = false;
+    if (!old_format && opts.get(RBD_IMAGE_OPTION_FEATURES, &features) != 0) {
+      features = cct->_conf->rbd_default_features;
+      negotiate_features = true;
     }
+
     uint64_t stripe_unit = 0;
     uint64_t stripe_count = 0;
     opts.get(RBD_IMAGE_OPTION_STRIPE_UNIT, &stripe_unit);
@@ -1380,7 +1406,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
       r = create_v2(io_ctx, imgname, bid, size, order, features, stripe_unit,
                    stripe_count, journal_order, journal_splay_width,
                     journal_pool, non_primary_global_image_id,
-                    primary_mirror_uuid);
+                    primary_mirror_uuid, negotiate_features);
     }
 
     int r1 = opts.set(RBD_IMAGE_OPTION_ORDER, order);
@@ -1479,6 +1505,13 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
        return -ENOSYS;
       }
       use_p_features = false;
+    } else {
+      // inherit default features -- ensure layering is enabled
+      if ((cct->_conf->rbd_default_features & RBD_FEATURE_LAYERING) !=
+            RBD_FEATURE_LAYERING) {
+       lderr(cct) << "clone image must support layering" << dendl;
+       return -EINVAL;
+      }
     }
 
     // make sure child doesn't already exist, in either format

diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index c08ad9f69c268d34bdcbe56903b346654dbcdfea..274b02d610377e32bb4de647477588737924fd03 100644 (file)
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -112,7 +112,8 @@ namespace librbd {
                 uint8_t journal_splay_width,
                 const std::string &journal_pool,
                 const std::string &non_primary_global_image_id,
-                const std::string &primary_mirror_uuid);
+                const std::string &primary_mirror_uuid,
+                bool negotiate_features);
   int clone(IoCtx& p_ioctx, const char *p_name, const char *p_snap_name,
            IoCtx& c_ioctx, const char *c_name,
            uint64_t features, int *c_order,

diff --git a/src/tools/rbd/Utils.cc b/src/tools/rbd/Utils.cc
index 618d257a463ae160f00ddff94bbfa96905e5d808..dcf0829f2e47e726de385862ae6122f0fbd61b9a 100644 (file)
--- a/src/tools/rbd/Utils.cc
+++ b/src/tools/rbd/Utils.cc
@@ -370,9 +370,11 @@ int get_image_options(const boost::program_options::variables_map &vm,
     order = g_conf->rbd_default_order;
   }
 
+  bool features_set = false;
   if (vm.count(at::IMAGE_FEATURES)) {
     features = vm[at::IMAGE_FEATURES].as<uint64_t>();
     features_specified = true;
+    features_set = true;
   } else {
     features = g_conf->rbd_default_features;
   }
@@ -404,16 +406,20 @@ int get_image_options(const boost::program_options::variables_map &vm,
       return -EINVAL;
     }
     features |= RBD_FEATURE_STRIPINGV2;
-  } else {
-    if (features_specified && ((features & RBD_FEATURE_STRIPINGV2) != 0)) {
+    features_set = true;
+  } else if ((features & RBD_FEATURE_STRIPINGV2) != 0) {
+    if (features_specified) {
       std::cerr << "must specify both of stripe-unit and stripe-count when specify striping features" << std::endl;
       return -EINVAL;
     }
+    
     features &= ~RBD_FEATURE_STRIPINGV2;
+    features_set = true;
   }
 
   if (vm.count(at::IMAGE_SHARED) && vm[at::IMAGE_SHARED].as<bool>()) {
     features &= ~RBD_FEATURES_SINGLE_CLIENT;
+    features_set = true;
   }
 
   if (get_format) {
@@ -464,7 +470,9 @@ int get_image_options(const boost::program_options::variables_map &vm,
   }
 
   opts->set(RBD_IMAGE_OPTION_ORDER, order);
-  opts->set(RBD_IMAGE_OPTION_FEATURES, features);
+  if (features_set) {
+    opts->set(RBD_IMAGE_OPTION_FEATURES, features);
+  }
   opts->set(RBD_IMAGE_OPTION_STRIPE_UNIT, stripe_unit);
   opts->set(RBD_IMAGE_OPTION_STRIPE_COUNT, stripe_count);
 

diff --git a/src/tools/rbd/action/Clone.cc b/src/tools/rbd/action/Clone.cc
index f399d58dd83a34218f6e598d99fff6ce35dd5b10..ad8856c76336767c40a792c3b6c6bb068353b113 100644 (file)
--- a/src/tools/rbd/action/Clone.cc
+++ b/src/tools/rbd/action/Clone.cc
@@ -15,21 +15,6 @@ namespace clone {
 namespace at = argument_types;
 namespace po = boost::program_options;
 
-int do_clone(librbd::RBD &rbd, librados::IoCtx &p_ioctx,
-             const char *p_name, const char *p_snapname,
-             librados::IoCtx &c_ioctx, const char *c_name,
-             librbd::ImageOptions& opts) {
-  uint64_t features;
-  int r = opts.get(RBD_IMAGE_OPTION_FEATURES, &features);
-  assert(r == 0);
-
-  if ((features & RBD_FEATURE_LAYERING) != RBD_FEATURE_LAYERING) {
-    return -EINVAL;
-  }
-
-  return rbd.clone3(p_ioctx, p_name, p_snapname, c_ioctx, c_name, opts);
-}
-
 void get_arguments(po::options_description *positional,
                    po::options_description *options) {
   at::add_snap_spec_options(positional, options, at::ARGUMENT_MODIFIER_SOURCE);
@@ -79,9 +64,18 @@ int execute(const po::variables_map &vm) {
     return r;
   }
 
+  uint64_t features;
+  r = opts.get(RBD_IMAGE_OPTION_FEATURES, &features);
+  if (r != -ENOENT) {
+    if ((features & RBD_FEATURE_LAYERING) != RBD_FEATURE_LAYERING) {
+      std::cerr << "rbd: clone image must support layering" << std::endl;
+      return -EINVAL;
+    }
+  }
+
   librbd::RBD rbd;
-  r = do_clone(rbd, io_ctx, image_name.c_str(), snap_name.c_str(), dst_io_ctx,
-               dst_image_name.c_str(), opts);
+  r = rbd.clone3(io_ctx, image_name.c_str(), snap_name.c_str(), dst_io_ctx,
+                dst_image_name.c_str(), opts);
   if (r < 0) {
     std::cerr << "rbd: clone error: " << cpp_strerror(r) << std::endl;
     return r;

diff --git a/src/tools/rbd_mirror/image_replayer/Utils.h b/src/tools/rbd_mirror/image_replayer/Utils.h
index 2fea40ee334761435350760101847234809abbf3..13c244d3b62c4244778756413cf6ebd6658c4c28 100644 (file)
--- a/src/tools/rbd_mirror/image_replayer/Utils.h
+++ b/src/tools/rbd_mirror/image_replayer/Utils.h
@@ -28,7 +28,8 @@ int create_image(librados::IoCtx& io_ctx, I *_image_ctx, const char *imgname,
   return librbd::create_v2(io_ctx, imgname, bid, size, order, features,
                            stripe_unit, stripe_count, journal_order,
                            journal_splay_width, journal_pool,
-                           non_primary_global_image_id, primary_mirror_uuid);
+                           non_primary_global_image_id, primary_mirror_uuid,
+                           false);
 }
 
 template <typename I>