Re: [PATCH] libxl: Add "grant_usage" parameter for virtio disk devices

[Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>]

On 06.02.24 12:27, Anthony PERARD wrote:


Hello Anthony

[snip]

>>>> diff --git a/docs/man/xl-disk-configuration.5.pod.in 
>>>> b/docs/man/xl-disk-configuration.5.pod.in
>>>> index bc945cc517..3c035456d5 100644
>>>> --- a/docs/man/xl-disk-configuration.5.pod.in
>>>> +++ b/docs/man/xl-disk-configuration.5.pod.in
>>>> @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. 
>>>> Please note, the virtual
>>>> +=item B<grant_usage=BOOLEAN>
>>>>
>>>> +=over 4
>>>> +
>>>> +=item Description
>>>> +
>>>> +Specifies the usage of Xen grants for accessing guest memory. Only 
>>>> applicable
>>>> +to specification "virtio".
>>>> +
>>>> +=item Supported values
>>>> +
>>>> +If this option is B<true>, the Xen grants are always enabled.
>>>> +If this option is B<false>, the Xen grants are always disabled.
>>>
>>> Unfortunately, this is wrong, the implementation in the patch only
>>> support two values: 1 / 0, nothing else, and trying to write "true" or
>>> "false" would lead to an error. (Well actually it's "grant_usage=1" or
>>> "grant_usage=0", there's nothing that cut that string at the '='.)
>>
>>
>> You are right, only 1 / 0 can be set unlike for virtio=[...] which seems
>> happy with false/true.
>>
>>
>>>
>>> Also, do we really need the extra verbal description of each value here?
>>> Is simply having the following would be enough?
>>>
>>>       =item Supported values
>>>
>>>       1, 0
>>>
>>> The description in "Description" section would hopefully be enough.
>>
>>
>> I think, this makes sense.
>>
>> So, shall I leave "grant_usage=1/grant_usage=0" or use proposed option
>> "use-grant/no-use-grant"?
> 
> Let's go with "grant_usage=*", at least this will be consistent with the
> option for "virtio".


thanks for the confirmation, will do


> 
> Cheers,
>