[Qemu-devel] [RFC v2 10/32] vhub: Support sending fds back to qemu

Dr. David Alan Gilbert (git) posted 32 patches 8 years, 5 months ago
There is a newer version of this series
[Qemu-devel] [RFC v2 10/32] vhub: Support sending fds back to qemu
Posted by Dr. David Alan Gilbert (git) 8 years, 5 months ago
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Allow replies with fds (for postcopy)

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 contrib/libvhost-user/libvhost-user.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index 8bbdf5fb40..47884c0a15 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -213,6 +213,30 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
 {
     int rc;
     uint8_t *p = (uint8_t *)vmsg;
+    char control[CMSG_SPACE(VHOST_MEMORY_MAX_NREGIONS * sizeof(int))] = { };
+    struct iovec iov = {
+        .iov_base = (char *)vmsg,
+        .iov_len = VHOST_USER_HDR_SIZE,
+    };
+    struct msghdr msg = {
+        .msg_iov = &iov,
+        .msg_iovlen = 1,
+        .msg_control = control,
+    };
+    struct cmsghdr *cmsg;
+
+    memset(control, 0, sizeof(control));
+    if (vmsg->fds) {
+        size_t fdsize = vmsg->fd_num * sizeof(int);
+        msg.msg_controllen = CMSG_SPACE(fdsize);
+        cmsg = CMSG_FIRSTHDR(&msg);
+        cmsg->cmsg_len = CMSG_LEN(fdsize);
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        memcpy(CMSG_DATA(cmsg), vmsg->fds, fdsize);
+    } else {
+        msg.msg_controllen = 0;
+    }
 
     /* Set the version in the flags when sending the reply */
     vmsg->flags &= ~VHOST_USER_VERSION_MASK;
@@ -220,7 +244,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
     vmsg->flags |= VHOST_USER_REPLY_MASK;
 
     do {
-        rc = write(conn_fd, p, VHOST_USER_HDR_SIZE);
+        rc = sendmsg(conn_fd, &msg, 0);
     } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
 
     do {
@@ -313,6 +337,7 @@ vu_get_features_exec(VuDev *dev, VhostUserMsg *vmsg)
     }
 
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     DPRINT("Sending back to guest u64: 0x%016"PRIx64"\n", vmsg->payload.u64);
 
@@ -454,6 +479,7 @@ vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg)
     dev->log_size = log_mmap_size;
 
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     return true;
 }
@@ -698,6 +724,7 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
 
     vmsg->payload.u64 = features;
     vmsg->size = sizeof(vmsg->payload.u64);
+    vmsg->fd_num = 0;
 
     return true;
 }
-- 
2.13.5


Re: [Qemu-devel] [RFC v2 10/32] vhub: Support sending fds back to qemu
Posted by Marc-André Lureau 8 years, 5 months ago
Hi

On Thu, Aug 24, 2017 at 12:27 PM, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Allow replies with fds (for postcopy)
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  contrib/libvhost-user/libvhost-user.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
> index 8bbdf5fb40..47884c0a15 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -213,6 +213,30 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
>  {
>      int rc;
>      uint8_t *p = (uint8_t *)vmsg;
> +    char control[CMSG_SPACE(VHOST_MEMORY_MAX_NREGIONS * sizeof(int))] = { };
> +    struct iovec iov = {
> +        .iov_base = (char *)vmsg,
> +        .iov_len = VHOST_USER_HDR_SIZE,
> +    };
> +    struct msghdr msg = {
> +        .msg_iov = &iov,
> +        .msg_iovlen = 1,
> +        .msg_control = control,
> +    };
> +    struct cmsghdr *cmsg;
> +
> +    memset(control, 0, sizeof(control));
> +    if (vmsg->fds) {

This is going to be always true, right? Check vmsg->fd_num > 0 instead?

I would also add check or assert(vmsg->fd_num <= VHOST_MEMORY_MAX_NREGIONS)

> +        size_t fdsize = vmsg->fd_num * sizeof(int);
> +        msg.msg_controllen = CMSG_SPACE(fdsize);
> +        cmsg = CMSG_FIRSTHDR(&msg);
> +        cmsg->cmsg_len = CMSG_LEN(fdsize);
> +        cmsg->cmsg_level = SOL_SOCKET;
> +        cmsg->cmsg_type = SCM_RIGHTS;
> +        memcpy(CMSG_DATA(cmsg), vmsg->fds, fdsize);
> +    } else {
> +        msg.msg_controllen = 0;
> +    }
>
>      /* Set the version in the flags when sending the reply */
>      vmsg->flags &= ~VHOST_USER_VERSION_MASK;
> @@ -220,7 +244,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
>      vmsg->flags |= VHOST_USER_REPLY_MASK;
>
>      do {
> -        rc = write(conn_fd, p, VHOST_USER_HDR_SIZE);
> +        rc = sendmsg(conn_fd, &msg, 0);
>      } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>
>      do {
> @@ -313,6 +337,7 @@ vu_get_features_exec(VuDev *dev, VhostUserMsg *vmsg)
>      }
>
>      vmsg->size = sizeof(vmsg->payload.u64);
> +    vmsg->fd_num = 0;
>
>      DPRINT("Sending back to guest u64: 0x%016"PRIx64"\n", vmsg->payload.u64);
>
> @@ -454,6 +479,7 @@ vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg)
>      dev->log_size = log_mmap_size;
>
>      vmsg->size = sizeof(vmsg->payload.u64);
> +    vmsg->fd_num = 0;
>
>      return true;
>  }
> @@ -698,6 +724,7 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
>
>      vmsg->payload.u64 = features;
>      vmsg->size = sizeof(vmsg->payload.u64);
> +    vmsg->fd_num = 0;
>
>      return true;
>  }
> --
> 2.13.5
>
>

