qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 2/2] vhost-user: back SET/GET_CONFIG requests


From: Maxime Coquelin
Subject: Re: [Qemu-devel] [PATCH v2 2/2] vhost-user: back SET/GET_CONFIG requests with a protocol feature
Date: Thu, 29 Mar 2018 09:55:36 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0



On 03/29/2018 02:57 AM, Liu, Changpeng wrote:


-----Original Message-----
From: Maxime Coquelin [mailto:address@hidden
Sent: Thursday, March 29, 2018 3:28 AM
To: address@hidden; Liu, Changpeng <address@hidden>;
address@hidden; address@hidden
Cc: Maxime Coquelin <address@hidden>
Subject: [PATCH v2 2/2] vhost-user: back SET/GET_CONFIG requests with a
protocol feature

Without a dedicated protocol feature, QEMU cannot know whether
the backend can handle VHOST_USER_SET_CONFIG and
VHOST_USER_GET_CONFIG messages.

This patch adds a protocol feature that is only advertised by
QEMU if the device implements the config ops. Vhost user init
fails if the device support the feature but the backend doesn't.

The backend should only send VHOST_USER_SLAVE_CONFIG_CHANGE_MSG
requests if the protocol feature has been negotiated.

Signed-off-by: Maxime Coquelin <address@hidden>

Passed our own vhost-user-blk target with the patch, I can submit a fix to QEMU 
vhost-user-blk example
after this commit.

Tested-by: Changpeng Liu <address@hidden>

Thanks for having tested, and for proposing to implement the example
part. Just notice I forgot to apply your Tested-by on the series.

Maxime
---
  docs/interop/vhost-user.txt | 21 ++++++++++++---------
  hw/virtio/vhost-user.c      | 22 ++++++++++++++++++++++
  2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index c058c407df..534caab18a 100644
--- a/docs/interop/vhost-user.txt
+++ b/docs/interop/vhost-user.txt
@@ -379,6 +379,7 @@ Protocol features
  #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
  #define VHOST_USER_PROTOCOL_F_CRYPTO_SESSION 7
  #define VHOST_USER_PROTOCOL_F_PAGEFAULT      8
+#define VHOST_USER_PROTOCOL_F_CONFIG         9

  Master message types
  --------------------
@@ -664,7 +665,8 @@ Master message types
        Master payload: virtio device config space
        Slave payload: virtio device config space

-      Submitted by the vhost-user master to fetch the contents of the virtio
+      When VHOST_USER_PROTOCOL_F_CONFIG is negotiated, this message is
+      submitted by the vhost-user master to fetch the contents of the virtio
        device configuration space, vhost-user slave's payload size MUST match
        master's request, vhost-user slave uses zero length of payload to
        indicate an error to vhost-user master. The vhost-user master may
@@ -677,7 +679,8 @@ Master message types
        Master payload: virtio device config space
        Slave payload: N/A

-      Submitted by the vhost-user master when the Guest changes the virtio
+      When VHOST_USER_PROTOCOL_F_CONFIG is negotiated, this message is
+      submitted by the vhost-user master when the Guest changes the virtio
        device configuration space and also can be used for live migration
        on the destination host. The vhost-user slave must check the flags
        field, and slaves MUST NOT accept SET_CONFIG for read-only
@@ -766,13 +769,13 @@ Slave message types
       Slave payload: N/A
       Master payload: N/A

-     Vhost-user slave sends such messages to notify that the virtio device's
-     configuration space has changed, for those host devices which can support
-     such feature, host driver can send VHOST_USER_GET_CONFIG message to
slave
-     to get the latest content. If VHOST_USER_PROTOCOL_F_REPLY_ACK is
-     negotiated, and slave set the VHOST_USER_NEED_REPLY flag, master must
-     respond with zero when operation is successfully completed, or non-zero
-     otherwise.
+     When VHOST_USER_PROTOCOL_F_CONFIG is negotiated, vhost-user slave
sends
+     such messages to notify that the virtio device's configuration space has
+     changed, for those host devices which can support such feature, host
+     driver can send VHOST_USER_GET_CONFIG message to slave to get the latest
+     content. If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, and slave
set
+     the VHOST_USER_NEED_REPLY flag, master must respond with zero when
+     operation is successfully completed, or non-zero otherwise.

  VHOST_USER_PROTOCOL_F_REPLY_ACK:
  -------------------------------
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 44aea5c0a8..cc8a24aa31 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -46,6 +46,7 @@ enum VhostUserProtocolFeature {
      VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
      VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7,
      VHOST_USER_PROTOCOL_F_PAGEFAULT = 8,
+    VHOST_USER_PROTOCOL_F_CONFIG = 9,
      VHOST_USER_PROTOCOL_F_MAX
  };

@@ -1211,6 +1212,17 @@ static int vhost_user_init(struct vhost_dev *dev, void
*opaque)

          dev->protocol_features =
              protocol_features & VHOST_USER_PROTOCOL_FEATURE_MASK;
+
+        if (!dev->config_ops || !dev->config_ops->vhost_dev_config_notifier) {
+            /* Dont acknowledge CONFIG feature if device doesn't support it */
+            dev->protocol_features &= ~(1ULL <<
VHOST_USER_PROTOCOL_F_CONFIG);
+        } else if (!(protocol_features &
+                    (1ULL << VHOST_USER_PROTOCOL_F_CONFIG))) {
+            error_report("Device expects VHOST_USER_PROTOCOL_F_CONFIG "
+                    "but backend does not support it.");
+            return -1;
+        }
+
          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
          if (err < 0) {
              return err;
@@ -1405,6 +1417,11 @@ static int vhost_user_get_config(struct vhost_dev
*dev, uint8_t *config,
          .hdr.size = VHOST_USER_CONFIG_HDR_SIZE + config_len,
      };

+    if (!virtio_has_feature(dev->protocol_features,
+                VHOST_USER_PROTOCOL_F_CONFIG)) {
+        return -1;
+    }
+
      if (config_len > VHOST_USER_MAX_CONFIG_SIZE) {
          return -1;
      }
@@ -1448,6 +1465,11 @@ static int vhost_user_set_config(struct vhost_dev
*dev, const uint8_t *data,
          .hdr.size = VHOST_USER_CONFIG_HDR_SIZE + size,
      };

+    if (!virtio_has_feature(dev->protocol_features,
+                VHOST_USER_PROTOCOL_F_CONFIG)) {
+        return -1;
+    }
+
      if (reply_supported) {
          msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
      }
--
2.14.3




reply via email to

[Prev in Thread] Current Thread [Next in Thread]