Merge pull request #303 from ceph/wip-librbd-config-create

Reviewed-by: Sage Weil <sage.weil@inktank.com>

8 files changed, 162 insertions, 13 deletions

diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc
index 92349ea9304..c09f1ee604a 100644
--- a/src/cls/rbd/cls_rbd.cc
+++ b/src/cls/rbd/cls_rbd.cc
@@ -678,7 +678,7 @@ int set_stripe_unit_count(cls_method_context_t hctx, bufferlist *in, bufferlist
     CLS_ERR("failed to read the order off of disk: %s", strerror(r));
     return r;
   }
-  if ((1ull << order) % stripe_unit) {
+  if ((1ull << order) % stripe_unit || stripe_unit > (1ull << order)) {
     CLS_ERR("stripe unit %llu is not a factor of the object size %llu",
             (unsigned long long)stripe_unit, 1ull << order);
     return -EINVAL;
diff --git a/src/common/config_opts.h b/src/common/config_opts.h
index 776ac1c067e..8a1da07e036 100644
--- a/src/common/config_opts.h
+++ b/src/common/config_opts.h
@@ -536,6 +536,7 @@ OPTION(journal_align_min_size, OPT_INT, 64 << 10)  // align data payloads >= thi
 OPTION(journal_replay_from, OPT_INT, 0)
 OPTION(journal_zero_on_create, OPT_BOOL, false)
 OPTION(journal_ignore_corruption, OPT_BOOL, false) // assume journal is not corrupt
+
 OPTION(rbd_cache, OPT_BOOL, false) // whether to enable caching (writeback unless rbd_cache_max_dirty is 0)
 OPTION(rbd_cache_writethrough_until_flush, OPT_BOOL, false) // whether to make writeback caching writethrough until flush is called, to be sure the user of librbd will send flushs so that writeback is safe
 OPTION(rbd_cache_size, OPT_LONGLONG, 32<<20)         // cache size in bytes
@@ -547,6 +548,28 @@ OPTION(rbd_concurrent_management_ops, OPT_INT, 10) // how many operations can be
 OPTION(rbd_balance_snap_reads, OPT_BOOL, false)
 OPTION(rbd_localize_snap_reads, OPT_BOOL, false)
 
+/*
+ * The following options change the behavior for librbd's image creation methods that
+ * don't require all of the parameters. These are provided so that older programs
+ * can take advantage of newer features without being rewritten to use new versions
+ * of the image creation functions.
+ *
+ * rbd_create()/RBD::create() are affected by all of these options.
+ *
+ * rbd_create2()/RBD::create2() and rbd_clone()/RBD::clone() are affected by:
+ * - rbd_default_order
+ * - rbd_default_stripe_count
+ * - rbd_default_stripe_size
+ *
+ * rbd_create3()/RBD::create3() and rbd_clone2/RBD::clone2() are only
+ * affected by rbd_default_order.
+ */
+OPTION(rbd_default_format, OPT_INT, 1)
+OPTION(rbd_default_order, OPT_INT, 22)
+OPTION(rbd_default_stripe_count, OPT_U64, 1) // changing requires stripingv2 feature
+OPTION(rbd_default_stripe_unit, OPT_U64, 4194304) // changing to non-object size requires stripingv2 feature
+OPTION(rbd_default_features, OPT_INT, 3) // 1 for layering, 3 for layering+stripingv2. only applies to format 2 images
+
 OPTION(nss_db_path, OPT_STR, "") // path to nss db
 
 OPTION(rgw_data, OPT_STR, "/var/lib/ceph/radosgw/$cluster-$id")
diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index d4a25e1ed8a..789ef8694c7 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -821,6 +821,15 @@ reprotect_and_return_err:
     return r;
   }
 
+  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);
+  }
+
   int create(IoCtx& io_ctx, const char *imgname, uint64_t size,
 	     bool old_format, uint64_t features, int *order,
 	     uint64_t stripe_unit, uint64_t stripe_count)
@@ -853,6 +862,11 @@ reprotect_and_return_err:
     if (!order)
       return -EINVAL;
 
