qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Tetsuya Mukawa
Subject: Re: [Qemu-devel] [RFC PATCH 4/4] vhost-user: Add new option to specify vhost-user backend supports
Date: Tue, 26 May 2015 11:46:31 +0900

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 <address@hidden>
> > ---
> 
> > +++ 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.

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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]