Re: [Qemu-devel] [PATCH 2/2] vhost-user: back SET/GET_CONFIG requests with a protocol feature

[Michael S. Tsirkin]

On Wed, Mar 28, 2018 at 05:56:57PM +0200, Maxime Coquelin wrote:
> 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. 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>

I presume vhost user blk should fail init if the
protocol feature isn't negotiated then.

> ---
>  docs/interop/vhost-user.txt | 21 ++++++++++++---------
>  hw/virtio/vhost-user.c      | 17 +++++++++++++++++
>  2 files changed, 29 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..a045203b26 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,12 @@ 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);
> +        }
> +
>          err = vhost_user_set_protocol_features(dev, dev->protocol_features);
>          if (err < 0) {
>              return err;
> @@ -1405,6 +1412,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 +1460,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