Re: [Qemu-devel] [PATCH v2 5/6] vhost-user: add slave-req-fd support

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, May 26, 2017 at 04:28:57PM +0200, Maxime Coquelin wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Learn to give a socket to the slave to let him make requests to the
> master.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  docs/specs/vhost-user.txt |  32 +++++++++++-
>  hw/virtio/vhost-user.c    | 127 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 157 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index 036890f..5fa7016 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -139,6 +139,7 @@ in the ancillary data:
>   * VHOST_USER_SET_VRING_KICK
>   * VHOST_USER_SET_VRING_CALL
>   * VHOST_USER_SET_VRING_ERR
> + * VHOST_USER_SET_SLAVE_REQ_FD
>  
>  If Master is unable to send the full message or receives a wrong reply it will
>  close the connection. An optional reconnection mechanism can be implemented.
> @@ -252,6 +253,18 @@ Once the source has finished migration, rings will be stopped by
>  the source. No further update must be done before rings are
>  restarted.
>  
> +Slave communication
> +-------------------
> +
> +An optional communication channel is provided if the slave declares
> +VHOST_USER_PROTOCOL_F_SLAVE_REQ protocol feature, to allow the slave to make
> +requests to the master.
> +
> +The fd is provided via VHOST_USER_SET_SLAVE_REQ_FD ancillary data.
> +
> +A slave may then send VHOST_USER_SLAVE_* messages to the master
> +using this fd communication channel.
> +
>  Protocol features
>  -----------------
>  
> @@ -260,9 +273,10 @@ Protocol features
>  #define VHOST_USER_PROTOCOL_F_RARP           2
>  #define VHOST_USER_PROTOCOL_F_REPLY_ACK      3
>  #define VHOST_USER_PROTOCOL_F_MTU            4
> +#define VHOST_USER_PROTOCOL_F_SLAVE_REQ      5
>  
> -Message types
> --------------
> +Master message types
> +--------------------
>  
>   * VHOST_USER_GET_FEATURES
>  


So down the road, we should make sure a device without
VHOST_USER_PROTOCOL_F_SLAVE_REQ does not advertise IOMMU
since you don't handle these messages otherwise.


