Re: [Qemu-devel] [RFC PATCH 4/4] vhost-user: Add new option to specify vhost-user backend supports

[Tetsuya Mukawa <mukawa@igel.co.jp>]

On 2015/05/26 11:46, Tetsuya Mukawa wrote:
> Hi Eric,
>
> On 月, 2015-05-25 at 16:11 -0600, Eric Blake wrote:
>> On 05/25/2015 01:28 AM, Tetsuya Mukawa wrote:
>>> This patch adds 'backend_features' option for vhost-user backends.
>>> If this option is specified, QEMU assumes vhost-user backends support
>>> the features specified by user, and QEMU can start without vhost-user
>>> backend.
>>>
>>> Here are examples.
>>> * QEMU is configured as vhost-user client.
>>>  -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \
>>>  -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend_features=0x68000 \
>>>  -device virtio-net-pci,netdev=net0 \
>>>
>>> * QEMU is configured as vhost-user server.
>>>  -chardev socket,id=chr0,path=/tmp/sock,server,nowait \
>>>  -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend_features=0x68000 \
>>>  -device virtio-net-pci,netdev=net0 \
>>>
>>> To know vhost-user backend features that the backend expects, please
>>> specify 0xffffffff as backend_features, then invoke QEMU and check error log
>>> like below.
>>>
>>>   Lack of backend features. Expected 0xffffffff, but receives 0x68000
>>>
>>> Above log indicates the backend features QEMU should be passed.
>>>
>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>>> ---
>>> +++ b/qapi-schema.json
>>> @@ -2243,6 +2243,8 @@
>>>  #
>>>  # @queues: #optional number of queues to be created for multiqueue capable tap
>>>  #
>>> +# @backend_features: #optional feature flag to support vhost user backend
>> Missing a (since 2.4) designation.
>>
>> New interfaces should prefer - over _, but I see that you are consistent
>> with vnet_hder already in this struct so it is okay.
>>
>>> +#
>>>  # Since 1.2
>>>  ##
>>>  { 'struct': 'NetdevTapOptions',
>>> @@ -2259,7 +2261,8 @@
>>>      '*vhostfd':    'str',
>>>      '*vhostfds':   'str',
>>>      '*vhostforce': 'bool',
>>> -    '*queues':     'uint32'} }
>>> +    '*queues':     'uint32',
>>> +    '*backend_features':'uint64'} }
>> Ewww. Making users figure out what integers to pass is NOT user
>> friendly.  Better would be an enum type, and make the parameter an
>> optional array of enum values.
> Thanks for your comments. 
> I guess below may be good example. Is this same as your suggestion?
>
>  virtio-net-pci,netdev=hostnet3,id=net3,gso=off,guest_tso4=off,guest_tso6=off
>
> So I will improve 'backend-features' like below.
>
>  backend-features=gso=off,guest_tso4=off,guest_tso6=off
>
> Also I will fix 'qapi-schema.json' to work like above.

I seems it's impossible to implement like above.
I may need to implement like below.

virtio-net-pci,netdev=hostnet3,id=net3,backend_gso=on,backend_guest_tso4=on,backend_guest_tso6=on


Tetsuya

>>>  
>>>  ##
>>>  # @NetdevSocketOptions
>>> @@ -2444,12 +2447,15 @@
>>>  #
>>>  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
>>>  #
>>> +# @backend_features: #optional feature flag to support vhost user backend (default: 0).
>>> +#
>> Long line. Please wrap to fit within 80 columns.  Missing (since 2.4)
>> deisgnation.  
> I will fix it in v2 patch.
>
>> Again, I don't like making users know a raw integer; an
>> enum type would be better.
>>
>>>  # Since 2.1
>>>  ##
>>>  { 'struct': 'NetdevVhostUserOptions',
>>>    'data': {
>>>      'chardev':        'str',
>>> -    '*vhostforce':    'bool' } }
>>> +    '*vhostforce':    'bool',
>>> +    '*backend_features':    'uint64' } }
>> This struct has no pre-existing _, so the name 'backend-features' is nicer.
>>
> Also I will fix it.
>
> Regards,
> Tetsuya
>
>