other than that
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



-- 
Marc-André Lureau

Re: [Qemu-devel] [RFC v2 10/32] vhub: Support sending fds back to qemu
Posted by Dr. David Alan Gilbert 8 years, 5 months ago
* Marc-André Lureau (marcandre.lureau@gmail.com) wrote:
> Hi
> 
> On Thu, Aug 24, 2017 at 12:27 PM, Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Allow replies with fds (for postcopy)
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  contrib/libvhost-user/libvhost-user.c | 29 ++++++++++++++++++++++++++++-
> >  1 file changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
> > index 8bbdf5fb40..47884c0a15 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -213,6 +213,30 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
> >  {
> >      int rc;
> >      uint8_t *p = (uint8_t *)vmsg;
> > +    char control[CMSG_SPACE(VHOST_MEMORY_MAX_NREGIONS * sizeof(int))] = { };
> > +    struct iovec iov = {
> > +        .iov_base = (char *)vmsg,
> > +        .iov_len = VHOST_USER_HDR_SIZE,
> > +    };
> > +    struct msghdr msg = {
> > +        .msg_iov = &iov,
> > +        .msg_iovlen = 1,
> > +        .msg_control = control,
> > +    };
> > +    struct cmsghdr *cmsg;
> > +
> > +    memset(control, 0, sizeof(control));
> > +    if (vmsg->fds) {
> 
> This is going to be always true, right? Check vmsg->fd_num > 0 instead?

Ah yes, thanks.

> I would also add check or assert(vmsg->fd_num <= VHOST_MEMORY_MAX_NREGIONS)

-    if (vmsg->fds) {
+    assert(vmsg->fd_num <= VHOST_MEMORY_MAX_NREGIONS);
+    if (vmsg->fd_num > 0) {

> 
> > +        size_t fdsize = vmsg->fd_num * sizeof(int);
> > +        msg.msg_controllen = CMSG_SPACE(fdsize);
> > +        cmsg = CMSG_FIRSTHDR(&msg);
> > +        cmsg->cmsg_len = CMSG_LEN(fdsize);
> > +        cmsg->cmsg_level = SOL_SOCKET;
> > +        cmsg->cmsg_type = SCM_RIGHTS;
> > +        memcpy(CMSG_DATA(cmsg), vmsg->fds, fdsize);
> > +    } else {
> > +        msg.msg_controllen = 0;
> > +    }
> >
> >      /* Set the version in the flags when sending the reply */
> >      vmsg->flags &= ~VHOST_USER_VERSION_MASK;
> > @@ -220,7 +244,7 @@ vu_message_write(VuDev *dev, int conn_fd, VhostUserMsg *vmsg)
> >      vmsg->flags |= VHOST_USER_REPLY_MASK;
> >
> >      do {
> > -        rc = write(conn_fd, p, VHOST_USER_HDR_SIZE);
> > +        rc = sendmsg(conn_fd, &msg, 0);
> >      } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> >
> >      do {
> > @@ -313,6 +337,7 @@ vu_get_features_exec(VuDev *dev, VhostUserMsg *vmsg)
> >      }
> >
> >      vmsg->size = sizeof(vmsg->payload.u64);
> > +    vmsg->fd_num = 0;
> >
> >      DPRINT("Sending back to guest u64: 0x%016"PRIx64"\n", vmsg->payload.u64);
> >
> > @@ -454,6 +479,7 @@ vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg)
> >      dev->log_size = log_mmap_size;
> >
> >      vmsg->size = sizeof(vmsg->payload.u64);
> > +    vmsg->fd_num = 0;
> >
> >      return true;
> >  }
> > @@ -698,6 +724,7 @@ vu_get_protocol_features_exec(VuDev *dev, VhostUserMsg *vmsg)
> >
> >      vmsg->payload.u64 = features;
> >      vmsg->size = sizeof(vmsg->payload.u64);
> > +    vmsg->fd_num = 0;
> >
> >      return true;
> >  }
> > --
> > 2.13.5
> >
> >
> 
> other than that
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks.

Dave

> 
> 
> -- 
> Marc-André Lureau
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK