diff mbox series

[PULL,v2,31/50] vhost+postcopy: Register shared ufd with postcopy

Message ID 1521515720-612046-32-git-send-email-mst@redhat.com
State New
Headers show
Series [PULL,v2,01/50] scripts/update-linux-headers: add ethtool.h and update to 4.16.0-rc4 | expand

Commit Message

Michael S. Tsirkin March 20, 2018, 3:17 a.m. UTC
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Register the UFD that comes in as the response to the 'advise' method
with the postcopy code.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/vhost-user.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Comments

Peter Maydell April 27, 2018, 4:12 p.m. UTC | #1
On 20 March 2018 at 03:17, Michael S. Tsirkin <mst@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Register the UFD that comes in as the response to the 'advise' method
> with the postcopy code.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/virtio/vhost-user.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)

> @@ -835,8 +847,14 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
>          error_setg(errp, "%s: Failed to get ufd", __func__);
>          return -1;
>      }
> +    fcntl(ufd, F_SETFL, O_NONBLOCK);

Hi; this would probably be more neatly done with
       qemu_set_nonblock(ufd);
unless you really wanted to clear the other fd flags.
Among other things, it avoids Coverity producing a complaint
that we didn't check the fcntl return value (though we seem
to assume it can't fail in general, hence qemu_set_nonblock()
returning NULL.) -- CID1390601, which I've marked as false-positive.

> -    /* TODO: register ufd with userfault thread */
> +    /* register ufd with userfault thread */
> +    u->postcopy_fd.fd = ufd;
> +    u->postcopy_fd.data = dev;
> +    u->postcopy_fd.handler = vhost_user_postcopy_fault_handler;
> +    u->postcopy_fd.idstr = "vhost-user"; /* Need to find unique name */
> +    postcopy_register_shared_ufd(&u->postcopy_fd);
>      return 0;
>  }

thanks
-- PMM
Dr. David Alan Gilbert May 2, 2018, 10:58 a.m. UTC | #2
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 20 March 2018 at 03:17, Michael S. Tsirkin <mst@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Register the UFD that comes in as the response to the 'advise' method
> > with the postcopy code.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/virtio/vhost-user.c | 20 +++++++++++++++++++-
> >  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> > @@ -835,8 +847,14 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
> >          error_setg(errp, "%s: Failed to get ufd", __func__);
> >          return -1;
> >      }
> > +    fcntl(ufd, F_SETFL, O_NONBLOCK);
> 
> Hi; this would probably be more neatly done with
>        qemu_set_nonblock(ufd);
> unless you really wanted to clear the other fd flags.
> Among other things, it avoids Coverity producing a complaint
> that we didn't check the fcntl return value (though we seem
> to assume it can't fail in general, hence qemu_set_nonblock()
> returning NULL.) -- CID1390601, which I've marked as false-positive.

Fix posted.  To be honest, I probably hadn't realised/forgot that
this would nuke all the other flags.  I bet some of the others uses
are the same, and may be losing important flags like noexec.

Dave

> > -    /* TODO: register ufd with userfault thread */
> > +    /* register ufd with userfault thread */
> > +    u->postcopy_fd.fd = ufd;
> > +    u->postcopy_fd.data = dev;
> > +    u->postcopy_fd.handler = vhost_user_postcopy_fault_handler;
> > +    u->postcopy_fd.idstr = "vhost-user"; /* Need to find unique name */
> > +    postcopy_register_shared_ufd(&u->postcopy_fd);
> >      return 0;
> >  }
> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index ceb17b0..5900583 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -171,6 +171,7 @@  struct vhost_user {
     CharBackend *chr;
     int slave_fd;
     NotifierWithReturn postcopy_notifier;
+    struct PostCopyFD  postcopy_fd;
 };
 
 static bool ioeventfd_enabled(void)
@@ -797,6 +798,17 @@  out:
 }
 
 /*
+ * Called back from the postcopy fault thread when a fault is received on our
+ * ufd.
+ * TODO: This is Linux specific
+ */
+static int vhost_user_postcopy_fault_handler(struct PostCopyFD *pcfd,
+                                             void *ufd)
+{
+    return 0;
+}
+
+/*
  * Called at the start of an inbound postcopy on reception of the
  * 'advise' command.
  */
@@ -835,8 +847,14 @@  static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
         error_setg(errp, "%s: Failed to get ufd", __func__);
         return -1;
     }
+    fcntl(ufd, F_SETFL, O_NONBLOCK);
 
-    /* TODO: register ufd with userfault thread */
+    /* register ufd with userfault thread */
+    u->postcopy_fd.fd = ufd;
+    u->postcopy_fd.data = dev;
+    u->postcopy_fd.handler = vhost_user_postcopy_fault_handler;
+    u->postcopy_fd.idstr = "vhost-user"; /* Need to find unique name */
+    postcopy_register_shared_ufd(&u->postcopy_fd);
     return 0;
 }