Re: [Qemu-devel] [PATCH v6 08/12] vhost-user: add vhost_user_input_get_config()

[Michael S. Tsirkin]

On Fri, Mar 08, 2019 at 03:04:50PM +0100, Marc-André Lureau wrote:
> Ask vhost user input backend the list of virtio_input_config.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> Reviewed-by: Gerd Hoffmann <address@hidden>

I was pushing this and I am puzzled now.


> ---
>  contrib/libvhost-user/libvhost-user.h |  1 +
>  include/hw/virtio/vhost-backend.h     |  4 ++
>  hw/virtio/vhost-user.c                | 60 +++++++++++++++++++++++++++
>  docs/interop/vhost-user.txt           |  8 ++++
>  4 files changed, 73 insertions(+)
> 
> diff --git a/contrib/libvhost-user/libvhost-user.h 
> b/contrib/libvhost-user/libvhost-user.h
> index c0133b7f3f..b0c798fa1a 100644
> --- a/contrib/libvhost-user/libvhost-user.h
> +++ b/contrib/libvhost-user/libvhost-user.h
> @@ -91,6 +91,7 @@ typedef enum VhostUserRequest {
>      VHOST_USER_POSTCOPY_ADVISE  = 28,
>      VHOST_USER_POSTCOPY_LISTEN  = 29,
>      VHOST_USER_POSTCOPY_END     = 30,
> +    VHOST_USER_INPUT_GET_CONFIG = 31,
>      VHOST_USER_MAX
>  } VhostUserRequest;
>  
> diff --git a/include/hw/virtio/vhost-backend.h 
> b/include/hw/virtio/vhost-backend.h
> index 81283ec50f..1fca321d8a 100644
> --- a/include/hw/virtio/vhost-backend.h
> +++ b/include/hw/virtio/vhost-backend.h
> @@ -12,6 +12,7 @@
>  #define VHOST_BACKEND_H
>  
>  #include "exec/memory.h"
> +#include "standard-headers/linux/virtio_input.h"
>  
>  typedef enum VhostBackendType {
>      VHOST_BACKEND_TYPE_NONE = 0,
> @@ -160,4 +161,7 @@ int vhost_backend_invalidate_device_iotlb(struct 
> vhost_dev *dev,
>  int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
>                                            struct vhost_iotlb_msg *imsg);
>  
> +int vhost_user_input_get_config(struct vhost_dev *dev,
> +                                struct virtio_input_config **config);
> +
>  #endif /* VHOST_BACKEND_H */
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 5df73405bc..cf3fd39035 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -93,6 +93,7 @@ typedef enum VhostUserRequest {
>      VHOST_USER_POSTCOPY_ADVISE  = 28,
>      VHOST_USER_POSTCOPY_LISTEN  = 29,
>      VHOST_USER_POSTCOPY_END     = 30,
> +    VHOST_USER_INPUT_GET_CONFIG = 31,
>      VHOST_USER_MAX
>  } VhostUserRequest;
>  
> @@ -342,6 +343,65 @@ static int vhost_user_write(struct vhost_dev *dev, 
> VhostUserMsg *msg,
>      return 0;
>  }
>  
> +static void *vhost_user_read_size(struct vhost_dev *dev, uint32_t size)
> +{
> +    struct vhost_user *u = dev->opaque;
> +    CharBackend *chr = u->user->chr;
> +    int r;
> +    uint8_t *p = g_malloc(size);
> +
> +    r = qemu_chr_fe_read_all(chr, p, size);
> +    if (r != size) {
> +        error_report("Failed to read msg payload."
> +                     " Read %d instead of %u.", r, size);
> +        g_free(p);
> +        return NULL;
> +    }
> +
> +    return p;
> +}
> +
> +int vhost_user_input_get_config(struct vhost_dev *dev,
> +                                struct virtio_input_config **config)
> +{
> +    void *p = NULL;
> +    VhostUserMsg msg = {
> +        .hdr.request = VHOST_USER_INPUT_GET_CONFIG,
> +        .hdr.flags = VHOST_USER_VERSION,
> +    };
> +
> +    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> +        goto err;
> +    }
> +
> +    if (vhost_user_read_header(dev, &msg) < 0) {
> +        goto err;
> +    }
> +
> +    if (msg.hdr.request != VHOST_USER_INPUT_GET_CONFIG) {
> +        error_report("Received unexpected msg type. Expected %d received %d",
> +                     VHOST_USER_INPUT_GET_CONFIG, msg.hdr.request);
> +        goto err;
> +    }
> +
> +    if (msg.hdr.size % sizeof(struct virtio_input_config)) {
> +        error_report("Invalid msg size");
> +        goto err;
> +    }
> +
> +    p = vhost_user_read_size(dev, msg.hdr.size);
> +    if (!p) {
> +        goto err;
> +    }
> +
> +    *config = p;
> +    return msg.hdr.size / sizeof(struct virtio_input_config);
> +
> +err:
> +    g_free(p);
> +    return -1;
> +}
> +
>  static int vhost_user_set_log_base(struct vhost_dev *dev, uint64_t base,
>                                     struct vhost_log *log)
>  {
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index 9ee2a60cfb..e145b3ec55 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -766,6 +766,14 @@ Master message types
>        was previously sent.
>        The value returned is an error indication; 0 is success.
>  
> + * VHOST_USER_INPUT_GET_CONFIG
> +      Id: 31
> +      Master payload: N/A
> +      Slave payload: (struct virtio_input_config)*
> +
> +      Ask vhost user input backend the list of virtio_input_config, in
> +      host endianness.
> +
>  Slave message types
>  -------------------
>  

Why do we need this? What is wrong with VHOST_USER_GET_CONFIG?
And why host endianness? Everything else is little endian ...

> -- 
> 2.21.0.4.g36eb1cb9cf