+    if (!*order)
+      *order = cct->_conf->rbd_default_order;
+    if (!*order)
+      *order = RBD_DEFAULT_OBJ_ORDER;
+
     if (*order && (*order > 64 || *order < 12)) {
       lderr(cct) << "order must be in the range [12, 64]" << dendl;
       return -EDOM;
@@ -860,8 +874,12 @@ reprotect_and_return_err:
 
     uint64_t bid = rbd_assign_bid(io_ctx);
 
-    if (!*order)
-      *order = RBD_DEFAULT_OBJ_ORDER;
+    // if striping is enabled, use possibly custom defaults
+    if (!old_format && (features & RBD_FEATURE_STRIPINGV2) &&
+	!stripe_unit && !stripe_count) {
+      stripe_unit = cct->_conf->rbd_default_stripe_unit;
+      stripe_count = cct->_conf->rbd_default_stripe_count;
+    }
 
     // normalize for default striping
     if (stripe_unit == (1ull << *order) && stripe_count == 1) {
@@ -973,7 +991,8 @@ reprotect_and_return_err:
     if (!order)
       order = p_imctx->order;
 
-    r = create(c_ioctx, c_name, size, false, features, &order, stripe_unit, stripe_count);
+    r = create(c_ioctx, c_name, size, false, features, &order,
+	       stripe_unit, stripe_count);
     if (r < 0) {
       lderr(cct) << "error creating child: " << cpp_strerror(r) << dendl;
       goto err_close_parent;
diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index a7d39b3c964..048e4387c41 100644
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -81,6 +81,8 @@ namespace librbd {
   int list_children(ImageCtx *ictx,
 		    std::set<pair<std::string, std::string> > & names);
   int create(librados::IoCtx& io_ctx, const char *imgname, uint64_t size,
+	     int *order);
+  int create(librados::IoCtx& io_ctx, const char *imgname, uint64_t size,
 	     bool old_format, uint64_t features, int *order,
 	     uint64_t stripe_unit, uint64_t stripe_count);
   int clone(IoCtx& p_ioctx, const char *p_name, const char *p_snap_name,
diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index 89bbe595752..af413dda04f 100644
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -115,7 +115,7 @@ namespace librbd {
 
   int RBD::create(IoCtx& io_ctx, const char *name, uint64_t size, int *order)
   {
-    return librbd::create(io_ctx, name, size, true, 0, order, 0, 0);
+    return librbd::create(io_ctx, name, size, order);
   }
 
   int RBD::create2(IoCtx& io_ctx, const char *name, uint64_t size,
@@ -128,7 +128,8 @@ namespace librbd {
 		   uint64_t features, int *order, uint64_t stripe_unit,
 		   uint64_t stripe_count)
   {
-    return librbd::create(io_ctx, name, size, false, features, order, stripe_unit, stripe_count);
+    return librbd::create(io_ctx, name, size, false, features, order,
+			  stripe_unit, stripe_count);
   }
 
   int RBD::clone(IoCtx& p_ioctx, const char *p_name, const char *p_snap_name,
@@ -559,7 +560,7 @@ extern "C" int rbd_create(rados_ioctx_t p, const char *name, uint64_t size, int
 {
   librados::IoCtx io_ctx;
   librados::IoCtx::from_rados_ioctx_t(p, io_ctx);
-  return librbd::create(io_ctx, name, size, true, 0, order, 0, 0);
+  return librbd::create(io_ctx, name, size, order);
 }
 
 extern "C" int rbd_create2(rados_ioctx_t p, const char *name,
@@ -578,7 +579,8 @@ extern "C" int rbd_create3(rados_ioctx_t p, const char *name,
 {
   librados::IoCtx io_ctx;
   librados::IoCtx::from_rados_ioctx_t(p, io_ctx);
-  return librbd::create(io_ctx, name, size, false, features, order, stripe_unit, stripe_count);
+  return librbd::create(io_ctx, name, size, false, features, order,
+			stripe_unit, stripe_count);
 }
 
 extern "C" int rbd_clone(rados_ioctx_t p_ioctx, const char *p_name,
diff --git a/src/pybind/rbd.py b/src/pybind/rbd.py
index b59ff63ab05..9d71738e728 100644
--- a/src/pybind/rbd.py
+++ b/src/pybind/rbd.py
@@ -734,15 +734,21 @@ written." % (self.name, ret, length))
         """
         Returns the stripe unit used for the image.
         """
-        ret = self.librbd.rbd_get_stripe_unit()
-        return ret.value
+        stripe_unit = c_uint64()
+        ret = self.librbd.rbd_get_stripe_unit(self.image, byref(stripe_unit))
+        if ret != 0:
+            raise make_ex(ret, 'error getting stripe unit for image' % (self.name))
+        return stripe_unit.value
 
     def stripe_count(self):
         """
         Returns the stripe count used for the image.
         """
-        ret = self.librbd.rbd_get_stripe_count()
-        return ret.value
+        stripe_count = c_uint64()
+        ret = self.librbd.rbd_get_stripe_count(self.image, byref(stripe_count))
+        if ret != 0:
+            raise make_ex(ret, 'error getting stripe count for image' % (self.name))
+        return stripe_count.value
 
     def flatten(self):
         """
diff --git a/src/test/cls_rbd/test_cls_rbd.cc b/src/test/cls_rbd/test_cls_rbd.cc
index 6308980f55f..c147b43f4cb 100644
--- a/src/test/cls_rbd/test_cls_rbd.cc
+++ b/src/test/cls_rbd/test_cls_rbd.cc
@@ -906,6 +906,15 @@ TEST(cls_rbd, stripingv2)
   ASSERT_EQ(8192ull, su);
   ASSERT_EQ(456ull, sc);
 
+  // su must not be larger than an object
+  ASSERT_EQ(-EINVAL, set_stripe_unit_count(&ioctx, "bar", 1 << 23, 1));
+  // su must be a factor of object size
+  ASSERT_EQ(-EINVAL, set_stripe_unit_count(&ioctx, "bar", 511, 1));
+  // su and sc must be non-zero
+  ASSERT_EQ(-EINVAL, set_stripe_unit_count(&ioctx, "bar", 0, 1));
+  ASSERT_EQ(-EINVAL, set_stripe_unit_count(&ioctx, "bar", 1, 0));
+  ASSERT_EQ(-EINVAL, set_stripe_unit_count(&ioctx, "bar", 0, 0));
+
   ioctx.close();
   ASSERT_EQ(0, destroy_one_pool_pp(pool_name, rados));
 }
diff --git a/src/test/pybind/test_rbd.py b/src/test/pybind/test_rbd.py
index ea3e5fc5e04..5f14e62bbde 100644
--- a/src/test/pybind/test_rbd.py
+++ b/src/test/pybind/test_rbd.py
@@ -8,7 +8,8 @@ from nose.tools import eq_ as eq, assert_raises
 from rados import Rados
 from rbd import (RBD, Image, ImageNotFound, InvalidArgument, ImageExists,
                  ImageBusy, ImageHasSnapshots, ReadOnlyImage,
-                 FunctionNotSupported, RBD_FEATURE_LAYERING)
+                 FunctionNotSupported, ArgumentOutOfRange,
+                 RBD_FEATURE_LAYERING, RBD_FEATURE_STRIPINGV2)
 
 
 rados = None
@@ -66,6 +67,93 @@ def test_create():
     create_image()
     remove_image()
 
+def check_default_params(format, order=None, features=None, stripe_count=None,
+                         stripe_unit=None, exception=None):
+    global rados
+    global ioctx
+    orig_vals = {}
+    for k in ['rbd_default_format', 'rbd_default_order', 'rbd_default_features',
+              'rbd_default_stripe_count', 'rbd_default_stripe_unit']:
+        orig_vals[k] = rados.conf_get(k)
+    try:
+        rados.conf_set('rbd_default_format', str(format))
+        if order is not None:
+            rados.conf_set('rbd_default_order', str(order or 0))
+        if features is not None:
+            rados.conf_set('rbd_default_features', str(features or 0))
+        if stripe_count is not None:
+            rados.conf_set('rbd_default_stripe_count', str(stripe_count or 0))
+        if stripe_unit is not None:
+            rados.conf_set('rbd_default_stripe_unit', str(stripe_unit or 0))
+        if exception is None:
+            RBD().create(ioctx, IMG_NAME, IMG_SIZE)
+            try:
+                with Image(ioctx, IMG_NAME) as image:
+                    eq(format == 1, image.old_format())
+
+                    expected_order = order
+                    if not order:
+                        expected_order = 22
+                    actual_order = image.stat()['order']
+                    eq(expected_order, actual_order)
+
+                    expected_features = features
+                    if expected_features is None or format == 1:
+                        expected_features = 0 if format == 1 else 3
+                    eq(expected_features, image.features())
+
+                    expected_stripe_count = stripe_count
+                    if not expected_stripe_count or format == 1 or \
+                           features & RBD_FEATURE_STRIPINGV2 == 0:
+                        expected_stripe_count = 1
+                    eq(expected_stripe_count, image.stripe_count())
+
+                    expected_stripe_unit = stripe_unit
+                    if not expected_stripe_unit or format == 1 or \
+                           features & RBD_FEATURE_STRIPINGV2 == 0:
+                        expected_stripe_unit = 1 << actual_order
+                    eq(expected_stripe_unit, image.stripe_unit())
+            finally:
+                RBD().remove(ioctx, IMG_NAME)
+        else:
+            assert_raises(exception, RBD().create, ioctx, IMG_NAME, IMG_SIZE)
+    finally:
+        for k, v in orig_vals.iteritems():
+            rados.conf_set(k, v)
+
+def test_create_defaults():
+    # basic format 1 and 2
+    check_default_params(1)
+    check_default_params(2)
+    # default order still works
+    check_default_params(1, 0)
+    check_default_params(2, 0)
+    # invalid order
+    check_default_params(1, 11, exception=ArgumentOutOfRange)
+    check_default_params(2, 11, exception=ArgumentOutOfRange)
+    check_default_params(1, 65, exception=ArgumentOutOfRange)
+    check_default_params(2, 65, exception=ArgumentOutOfRange)
+    # striping and features are ignored for format 1
+    check_default_params(1, 20, 0, 1, 1)
+    check_default_params(1, 20, 3, 1, 1)
+    check_default_params(1, 20, 0, 0, 0)
+    # striping is ignored if stripingv2 is not set
+    check_default_params(2, 20, 0, 1, 1 << 20)
+    check_default_params(2, 20, RBD_FEATURE_LAYERING, 1, 1 << 20)
+    check_default_params(2, 20, 0, 0, 0)
+    # striping with stripingv2 is fine
+    check_default_params(2, 20, RBD_FEATURE_STRIPINGV2, 1, 1 << 16)
+    check_default_params(2, 20, RBD_FEATURE_STRIPINGV2, 10, 1 << 20)
+    check_default_params(2, 20, RBD_FEATURE_STRIPINGV2, 10, 1 << 16)
+    # make sure invalid combinations of stripe unit and order are still invalid
+    check_default_params(2, 20, RBD_FEATURE_STRIPINGV2, exception=InvalidArgument)
+    check_default_params(2, 22, RBD_FEATURE_STRIPINGV2, 10, 1 << 50, exception=InvalidArgument)
+    check_default_params(2, 22, RBD_FEATURE_STRIPINGV2, 10, 100, exception=InvalidArgument)
+    check_default_params(2, 22, RBD_FEATURE_STRIPINGV2, 0, 1, exception=InvalidArgument)
+    check_default_params(2, 22, RBD_FEATURE_STRIPINGV2, 1, 0, exception=InvalidArgument)
+    # 0 stripe unit and count are still ignored
+    check_default_params(2, 22, RBD_FEATURE_STRIPINGV2, 0, 0)
+
 def test_context_manager():
     with Rados(conffile='') as cluster:
         with cluster.open_ioctx('rbd') as ioctx: