Re: [libvirt] [PATCH] rbd: Use RBD format 2 by default when creating images.

On 07/14/2015 04:13 PM, Josh Durgin wrote: > On 07/14/2015 12:42 PM, John Ferlan wrote: >> >> >> On 07/14/2015 04:15 AM, Wido den Hollander wrote: >>> We used to look at the librbd code version and depending on >>> that we would invoke rbd_create3() or rbd_create(). >>> >>> Since librbd version 0.67.9 we can however tell RBD that it >>> should create rbd format 2 images even if we invoke >>> rbd_create(). >>> >>> The>> less options we pass to librbd, the more we can lean on >>> the sane defaults it uses. >>> >>> For rbd_create3() we had things like the stripe count and >>> unit hardcoded in libvirt and that might cause problems down >>> the road. > > Hardcoding the feature bits is even worse. I think this is the > right approach. > OK - just trying to be sure... and since no one else spoke up yet, seems the approach is fine.

... >>> virStorageBackendRBDCreateImage(rados_ioctx_t io, char *name, >>> long capacity) { int order = 0; -#if LIBRBD_VERSION_CODE > >>> 260 - uint64_t features = 3; - uint64_t stripe_count = >>> 1; - uint64_t stripe_unit = 4194304; - - if >>> (rbd_create3(io, name, capacity, features, &order, - >>> stripe_unit, stripe_count) < 0) { -#else if (rbd_create(io, >>> name, capacity, &order) < 0) { -#endif This change fails make syntax-check due to unnecessary curly braces. Before I push, the question I see a second issue... The current code returns -1 regardless of error, which is then used in a call to virReportSystemError(-r,...)... That means regardless of the error you're returning EPERM which could throw someone off if something other than PERM was returned from rbd_create I can fix that as well by : return rbd_create(io, name, capacity, &order);

FWIW: This reporting snafu was introduced by commit id '761491e' which just took the return of virStorageBackendRBDCreateImage and used it as the basis for the message generated. John