[Qemu-devel] [PATCH 1/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 containing a status code for each requested operation. Status codes are '0' for success, and non-zero for error.

[Prerna Saxena]

From: Prerna Saxena <address@hidden>

Signed-off-by: Prerna Saxena <address@hidden>
---
 docs/specs/vhost-user.txt |  36 +++++++++++
 hw/virtio/vhost-user.c    | 153 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 186 insertions(+), 3 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 777c49c..e5388b2 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -37,6 +37,7 @@ 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.
  * Size - 32-bit size of the payload
 
 
@@ -126,6 +127,8 @@ the ones that do:
  * VHOST_GET_VRING_BASE
  * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD)
 
+[ Also see the section on REPLY_ACK protocol extension]
+
 There are several messages that the master sends with file descriptors passed
 in the ancillary data:
 
@@ -254,6 +257,7 @@ 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
 
 Message types
 -------------
@@ -464,3 +468,35 @@ Message types
       is present in VHOST_USER_GET_PROTOCOL_FEATURES.
       The first 6 bytes of the payload contain the mac address of the guest to
       allow the vhost user backend to construct and broadcast the fake RARP.
+
+VHOST_USER_PROTOCOL_F_REPLY_ACK:
+--------------------------------
+The original vhost-user specification only demands responses for certain
+commands. This differs from the vhost protocol implementation where commands
+are sent over an ioctl() call and block until the client has completed. Not
+receiving a response for commands like VHOST_SET_MEM_TABLE makes the sender
+unable to tell when the client has finished (re)mapping the GPA, or whether it
+has failed altogether.
+
+With this protocol extension negotiated, the sender can set the newly
+introduced "need_response" [Bit 3] flag to any command. This indicates that
+the client MUST to respond with a Payload VhostUserMsg indicating success or
+failure. The payload should be set to zero on success or non-zero on failure.
+In other words, response must be in the following format :
+------------------------------------
+| request | flags | size | payload |
+------------------------------------
+
+ * Request: 32-bit type of the original request which is being responded to.
+ * Flags: 32-bit bit field: (VHOST_USER_VERSION | VHOST_USER_REPLY_MASK)
+ * Size: size of the payload ( see below)
+ * Payload : a u64 integer, where a non-zero value indicates a failure.
+
+Note that as per the original vhost-user protocol, the following four messages 
anyway
+require distinct responses from the vhost-user client process :
+ * VHOST_GET_FEATURES
+ * VHOST_GET_PROTOCOL_FEATURES
+ * VHOST_GET_VRING_BASE
+ * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD)
+For these message types, the presence of VHOST_USER_PROTOCOL_F_REPLY_ACK or
+need_response bit being set brings no behaviourial change.
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 495e09f..f01ebb4 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -31,6 +31,7 @@ enum VhostUserProtocolFeature {
     VHOST_USER_PROTOCOL_F_MQ = 0,
     VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
     VHOST_USER_PROTOCOL_F_RARP = 2,
+    VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
 
     VHOST_USER_PROTOCOL_F_MAX
 };
@@ -82,8 +83,9 @@ typedef struct VhostUserLog {
 typedef struct VhostUserMsg {
     VhostUserRequest request;
 
-#define VHOST_USER_VERSION_MASK     (0x3)
-#define VHOST_USER_REPLY_MASK       (0x1<<2)
+#define VHOST_USER_VERSION_MASK         (0x3)
+#define VHOST_USER_REPLY_MASK           (0x1 << 2)
+#define VHOST_USER_NEED_RESPONSE_MASK   (0x1 << 3)
     uint32_t flags;
     uint32_t size; /* the following payload size */
     union {
@@ -239,10 +241,17 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
     int fds[VHOST_MEMORY_MAX_NREGIONS];
     int i, fd;
     size_t fd_num = 0;
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                    VHOST_USER_PROTOCOL_F_REPLY_ACK);
     VhostUserMsg msg = {
         .request = VHOST_USER_SET_MEM_TABLE,
         .flags = VHOST_USER_VERSION,
     };
+    VhostUserRequest request = msg.request;
+
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_RESPONSE_MASK;
+    }
 
     for (i = 0; i < dev->mem->nregions; ++i) {
         struct vhost_memory_region *reg = dev->mem->regions + i;
@@ -277,6 +286,20 @@ static int vhost_user_set_mem_table(struct vhost_dev *dev,
 
     vhost_user_write(dev, &msg, fds, fd_num);
 
+    if (reply_supported) {
+        if (vhost_user_read(dev, &msg) < 0) {
+            return 0;
+        }
+
+        if (msg.request != request) {
+            error_report("Received unexpected msg type."
+                            "Expected %d received %d",
+                            request, msg.request);
+            return -1;
+        }
+        return msg.payload.u64 ? -1 : 0;
+    }
+
     return 0;
 }
 
@@ -289,9 +312,29 @@ static int vhost_user_set_vring_addr(struct vhost_dev *dev,
         .payload.addr = *addr,
         .size = sizeof(msg.payload.addr),
     };
+    VhostUserRequest request = msg.request;
+
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                    VHOST_USER_PROTOCOL_F_REPLY_ACK);
 
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_RESPONSE_MASK;
+    }
     vhost_user_write(dev, &msg, NULL, 0);
 
