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

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

On 2015/06/16 21:27, Eric Blake wrote:
> On 05/28/2015 10:42 PM, Tetsuya Mukawa wrote:
>> This patch adds below '-net' options to let QEMU know which features
>> vhost-user backend will support.
> [meta-comment: when posting a new revision of a series, do it as a new
> top-level thread rather than buried in-reply-to an earlier version]

Hi Eric,

Thanks for comments.
Yes, I will follow like above in next patches.

>> If above features are specified, QEMU assumes vhost-user backend supports
>> the features, then QEMU can start without vhost-user backend connection.
>> (While no connection, link status of virtio-net device will be down)
>>
>> When connection between QEMU and the backend is established, QEMU checkes feature
> s/checkes/checks/
>
>> values of the backend to make sure the expected features are provided.
>> If it doesn't, the connection will be closed by QEMU.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>> +++ b/qapi-schema.json
>> @@ -2241,25 +2241,29 @@
>>  #
>>  # @vhostforce: #optional vhost on for non-MSIX virtio guests
>>  #
>> +# @backend_features: #optional feature flag to support vhost user backend
>> +#                    (Since 2.4)
>> +#
> s/backend_features/backend-features/
>
> (we document that qapi should prefer - over _ unless there is a
> consistency issue with _ already used in the struct)
>
>
>> @@ -2444,12 +2448,92 @@
>>  #
>>  # @vhostforce: #optional vhost on for non-MSIX virtio guests (default: false).
>>  #
>> +# @backend_csum: #optional feature backend supports (default: false).
>> +#                (Since 2.4)
> Again, s/_/-/ throughout this struct.

I will use '-' instead of '_'. Also will fix typo.

>> +#
>> +# @backend_mq: #optional feature backend supports (default: false).
>> +#              (Since 2.4)
> A lot of identical descriptions. Might be worth more specific text to
> each option; for example,
>  @backend_mq: #optional: True if backend supports multiqueue feature
> (default: false). (Since 2.4)
>

Sure I will add short descriptions, if we use this kind of command options.

>> +++ b/qemu-options.hx
>> @@ -1467,6 +1467,15 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>>      "-netdev tap,id=str[,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile]\n"
>>      "         [,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
>>      "         [,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
>> +    "         [,backend_csum=on|off][,backend_guest_csum=on|off][,backend_gso=on|off]\n"
>> +    "         [,backend_guest_tso4=on|off][,backend_guest_tso6=on|off]\n"
>> +    "         [,backend_guest_ecn=on|off][,backend_guest_ufo=on|off]\n"
>> +    "         [,backend_guest_announce=on|off][,backend_host_tso4=on|off]\n"
>> +    "         [,backend_host_tso6=on|off][,backend_host_ecn=on|off]\n"
>> +    "         [,backend_host_ufo=on|off][,backend_mrg_rxbuf=on|off][,backend_status=on|off]\n"
>> +    "         [,backend_ctrl_vq=on|off][,backend_ctrl_vx=on|off][,backend_ctrl_vlan=on|off]\n"
>> +    "         [,backend_ctrl_rx_extra=on|off][,backend_ctrl_mac_addr=on|off]\n"
>> +    "         [,backend_ctrl_guest_offloads=on|off][,backend_mq=on|off]\n"
>>      "                configure a host TAP network backend with ID 'str'\n"
>>      "                use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
>>      "                to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
>> @@ -1486,6 +1495,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>>      "                use 'vhostfd=h' to connect to an already opened vhost net device\n"
>>      "                use 'vhostfds=x:y:...:z to connect to multiple already opened vhost net devices\n"
>>      "                use 'queues=n' to specify the number of queues to be created for multiqueue TAP\n"
>> +    "                use 'backend_*=on' to specify virtio-net feature that vhost-user backend supports\n"
> Bikeshedding: Rather than having a lot of booleans, I wonder if it would
> be better to have an array of flag names.  That is, in QMP form, this
> would select three features (and leave the others off)
>
> 'backend-features':[ 'csum', 'guest-csum', gso' ]
>
> although I'm not sure on how that would best translate to the command line.
>

One more suggestion is here.
https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg03807.html

I am also not sure what is best. But I guess above explanation is
reasonable at this time.
When we find a case that user needs to describe or change
backend-features frequently by hand, expand backend-features and add
these human readable options.

Regards,
Tetsuya