[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: |
Liu, Changpeng |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] vhost-user: back SET/GET_CONFIG requests with a protocol feature |
Date: |
Thu, 29 Mar 2018 00:57:13 +0000 |
> -----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>
> ---
> 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