On Sat, Jul 30, 2016 at 06:38:23AM +0000, Prerna Saxena wrote: > > > > > > On 30/07/16 2:19 am, "Eric Blake" <ebl...@redhat.com> wrote: > > >On 07/28/2016 01:07 AM, Prerna Saxena wrote: > >> From: Prerna Saxena <prerna.sax...@nutanix.com> > >> > >> This introduces the VHOST_USER_PROTOCOL_F_REPLY_ACK. > >> > > > >> + > >> +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 success or > >> +failure. The payload should be set to zero on success or non-zero on > >> 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. > > 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’d posted last night before > your message. > See https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06772.html > > > > > >> + > >> +This indicates to QEMU that the requested operation has deterministically > >> +been met or not. Today, QEMU is expected to terminate the main vhost-user > > > >Reads awkwardly; maybe: > > > >The response payload gives QEMU a deterministic indication of the result > >of the command. > > Hmm, it is more of personal taste, so I’ll refrain from commenting 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 details.) > > > >s/behaviourial/behavioural/ (or if the document widely favors US > >spelling, behavioral) > > > The last 3 iterations of this patchset have only seen review comments > focussed on documentation suggestions and indentation of code, but nothing on > the idea/code itself. This gives me hope that the patch is possibly close to > merging within 2.7 timeframe :-) > May I request the maintainers to please correct this tiny spelling typo as > this is checked in? > > Regards, > Prerna Probably easier to post v7 with above minor things. -- MST
