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. > > > >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 > > > > > >
