qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for cl


From: Michael S. Tsirkin
Subject: Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands.
Date: Sat, 25 Jun 2016 02:13:47 +0300

On Fri, Jun 24, 2016 at 05:39:31PM +0000, Prerna Saxena wrote:
> 
> 
> On 24/06/16 9:15 pm, "Felipe Franciosi" <address@hidden> 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" 
> ><address@hidden on behalf of address@hidden> wrote:
> >
> >Hi
> >
> >On Fri, Jun 24, 2016 at 10:17 AM, Prerna Saxena <address@hidden> wrote:
> >> From: Prerna Saxena <address@hidden>
> >>
> >> 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
> >
> >
> >



reply via email to

[Prev in Thread] Current Thread [Next in Thread]