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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2849 bytes --]

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.

>  
>  ##
>  # @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.  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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]