From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60204) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z59ek-0002qP-Ec for qemu-devel@nongnu.org; Wed, 17 Jun 2015 05:29:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z59eb-0002BL-I0 for qemu-devel@nongnu.org; Wed, 17 Jun 2015 05:29:14 -0400 Received: from mail-pd0-f178.google.com ([209.85.192.178]:33376) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z59eb-0002B6-AG for qemu-devel@nongnu.org; Wed, 17 Jun 2015 05:29:05 -0400 Received: by pdjn11 with SMTP id n11so35230838pdj.0 for ; Wed, 17 Jun 2015 02:29:04 -0700 (PDT) Message-ID: <55813DDD.50707@igel.co.jp> Date: Wed, 17 Jun 2015 18:29:01 +0900 From: Tetsuya Mukawa MIME-Version: 1.0 References: <1432538908-26298-5-git-send-email-mukawa@igel.co.jp> <1432874550-10921-1-git-send-email-mukawa@igel.co.jp> <1432874550-10921-5-git-send-email-mukawa@igel.co.jp> <5580164D.8020505@redhat.com> In-Reply-To: <5580164D.8020505@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v1 4/4] vhost-user: Add new option to specify vhost-user backend features List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: jasowang@redhat.com, n.nikolaev@virtualopensystems.com, stefanha@redhat.com 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 suppo= rts >> the features, then QEMU can start without vhost-user backend connectio= n. >> (While no connection, link status of virtio-net device will be down) >> >> When connection between QEMU and the backend is established, QEMU chec= kes 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 >> --- >> +++ 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 bac= kend >> +# (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 option= s. >> +++ b/qemu-options.hx >> @@ -1467,6 +1467,15 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, >> "-netdev tap,id=3Dstr[,fd=3Dh][,fds=3Dx:y:...:z][,ifname=3Dname][= ,script=3Dfile][,downscript=3Ddfile]\n" >> " [,helper=3Dhelper][,sndbuf=3Dnbytes][,vnet_hdr=3Don|off= ][,vhost=3Don|off]\n" >> " [,vhostfd=3Dh][,vhostfds=3Dx:y:...:z][,vhostforce=3Don|= off][,queues=3Dn]\n" >> + " [,backend_csum=3Don|off][,backend_guest_csum=3Don|off][= ,backend_gso=3Don|off]\n" >> + " [,backend_guest_tso4=3Don|off][,backend_guest_tso6=3Don= |off]\n" >> + " [,backend_guest_ecn=3Don|off][,backend_guest_ufo=3Don|o= ff]\n" >> + " [,backend_guest_announce=3Don|off][,backend_host_tso4=3D= on|off]\n" >> + " [,backend_host_tso6=3Don|off][,backend_host_ecn=3Don|of= f]\n" >> + " [,backend_host_ufo=3Don|off][,backend_mrg_rxbuf=3Don|of= f][,backend_status=3Don|off]\n" >> + " [,backend_ctrl_vq=3Don|off][,backend_ctrl_vx=3Don|off][= ,backend_ctrl_vlan=3Don|off]\n" >> + " [,backend_ctrl_rx_extra=3Don|off][,backend_ctrl_mac_add= r=3Don|off]\n" >> + " [,backend_ctrl_guest_offloads=3Don|off][,backend_mq=3Do= n|off]\n" >> " configure a host TAP network backend with ID 'st= r'\n" >> " use network scripts 'file' (default=3D" DEFAULT_= NETWORK_SCRIPT ")\n" >> " to configure it and 'dfile' (default=3D" DEFAULT= _NETWORK_DOWN_SCRIPT ")\n" >> @@ -1486,6 +1495,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, >> " use 'vhostfd=3Dh' to connect to an already opene= d vhost net device\n" >> " use 'vhostfds=3Dx:y:...:z to connect to multiple= already opened vhost net devices\n" >> " use 'queues=3Dn' to specify the number of queues= to be created for multiqueue TAP\n" >> + " use 'backend_*=3Don' to specify virtio-net featu= re that vhost-user backend supports\n" > Bikeshedding: Rather than having a lot of booleans, I wonder if it woul= d > 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 l= ine. > 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