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