On 04/08/16 9:41 am, "Michael S. Tsirkin" <m...@redhat.com> wrote:
>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. Done. > >> > >> >> +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. Posted a v7 which incorporates all suggestions made by Eric. https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg01027.html Regards,
