librbd: default features should be negotiated with the OSD

Fixes: http://tracker.ceph.com/issues/17010 Signed-off-by: Mykola Golub <mgolub@mirantis.com>
ceph · Nov 7, 2016 · f066ce8 · f066ce8
1 parent 84f6d5c
commit f066ce8
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 8 deletions.
diff --git a/src/librbd/Utils.cc b/src/librbd/Utils.cc
@@ -64,15 +64,15 @@ std::string generate_image_id(librados::IoCtx &ioctx) {
   return id;
 }
 
-uint64_t parse_rbd_default_features(CephContext* cct) 
+uint64_t parse_rbd_default_features(CephContext* cct)
 {
   int ret = 0;
   uint64_t value = 0;
   auto str_val = cct->_conf->get_val<std::string>("rbd_default_features");
   try {
       value = boost::lexical_cast<decltype(value)>(str_val);
   } catch (const boost::bad_lexical_cast& ) {
-    map<std::string, int> conf_vals = {{RBD_FEATURE_NAME_LAYERING, RBD_FEATURE_LAYERING}, 
+    map<std::string, int> conf_vals = {{RBD_FEATURE_NAME_LAYERING, RBD_FEATURE_LAYERING},
                                        {RBD_FEATURE_NAME_STRIPINGV2, RBD_FEATURE_STRIPINGV2},
                                        {RBD_FEATURE_NAME_EXCLUSIVE_LOCK, RBD_FEATURE_EXCLUSIVE_LOCK},
                                        {RBD_FEATURE_NAME_OBJECT_MAP, RBD_FEATURE_OBJECT_MAP},
@@ -89,7 +89,7 @@ uint64_t parse_rbd_default_features(CephContext* cct)
         value += conf_vals[feature];
       } else {
         ret = -EINVAL;
-        ldout(cct, 1) << "Warning: unknown rbd feature " << feature << dendl;
+        lderr(cct) << "ignoring unknown feature " << feature << dendl;
       }
     }
     if (value == 0 && ret == -EINVAL)

diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc
@@ -136,6 +136,7 @@ CreateRequest<I>::CreateRequest(IoCtx &ioctx, const std::string &image_name,
 
   if (image_options.get(RBD_IMAGE_OPTION_FEATURES, &m_features) != 0) {
     m_features = util::parse_rbd_default_features(m_cct);
+    m_negotiate_features = true;
   }
 
   uint64_t features_clear = 0;
@@ -381,6 +382,48 @@ Context *CreateRequest<I>::handle_add_image_to_directory(int *result) {
     return nullptr;
   }
 
+  negotiate_features();
+  return nullptr;
+}
+
+template<typename I>
+void CreateRequest<I>::negotiate_features() {
+  if (!m_negotiate_features) {
+    create_image();
+    return;
+  }
+
+  ldout(m_cct, 20) << this << " " << __func__ << dendl;
+
+  librados::ObjectReadOperation op;
+  cls_client::get_all_features_start(&op);
+
+  using klass = CreateRequest<I>;
+  librados::AioCompletion *comp =
+    create_rados_ack_callback<klass, &klass::handle_negotiate_features>(this);
+  int r = m_ioctx.aio_operate(RBD_DIRECTORY, comp, &op, &m_outbl);
+  assert(r == 0);
+  comp->release();
+}
+
+template<typename I>
+Context *CreateRequest<I>::handle_negotiate_features(int *result) {
+  ldout(m_cct, 20) << __func__ << ": r=" << *result << dendl;
+
+  uint64_t all_features;
+  if (*result == 0) {
+    bufferlist::iterator it = m_outbl.begin();
+    *result = cls_client::get_all_features_finish(&it, &all_features);
+  }
+  if (*result < 0) {
+    ldout(m_cct, 10) << "error retrieving server supported features set: "
+                     << cpp_strerror(*result) << dendl;
+  } else if ((m_features & all_features) != m_features) {
+    m_features &= all_features;
+    ldout(m_cct, 10) << "limiting default features set to server supported: "
+		     << m_features << dendl;
+  }
+
   create_image();
   return nullptr;
 }

diff --git a/src/librbd/image/CreateRequest.h b/src/librbd/image/CreateRequest.h
@@ -60,7 +60,10 @@ class CreateRequest {
    * |               |                   v
    * |               |          ADD IMAGE TO DIRECTORY
    * |               |               /   |
-   * |      REMOVE ID OBJECT<-------/    v         (stripingv2 disabled)
+   * |      REMOVE ID OBJECT<-------/    v
+   * |               |           NEGOTIATE FEATURES (when using default features)
+   * |               |                   |
+   * |               |                   v         (stripingv2 disabled)
    * |               |              CREATE IMAGE. . . . > . . . .
    * v               |               /   |                      .
    * |      REMOVE FROM DIR<--------/    v                      .
@@ -112,6 +115,7 @@ class CreateRequest {
   int64_t m_data_pool_id = -1;
   const std::string m_non_primary_global_image_id;
   const std::string m_primary_mirror_uuid;
+  bool m_negotiate_features = false;
 
   ContextWQ *m_op_work_queue;
   Context *m_on_finish;
@@ -135,6 +139,9 @@ class CreateRequest {
   void add_image_to_directory();
   Context *handle_add_image_to_directory(int *result);
 
+  void negotiate_features();
+  Context *handle_negotiate_features(int *result);
+
   void create_image();
   Context *handle_create_image(int *result);
 

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
@@ -900,10 +900,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 : librbd::util::parse_rbd_default_features(cct);
-    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,