[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/2] vhost-user: Introduce a new protocol fea
Re: [Qemu-devel] [PATCH v4 1/2] vhost-user: Introduce a new protocol feature REPLY_ACK.
Thu, 28 Jul 2016 06:28:32 +0000
On 27/07/16 6:58 pm, "Michael S. Tsirkin" <address@hidden> wrote:
>On Wed, Jul 27, 2016 at 12:56:18PM +0000, Prerna Saxena wrote:
>> Hi Marc,
>> Thanks, please find my reply inline.
>> On 27/07/16 4:35 pm, "Marc-André Lureau" <address@hidden> wrote:
>> >On Wed, Jul 27, 2016 at 1:52 PM, Prerna Saxena <address@hidden> wrote:
>> >> From: Prerna Saxena <address@hidden>
>> >> This introduces the VHOST_USER_PROTOCOL_F_REPLY_ACK.
>> >> If negotiated, client applications should send a u64 payload in
>> >> response to any message that contains the "need_response" bit set
>> >> on the message flags. Setting the payload to "zero" indicates the
>> >> command finished successfully. Likewise, setting it to "non-zero"
>> >> indicates an error.
>> >> Currently implemented only for SET_MEM_TABLE.
>> >> Signed-off-by: Prerna Saxena <address@hidden>
>> >> ---
>> >> docs/specs/vhost-user.txt | 41 +++++++++++++++++++++++++++++++++++++++++
>> >> hw/virtio/vhost-user.c | 32 ++++++++++++++++++++++++++++++++
>> >> 2 files changed, 73 insertions(+)
>> >> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>> >> index 777c49c..57df586 100644
>> >> --- a/docs/specs/vhost-user.txt
>> >> +++ b/docs/specs/vhost-user.txt
>> >> @@ -37,6 +37,8 @@ consists of 3 header fields and a payload:
>> >> * Flags: 32-bit bit field:
>> >> - Lower 2 bits are the version (currently 0x01)
>> >> - Bit 2 is the reply flag - needs to be sent on each reply from the
>> >> slave
>> >> + - Bit 3 is the need_response flag - see
>> >> VHOST_USER_PROTOCOL_F_REPLY_ACK for
>> >> + details.
>> >Why need_response and not "need reply"?
>> (I’d already pointed this out earlier, but looks like I was possibly not
>> very clear.)
>> Before deciding on the right name for Bit 3, let us see the nomenclature for
>> Bit 2 above : "Bit 2 is the reply flag - needs to be sent on each reply from
>> the slave”.
>> So we already have a _reply_ flag in use. If the name Bit 3 as the
>> _need_reply_ flag, don’t you think it would be ultra-confusing ? I found it
>> confusing when I reviewed the documentation with this different term.
>> So I chose the name need_response with much deliberation — it conveys the
>> essence of what this flag means to achieve, but without adding to confusion.
>I don't see confusion, I think I agree with Marc André.
Allright. Posted a new series with the reworded terminology and updated (more
[Qemu-devel] [PATCH v4 2/2] vhost-user: Attempt to fix a race with set_mem_table., Prerna Saxena, 2016/07/27