Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands.

[Felipe Franciosi]

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

Felipe

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