[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 3/4] vhost-user: add slave-req-fd support
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [RFC v2 3/4] vhost-user: add slave-req-fd support |
Date: |
Mon, 17 Apr 2017 11:29:58 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Fri, Apr 14, 2017 at 07:40:55PM +0200, Maxime Coquelin wrote:
[...]
> @@ -486,6 +500,18 @@ 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.
Here, do we need to mention REPLY_ACK as well? Like:
If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must
respond with zero in case the slave request channel is setup
correctly, or non-zero otherwise.
Since I see the other two users are mentioning it.
[...]
> +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];
> + 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;
> + }
> +
> + vhost_user_write(dev, &msg, &sv[1], 1);
Do we need to close(sv[1]) afterward?
> +
> + if (reply_supported) {
> + return process_message_reply(dev, msg.request);
Here do we need to cleanup u->slave_fd if backend replied something
wrong? Or I guess it might be leaked.
Thanks,
> + }
> +
> + return 0;
> +}
> +
--
Peter Xu