On Sat, Jun 25, 2016 at 03:13:54AM +0000, Prerna Saxena wrote: > > > > > > On 25/06/16 4:43 am, "Michael S. Tsirkin" <m...@redhat.com> wrote: > > >On Fri, Jun 24, 2016 at 05:39:31PM +0000, Prerna Saxena wrote: > >> > >> > >> On 24/06/16 9:15 pm, "Felipe Franciosi" <fel...@nutanix.com> wrote: > >> > >> >We talked to MST on IRC a while back and he brainstormed the idea of > >> >doing this per-message. > >> >(I even recall proposing to call this feature REPLY_ALL and he suggested > >> >REPLY_ANY due to that.) > >> > > >> >I agree with doing it per message, as the protocol itself should be > >> >flexible in that sense. > >> >(Even if qemu today will probably want to ask for a reply in all > >> >messages.) > >> > >> In fact, the current implementation does exactly this. If > >> VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, the current QEMU patch sets > >> the NEED_RESPONSE flag bit for all outgoing messages — basically enforcing > >> the vhost-user application to respond to all messages. > > > > > >This seems unnecessary. Let's only do that for messages that actually > >need to be synchronous. > > It would be nice to distinguish the vhost-user protocol itself from its QEMU > implementation. > The protocol should, in theory, have provision for an implementation (such as > QEMU’s vhost-user implementation) to seek response for _any_ command. > However, we can choose to be selective in our QEMU implementation and just > have limited commands currently send a response, such as SET_MEM_TABLE. > > In other words, we will still require the NEED_RESPONSE flag bit defined, but > we can just set it to 1 it for SET_MEM_TABLE command in our QEMU > implementation. All other vhost-user commands are sent from QEMU setting this > to 0, so the application does not send an ack. > > Michael, Does that correctly summarize what you were meaning to suggest here ? > > Regards, > Prerna
Exactly. > > > > >> > > >> >On 24/06/2016, 14:59, "Qemu-devel on behalf of Marc-André Lureau" > >> ><qemu-devel-bounces+felipe=nutanix....@nongnu.org on behalf of > >> >marcandre.lur...@gmail.com> wrote: > >> > > >> >Hi > >> > > >> >On Fri, Jun 24, 2016 at 10:17 AM, Prerna Saxena <saxenap....@gmail.com> > >> >wrote: > >> >> From: Prerna Saxena <prerna.sax...@nutanix.com> > >> >> > >> >> The current vhost-user protocol requires the client to send responses > >> >> to only few commands. For the remaining commands, it is impossible for > >> >> QEMU to know the status of the requested operation -- ie, did it > >> >> succeed at all, and if so, at what time. > >> >> > >> >> This is inconvenient, and can also lead to races. As an example: > >> >> > >> >> (1) qemu sends a SET_MEM_TABLE to the backend (eg, a vhost-user net > >> >> application) and SET_MEM_TABLE doesn't require a reply according to the > >> >> spec. > >> >> (2) qemu commits the memory to the guest. > >> >> (3) guest issues an I/O operation over a new memory region which was > >> >> configured on (1) > >> >> (4) The application hasn't yet remapped the memory, but it sees the I/O > >> >> request. > >> >> (5) The application cannot satisfy the request because it doesn't know > >> >> about those GPAs > >> >> > >> >> Note that the kernel implementation does not suffer from this > >> >> limitation since messages are sent via an ioctl(). The ioctl() blocks > >> >> until the backend (eg. vhost-net) completes the command and returns > >> >> (with an error code). > >> >> > >> >> Changing the behaviour of current vhost-user commands would break > >> >> existing applications. This patch introduces a protocol extension, > >> >> VHOST_USER_PROTOCOL_F_REPLY_ACK. This feature, if negotiated, allows > >> >> QEMU to annotate messages to the application that it seeks a response > >> >> for. The application must then respond to qemu by providing a status > >> >> about the requested operation. > >> > > >> >I like the idea, as I encountered a similar issue in my > >> >"vhost-user-gpu" development (which I worked around by sending a dump > >> >GET_FEATURES.. to sync things). But I question the need to have a flag > >> >per message. I think if the protocol feature is negociated, all > >> >messages should have a reply. Why do you want it to be per-message? > >> > > >> >thanks > >> > > >> >-- > >> >Marc-André Lureau > >> > > >> > > >> >
