Re: [PATCH v2 4/4] spec/vhost-user: add VHOST_USER_PROTOCOL_F_VERSATILE_SLAVE

[Marc-André Lureau <marcandre.lureau@gmail.com>]

Hi

On Thu, Nov 24, 2016 at 7:20 AM Wei Wang <wei.w.wang@intel.com> 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 <wei.w.wang@intel.com>
---
 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

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

 

+      This request should be sent only when VHOST_USER_PROTOCOL_F_VERSATILE_SLAVE
+      has been negotiated.
+
 VHOST_USER_PROTOCOL_F_REPLY_ACK:
 -------------------------------
 The original vhost-user specification only demands replies for certain
--
2.7.4

--

Marc-André Lureau