On 16-07-15 18:28, John Ferlan wrote:
>
>
> 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.
>
Yes, I think Josh gave the right comments.
> ...
>
>>>> 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);
>
Seems like a sane approach to me. Less if-statements and the code
still works just fine.
You can apply my patch and fix this afterwards? Or do you want a new
patch from me?
Wido
>
> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list