Re: [Qemu-devel] [virtio-comment] Re: [PATCH v2 4/4] spec/vhost-user: add VHOST_USER_PROTOCOL_F_VERSATILE_SLAVE

[Wei Wang]

On 11/28/2016 08:41 PM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Nov 24, 2016 at 7:20 AM Wei Wang <address@hidden 
> <mailto:address@hidden>> wrote:
>
>     The VHOST_USER_PROTOCOL_F_VERSATILE_SLAVE protocol feature indicates
>     that the slave side implementation supports different types of
>     devices.
>     The master tells the slave what type of device to create by sending
>     the VHOST_USER_SET_DEV_INFO message.
>
>     Signed-off-by: Wei Wang <address@hidden
>     <mailto:address@hidden>>
>     ---
>      docs/specs/vhost-user.txt | 21 ++++++++++++++++-----
>      1 file changed, 16 insertions(+), 5 deletions(-)
>
>     diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>     index fdc99ea..da1314d 100644
>     --- a/docs/specs/vhost-user.txt
>     +++ b/docs/specs/vhost-user.txt
>     @@ -264,11 +264,12 @@ restarted.
>      Protocol features
>      -----------------
>
>     -#define VHOST_USER_PROTOCOL_F_MQ             0
>     -#define VHOST_USER_PROTOCOL_F_LOG_SHMFD      1
>     -#define VHOST_USER_PROTOCOL_F_RARP           2
>     -#define VHOST_USER_PROTOCOL_F_REPLY_ACK      3
>     -#define VHOST_USER_PROTOCOL_F_VHOST_PCI      4
>     +#define VHOST_USER_PROTOCOL_F_MQ               0
>     +#define VHOST_USER_PROTOCOL_F_LOG_SHMFD        1
>     +#define VHOST_USER_PROTOCOL_F_RARP             2
>     +#define VHOST_USER_PROTOCOL_F_REPLY_ACK        3
>     +#define VHOST_USER_PROTOCOL_F_VHOST_PCI        4
>     +#define VHOST_USER_PROTOCOL_F_VERSATILE_SLAVE  5
>
>
>  I would rather name it after the message, 
> VHOST_USER_PROTOCOL_F_SET_DEV_INFO

OK, I will take this suggestion. Thanks. <*v2-AR1*>
>      Message types
>      -------------
>     @@ -514,6 +515,16 @@ Message types
>            #define VHOST_USER_SET_PEER_CONNECTION_F_CREATE 2
>            #define VHOST_USER_SET_PEER_CONNECTION_F_DESTROY  3
>
>     + * VHOST_USER_SET_DEV_INFO
>     +
>     +      Id: 21
>     +      Equivalent ioctl: N/A
>     +      Master payload: u64
>
>     +
>     +      The master sends the device type info to the slave.
>
>
> What is the meaning of this payload? Is it the virtio device id? 
> better be explicit about it.
>
> If it is the case, I would name the message "VHOST_USER_SET_DEVICE_ID".

Yes, currently we only have the virtio device id as the payload. I was 
thinking that in the future we would have other more info about the 
device. But no problem, I will change to use "VHOST_USER_SET_DEVICE_ID" 
for now. <*v2-AR2*>


Best,
Wei