[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: |
Felipe Franciosi |
Subject: |
Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands. |
Date: |
Mon, 27 Jun 2016 10:45:48 +0000 |
> On 24 Jun 2016, at 23:43, Michael S. Tsirkin <address@hidden> wrote:
>
> On Fri, Jun 24, 2016 at 01:17:07AM -0700, Prerna Saxena 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
>
> OK, but there's a simple way to fix the race: submit
> another request e.g. GET_FEATURES and wait for a response.
Actually, that doesn't guarantee to fix the problem. I argue it's quite a poor
workaround.
The protocol doesn't specify that messages need to be processed in series. I
could easily see a client application receiving a SET_MEM_TABLE message and
dispatching a task to process it asynchronously. The same application could
respond to a following GET_FEATURES in line, since the command requires a
response, but no extra processing. In summary, qemu could get a response for
GET_FEATURES while SET_MEM_TABLE is being processed in the background at the
client application.
It might seem like this is unlikely to happen, but consider that a
multi-threaded application might need to take a lock in order to process
SET_MEM_TABLE. For example, waiting for all virtqueues being processed to
quiesce.
>
>
>>
>> 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 would like this structured like this:
>
> 1. fix races by sending GET_FEATURES and waiting for it
We can still add this first if you think it will help. But is it worth it?
Asking for a reply to certain commands is clearly a better fix.
Thanks,
Felipe
> 2. add VHOST_USER_PROTOCOL_F_REPLY_ACK
>
>
>>
>> Prerna Saxena (1):
>> vhost-user : Introduce a new feature, VHOST_USER_PROTOCOL_F_REPLY_ACK
>> This feature, if negotiated, forces the remote vhost-user
>> process to send a u64 reply containin status code for each
>> requested operation.
>> Status codes are '0' for success, and non-zero for error.
>>
>> docs/specs/vhost-user.txt | 36 +++++++++++
>> hw/virtio/vhost-user.c | 153
>> +++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 186 insertions(+), 3 deletions(-)
>>
>> --
>> 1.8.1.2
>>
>
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., (continued)
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Prerna, 2016/06/24
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Marc-André Lureau, 2016/06/24
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Felipe Franciosi, 2016/06/24
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Prerna Saxena, 2016/06/24
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Michael S. Tsirkin, 2016/06/24
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Prerna Saxena, 2016/06/24
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Michael S. Tsirkin, 2016/06/25
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Prerna Saxena, 2016/06/26
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Michael S. Tsirkin, 2016/06/25
Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands., Michael S. Tsirkin, 2016/06/24
- Re: [Qemu-devel] [PATCH 0/1] vhost-user: Add a protocol extension for client responses to vhost commands.,
Felipe Franciosi <=