On Wed, Oct 14, 2015 at 03:34:52PM -0400, Jason Dillaman wrote: > In general, I like the approach. > > I am concerned about passing a void* + length to specify the option > value since you really can't protect against the user providing data > in the incorrect format. For example, if the backend treated > RBD_OPTION_STRIPE_UNIT as a 4byte int, what happens if someone > passes a 2- or 8-byte int or a 4-byte char* string? Then rbd_image_options_set() will fail with EINVAL, because the option type (size) is a part of interface. I do this by analogy to setsockopt(2): http://pubs.opengroup.org/onlinepubs/009695399/functions/setsockopt.html Note option type documented for every option there, and it works fairly well. Following a common practice is an additional argument to this approach to me. > Therefore, I would vote for passing strings a la librados > rados_conf_set. I'd rather don't use this approach, as it adds unnecessary work on both user (encode the option to string) and backend (decode from string) sides. > Perhaps rbd_create4 and rbd_clone3 should move the order and > features options to rbd_image_options_t as well? My initial thought was that a user would want to set features and orders more frequently than other options, so keeping them as additional arguments would be useful. But now thinking more about it, I agree that they can be moved to options. > -- > > Jason Dillaman > > > ----- Original Message ----- > > From: "Mykola Golub" <mgolub@xxxxxxxxxxxx> > > To: ceph-devel@xxxxxxxxxxxxxxx > > Cc: "Jason Dillaman" <dillaman@xxxxxxxxxx>, "Josh Durgin" <jdurgin@xxxxxxxxxx> > > Sent: Wednesday, September 30, 2015 2:50:45 AM > > Subject: CEPH_RBD_API: options on image create > > > > Hi, > > > > It was mentioned several times eralier that it would be nice to pass > > options as key/value configuration pairs on image create instead of > > expanding rbd_create/rbd_clone/rbd_copy for every possible > > configuration override. > > > > What do you think about this API? > > > > Introduce rbd_image_options_t and functions to manipulate it: > > > > int rbd_image_options_create(rbd_image_options_t* opts); > > > > void rbd_image_options_destroy(rbd_image_options_t opts); > > > > int rbd_image_options_set(rbd_image_options_t opts, int optname, > > const void* optval, size_t optlen); > > > > int rbd_image_options_get(rbd_image_options_t opts, int optname, > > void* optval, size_t* optlen); > > > > void rbd_image_options_iterate(rbd_image_options_t opts, > > void (*func)(int* optname, void* optval, > > size_t* optlen)); > > > > Functions that return a value return 0 on success, and -ERROR on > > failure. > > > > optname is a constant like RBD_OPTION_STRIPE_UNIT, > > RBD_OPTION_STRIPE_COUNT... > > > > Pass options as additional argument to rbd_create, rbd_clone (and may > > be rbd_copy) functions: > > > > int rbd_create4(rados_ioctx_t io, const char *name, uint64_t size, > > uint64_t features, int *order, rbd_image_options_t opts); > > > > int rbd_clone3(rados_ioctx_t p_ioctx, const char *p_name, > > const char *p_snapname, rados_ioctx_t c_ioctx, > > const char *c_name, uint64_t features, int *c_order, > > rbd_image_options_t opts); > > > > int rbd_copy3(rbd_image_t src, rbd_image_t dest, rbd_image_options_t opts); > > // possibly > > > > > > Example: > > > > rbd_image_options_t opts; > > int r; > > r = rbd_image_options_create(&opts); > > assert(r == 0); > > uint64_t stripe_unit = 65536; > > r = rbd_image_options_set(opts, RBD_OPTION_STRIPE_UNIT, > > &stripe_unit, size_of(stripe_unit)); > > assert(r == 0); > > uint64_t stripe_count = 16; > > r = rbd_image_options_set(opts, RBD_OPTION_STRIPE_COUNT, > > &stripe_count, size_of(stripe_count)); > > assert(r == 0); > > const char* journal_object_pool = "journal"; > > r = rbd_image_options_set(opts, RBD_OPTION_JOURNAL_OBJECT_POOL, > > journal_object_pool, strlen(journal_object_pool) + > > 1); > > assert(r == 0); > > r = rbd_create4(io, name, size, features, int *order, rbd_image_options_t > > opts); > > > > cleanup: > > rbd_image_options_destroy(opts); > > > > -- > > Mykola Golub > > -- > > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Mykola Golub -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