> @@ -486,6 +500,20 @@ Message types
>        If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
>        with zero in case the specified MTU is valid, or non-zero otherwise.
>  
> + * VHOST_USER_SET_SLAVE_REQ_FD
> +
> +      Id: 21
> +      Equivalent ioctl: N/A
> +      Master payload: N/A
> +
> +      Set the socket file descriptor for slave initiated requests. It is passed
> +      in the ancillary data.
> +      This request should be sent only when VHOST_USER_F_PROTOCOL_FEATURES
> +      has been negotiated, and protocol feature bit VHOST_USER_PROTOCOL_F_SLAVE_REQ
> +      bit is present in VHOST_USER_GET_PROTOCOL_FEATURES.
> +      If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must respond
> +      with zero for success, non-zero otherwise.
> +
>  VHOST_USER_PROTOCOL_F_REPLY_ACK:
>  -------------------------------
>  The original vhost-user specification only demands replies for certain
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 8a602e0..ea988fe 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -32,6 +32,7 @@ enum VhostUserProtocolFeature {
>      VHOST_USER_PROTOCOL_F_RARP = 2,
>      VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
>      VHOST_USER_PROTOCOL_F_NET_MTU = 4,
> +    VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
>  
>      VHOST_USER_PROTOCOL_F_MAX
>  };
> @@ -60,9 +61,15 @@ typedef enum VhostUserRequest {
>      VHOST_USER_SET_VRING_ENABLE = 18,
>      VHOST_USER_SEND_RARP = 19,
>      VHOST_USER_NET_SET_MTU = 20,
> +    VHOST_USER_SET_SLAVE_REQ_FD = 21,
>      VHOST_USER_MAX
>  } VhostUserRequest;
>  
> +typedef enum VhostUserSlaveRequest {
> +    VHOST_USER_SLAVE_NONE = 0,
> +    VHOST_USER_SLAVE_MAX
> +}  VhostUserSlaveRequest;
> +
>  typedef struct VhostUserMemoryRegion {
>      uint64_t guest_phys_addr;
>      uint64_t memory_size;
> @@ -112,6 +119,7 @@ static VhostUserMsg m __attribute__ ((unused));
>  
>  struct vhost_user {
>      CharBackend *chr;
> +    int slave_fd;
>  };
>  
>  static bool ioeventfd_enabled(void)
> @@ -578,6 +586,115 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
>      return 0;
>  }
>  
> +static void slave_read(void *opaque)
> +{
> +    struct vhost_dev *dev = opaque;
> +    struct vhost_user *u = dev->opaque;
> +    VhostUserMsg msg = { 0, };
> +    int size, ret = 0;
> +
> +    /* Read header */
> +    size = read(u->slave_fd, &msg, VHOST_USER_HDR_SIZE);
> +    if (size != VHOST_USER_HDR_SIZE) {
> +        error_report("Failed to read from slave.");
> +        goto err;
> +    }
> +
> +    if (msg.size > VHOST_USER_PAYLOAD_SIZE) {
> +        error_report("Failed to read msg header."
> +                " Size %d exceeds the maximum %zu.", msg.size,
> +                VHOST_USER_PAYLOAD_SIZE);
> +        goto err;
> +    }
> +
> +    /* Read payload */
> +    size = read(u->slave_fd, &msg.payload, msg.size);
> +    if (size != msg.size) {
> +        error_report("Failed to read payload from slave.");
> +        goto err;
> +    }
> +
> +    switch (msg.request) {
> +    default:
> +        error_report("Received unexpected msg type.");
> +        ret = -EINVAL;
> +    }
> +
> +    /*
> +     * REPLY_ACK feature handling. Other reply types has to be managed
> +     * directly in their request handlers.
> +     */
> +    if (msg.flags & VHOST_USER_NEED_REPLY_MASK) {
> +        msg.flags &= ~VHOST_USER_NEED_REPLY_MASK;
> +        msg.flags |= VHOST_USER_REPLY_MASK;
> +
> +        msg.payload.u64 = !!ret;
> +        msg.size = sizeof(msg.payload.u64);
> +
> +        size = write(u->slave_fd, &msg, VHOST_USER_HDR_SIZE + msg.size);
> +        if (size != VHOST_USER_HDR_SIZE + msg.size) {
> +            error_report("Failed to send msg reply to slave.");
> +            goto err;
> +        }
> +    }
> +
> +    return;
> +
> +err:
> +    qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL);
> +    close(u->slave_fd);
> +    u->slave_fd = -1;
> +    return;
> +}
> +
> +static int vhost_setup_slave_channel(struct vhost_dev *dev)
> +{
> +    VhostUserMsg msg = {
> +        .request = VHOST_USER_SET_SLAVE_REQ_FD,
> +        .flags = VHOST_USER_VERSION,
> +    };
> +    struct vhost_user *u = dev->opaque;
> +    int sv[2], ret = 0;
> +    bool reply_supported = virtio_has_feature(dev->protocol_features,
> +                                              VHOST_USER_PROTOCOL_F_REPLY_ACK);
> +
> +    if (!virtio_has_feature(dev->protocol_features,
> +                            VHOST_USER_PROTOCOL_F_SLAVE_REQ)) {
> +        return 0;
> +    }
> +
> +    if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
> +        error_report("socketpair() failed");
> +        return -1;
> +    }
> +
> +    u->slave_fd = sv[0];
> +    qemu_set_fd_handler(u->slave_fd, slave_read, NULL, dev);
> +
> +    if (reply_supported) {
> +        msg.flags |= VHOST_USER_NEED_REPLY_MASK;
> +    }
> +
> +    ret = vhost_user_write(dev, &msg, &sv[1], 1);
> +    if (ret) {
> +        goto out;
> +    }
> +
> +    if (reply_supported) {
> +        ret = process_message_reply(dev, msg);
> +    }
> +
> +out:
> +    close(sv[1]);
> +    if (ret) {
> +        qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL);
> +        close(u->slave_fd);
> +        u->slave_fd = -1;
> +    }
> +
> +    return ret;
> +}
> +
>  static int vhost_user_init(struct vhost_dev *dev, void *opaque)
>  {
>      uint64_t features;
> @@ -588,6 +705,7 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
>  
>      u = g_new0(struct vhost_user, 1);
>      u->chr = opaque;
> +    u->slave_fd = -1;
>      dev->opaque = u;
>  
>      err = vhost_user_get_features(dev, &features);
> @@ -628,6 +746,11 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque)
>                     "VHOST_USER_PROTOCOL_F_LOG_SHMFD feature.");
>      }
>  
> +    err = vhost_setup_slave_channel(dev);
> +    if (err < 0) {
> +        return err;
> +    }
> +
>      return 0;
>  }
>  
> @@ -638,6 +761,10 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
>      assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
>  
>      u = dev->opaque;
> +    if (u->slave_fd >= 0) {
> +        close(u->slave_fd);
> +        u->slave_fd = -1;
> +    }
>      g_free(u);
>      dev->opaque = 0;
>  
> -- 
> 2.9.4