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