From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46512) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bVA00-0000Q6-QR for qemu-devel@nongnu.org; Thu, 04 Aug 2016 00:11:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bV9zw-0005ui-GD for qemu-devel@nongnu.org; Thu, 04 Aug 2016 00:11:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56648) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bV9zw-0005ue-80 for qemu-devel@nongnu.org; Thu, 04 Aug 2016 00:11:08 -0400 Date: Thu, 4 Aug 2016 07:11:02 +0300 From: "Michael S. Tsirkin" Message-ID: <20160804070906-mutt-send-email-mst@kernel.org> References: <1469689643-11556-1-git-send-email-saxenap.ltc@gmail.com> <1469689643-11556-2-git-send-email-saxenap.ltc@gmail.com> <579BC143.4090203@redhat.com> <34904E8F-28F7-48C7-9880-44BC2CCA9935@nutanix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <34904E8F-28F7-48C7-9880-44BC2CCA9935@nutanix.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-2.7 v5.1 1/2] vhost-user: Introduce a new protocol feature REPLY_ACK. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Prerna Saxena Cc: Eric Blake , Prerna Saxena , "qemu-devel@nongnu.org" , Anil Kumar Boggarapu , Felipe Franciosi , "marcandre.lureau@gmail.com" On Sat, Jul 30, 2016 at 06:38:23AM +0000, Prerna Saxena wrote: >=20 >=20 >=20 >=20 >=20 > On 30/07/16 2:19 am, "Eric Blake" wrote: >=20 > >On 07/28/2016 01:07 AM, Prerna Saxena wrote: > >> From: Prerna Saxena > >>=20 > >> This introduces the VHOST_USER_PROTOCOL_F_REPLY_ACK. > >>=20 > > > >> + > >> +With this protocol extension negotiated, the sender (QEMU) can set = the > >> +"need_reply" [Bit 3] flag to any command. This indicates that > >> +the client MUST respond with a Payload VhostUserMsg indicating succ= ess or > >> +failure. The payload should be set to zero on success or non-zero o= n failure. > >> +(Unless the message already has an explicit reply body) > > > >Rather than make this parenthetical, I would go with: > > > >The payload should be set to zero on success or non-zero on failure, > >unless the message already has an explicit reply body. >=20 > Hi Eric, > Thank you for taking a look, but I think you possibly missed the latest= patchset posted last night. > This had already been incorporated in v6 that I=E2=80=99d posted last n= ight before your message. > See https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06772.html >=20 >=20 > > > >> + > >> +This indicates to QEMU that the requested operation has determinist= ically > >> +been met or not. Today, QEMU is expected to terminate the main vhos= t-user > > > >Reads awkwardly; maybe: > > > >The response payload gives QEMU a deterministic indication of the resu= lt > >of the command. >=20 > Hmm, it is more of personal taste, so I=E2=80=99ll refrain from comment= ing either way. I prefer Eric's form too. "that ... or not" isn't very clear. > > > >> +loop upon receiving such errors. In future, qemu could be taught to= be more > >> +resilient for selective requests. > >> + > >> +For the message types that already solicit a reply from the client,= the > >> +presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or need_reply bit being= set brings > >> +no behaviourial change. (See the 'Communication' section for detail= s.) > > > >s/behaviourial/behavioural/ (or if the document widely favors US > >spelling, behavioral) >=20 >=20 > The last 3 iterations of this patchset have only seen review comments f= ocussed on documentation suggestions and indentation of code, but nothing= on the idea/code itself. This gives me hope that the patch is possibly c= lose to merging within 2.7 timeframe :-) > May I request the maintainers to please correct this tiny spelling typo= as this is checked in? >=20 > Regards, > Prerna Probably easier to post v7 with above minor things. --=20 MST