+    if (reply_supported) {
+        if (vhost_user_read(dev, &msg) < 0) {
+            return 0;
+        }
+
+        if (msg.request != request) {
+            error_report("Received unexpected msg type."
+                            "Expected %d received %d",
+                            request, msg.request);
+            return -1;
+        }
+        return msg.payload.u64 ? -1 : 0;
+    }
     return 0;
 }
 
@@ -313,8 +356,28 @@ static int vhost_set_vring(struct vhost_dev *dev,
         .size = sizeof(msg.payload.state),
     };
 
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                    VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_RESPONSE_MASK;
+    }
+
     vhost_user_write(dev, &msg, NULL, 0);
 
+    if (reply_supported) {
+        if (vhost_user_read(dev, &msg) < 0) {
+            return 0;
+        }
+
+        if (msg.request != request) {
+            error_report("Received unexpected msg type."
+                            "Expected %lu received %lu",
+                            request, msg.request);
+            return -1;
+        }
+        return msg.payload.u64 ? -1 : 0;
+    }
     return 0;
 }
 
@@ -395,6 +458,13 @@ static int vhost_set_vring_file(struct vhost_dev *dev,
         .size = sizeof(msg.payload.u64),
     };
 
+    bool reply_Supported = virtio_has_feature(dev->protocol_features,
+                                    VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_RESPONSE_MASK;
+    }
+
     if (ioeventfd_enabled() && file->fd > 0) {
         fds[fd_num++] = file->fd;
     } else {
@@ -403,6 +473,20 @@ static int vhost_set_vring_file(struct vhost_dev *dev,
 
     vhost_user_write(dev, &msg, fds, fd_num);
 
+    if (reply_supported) {
+        if (vhost_user_read(dev, &msg) < 0) {
+            return 0;
+        }
+
+        if (msg.request != request) {
+            error_report("Received unexpected msg type."
+                            "Expected %d received %d",
+                            request, msg.request);
+            return -1;
+        }
+        return msg.payload.u64 ? -1 : 0;
+    }
+
     return 0;
 }
 
@@ -489,8 +573,30 @@ static int vhost_user_set_owner(struct vhost_dev *dev)
         .flags = VHOST_USER_VERSION,
     };
 
+    VhostUserRequest request = msg.request;
+
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                    VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_RESPONSE_MASK;
+    }
+
     vhost_user_write(dev, &msg, NULL, 0);
 
+    if (reply_supported) {
+        if (vhost_user_read(dev, &msg) < 0) {
+            return 0;
+        }
+
+        if (msg.request != request) {
+            error_report("Received unexpected msg type."
+                             "Expected %d received %d",
+                             request, msg.request);
+            return -1;
+        }
+        return msg.payload.u64 ? -1 : 0;
+    }
     return 0;
 }
 
@@ -500,9 +606,30 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
         .request = VHOST_USER_RESET_OWNER,
         .flags = VHOST_USER_VERSION,
     };
+    VhostUserRequest request = msg.request;
+
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                    VHOST_USER_PROTOCOL_F_REPLY_ACK);
+
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_RESPONSE_MASK;
+    }
 
     vhost_user_write(dev, &msg, NULL, 0);
 
+    if (reply_suported) {
+        if (vhost_user_read(dev, &msg) < 0) {
+            return 0;
+        }
+
+        if (msg.request != request) {
+            error_report("Received unexpected msg type."
+                            "Expected %d received %d",
+                            request, msg.request);
+            return -1;
+        }
+        return msg.payload.u64 ? -1 : 0;
+    }
     return 0;
 }
 
@@ -589,9 +716,15 @@ static int vhost_user_migration_done(struct vhost_dev 
*dev, char* mac_addr)
 {
     VhostUserMsg msg = { 0 };
     int err;
+    VhostUserRequest request = msg.request;
+    bool reply_supported = virtio_has_feature(dev->protocol_features,
+                                    VHOST_USER_PROTOCOL_F_REPLY_ACK);
 
     assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
+    if (reply_supported) {
+        msg.flags |= VHOST_USER_NEED_RESPONSE_MASK;
+    }
     /* If guest supports GUEST_ANNOUNCE do nothing */
     if (virtio_has_feature(dev->acked_features, VIRTIO_NET_F_GUEST_ANNOUNCE)) {
         return 0;
@@ -606,7 +739,21 @@ static int vhost_user_migration_done(struct vhost_dev 
*dev, char* mac_addr)
         msg.size = sizeof(msg.payload.u64);
 
         err = vhost_user_write(dev, &msg, NULL, 0);
-        return err;
+        if (reply_supported) {
+            if (vhost_user_read(dev, &msg) < 0) {
+                return 0;
+            }
+
+            if (msg.request != request) {
+                error_report("Received unexpected msg type."
+                                "Expected %d received %d",
+                                request, msg.request);
+                return -1;
+            }
+            return msg.payload.u64 ? -1 : 0;
+        } else {
+            return err;
+        }
     }
     return -1;
 }
-- 
1.8.1.2