Re: [Qemu-devel] [RFC v2 11/32] vhub: Open userfaultfd

2017-09-15 Thread Dr. David Alan Gilbert
* Peter Xu (pet...@redhat.com) wrote:
> On Thu, Aug 24, 2017 at 08:27:09PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Open a userfaultfd (on a postcopy_advise) and send it back in
> > the reply to the qemu for it to monitor.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  contrib/libvhost-user/libvhost-user.c | 26 +++---
> >  contrib/libvhost-user/libvhost-user.h |  3 +++
> >  2 files changed, 26 insertions(+), 3 deletions(-)
> > 
> > diff --git a/contrib/libvhost-user/libvhost-user.c 
> > b/contrib/libvhost-user/libvhost-user.c
> > index 47884c0a15..f9b5b12b28 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -15,6 +15,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  #include "qemu/atomic.h"
> > @@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg 
> > *vmsg)
> >  static bool
> >  vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
> >  {
> > -/* TODO: Open ufd, pass it back in the request
> > - * TODO: Add addresses 
> > - */
> > +struct uffdio_api api_struct;
> > +
> > +dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> > +/* TODO: Add addresses */
> >  vmsg->payload.u64 = 0xcafe;
> >  vmsg->size = sizeof(vmsg->payload.u64);
> > +
> > +if (dev->postcopy_ufd == -1) {
> > +vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
> > +goto out;
> 
> We got error but still goto out?  I feel like we should reply with
> some kind of error code when any error happens.

See that the out: code returns the dev->postcopy_ufd to qemu
and in this case it's -1 so it is already flagged as an error.

> > +}
> > +api_struct.api = UFFD_API;
> > +api_struct.features = 0;
> > +if (ioctl(dev->postcopy_ufd, UFFDIO_API, _struct)) {
> > +vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
> > +close(dev->postcopy_ufd);
> > +dev->postcopy_ufd = -1;
> > +goto out;
> 
> Same here.

And there we explicitly set dev->postcopy_ufd = -1 before going to out
so it's sent as the error value.

> > +}
> > +/* TODO: Stash feature flags somewhere */
> > +out:
> > +/* Return a ufd to the QEMU */
> > +vmsg->fd_num = 1;
> > +vmsg->fds[0] = dev->postcopy_ufd;

^^^ see that's where the -1's end up.
> >  return true; /* = send a reply */
> >  }

Dave

> >  
> > diff --git a/contrib/libvhost-user/libvhost-user.h 
> > b/contrib/libvhost-user/libvhost-user.h
> > index 3987ce643d..3e8efdd919 100644
> > --- a/contrib/libvhost-user/libvhost-user.h
> > +++ b/contrib/libvhost-user/libvhost-user.h
> > @@ -234,6 +234,9 @@ struct VuDev {
> >   * re-initialize */
> >  vu_panic_cb panic;
> >  const VuDevIface *iface;
> > +
> > +/* Postcopy data */
> > +int postcopy_ufd;
> >  };
> >  
> >  typedef struct VuVirtqElement {
> > -- 
> > 2.13.5
> > 
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [RFC v2 11/32] vhub: Open userfaultfd

2017-09-07 Thread Dr. David Alan Gilbert
* Marc-André Lureau (marcandre.lur...@gmail.com) wrote:
> I would rather use libvhost-user: message prefix (same for similar
> libvhost-user patches)

Done.

> On Thu, Aug 24, 2017 at 12:27 PM, Dr. David Alan Gilbert (git)
>  wrote:
> > From: "Dr. David Alan Gilbert" 
> >
> > Open a userfaultfd (on a postcopy_advise) and send it back in
> > the reply to the qemu for it to monitor.
> >
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  contrib/libvhost-user/libvhost-user.c | 26 +++---
> >  contrib/libvhost-user/libvhost-user.h |  3 +++
> >  2 files changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/contrib/libvhost-user/libvhost-user.c 
> > b/contrib/libvhost-user/libvhost-user.c
> > index 47884c0a15..f9b5b12b28 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -15,6 +15,7 @@
> >
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >
> >  #include "qemu/atomic.h"
> > @@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg 
> > *vmsg)
> >  static bool
> >  vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
> >  {
> > -/* TODO: Open ufd, pass it back in the request
> > - * TODO: Add addresses
> > - */
> > +struct uffdio_api api_struct;
> > +
> > +dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> 
> This will likely fail to compile on !Linux, could you add some
> appropriate #ifdef?

Note that we already #include  so this file only builds
on Linux anyway before I came along, however I'll add some ifdef's for
my new code.

Dave


> > +/* TODO: Add addresses */
> >  vmsg->payload.u64 = 0xcafe;
> >  vmsg->size = sizeof(vmsg->payload.u64);
> > +
> > +if (dev->postcopy_ufd == -1) {
> > +vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
> > +goto out;
> > +}
> > +api_struct.api = UFFD_API;
> > +api_struct.features = 0;
> > +if (ioctl(dev->postcopy_ufd, UFFDIO_API, _struct)) {
> > +vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
> > +close(dev->postcopy_ufd);
> > +dev->postcopy_ufd = -1;
> > +goto out;
> > +}
> > +/* TODO: Stash feature flags somewhere */
> > +out:
> > +/* Return a ufd to the QEMU */
> > +vmsg->fd_num = 1;
> > +vmsg->fds[0] = dev->postcopy_ufd;
> >  return true; /* = send a reply */
> >  }
> >
> > diff --git a/contrib/libvhost-user/libvhost-user.h 
> > b/contrib/libvhost-user/libvhost-user.h
> > index 3987ce643d..3e8efdd919 100644
> > --- a/contrib/libvhost-user/libvhost-user.h
> > +++ b/contrib/libvhost-user/libvhost-user.h
> > @@ -234,6 +234,9 @@ struct VuDev {
> >   * re-initialize */
> >  vu_panic_cb panic;
> >  const VuDevIface *iface;
> > +
> > +/* Postcopy data */
> > +int postcopy_ufd;
> >  };
> >
> >  typedef struct VuVirtqElement {
> > --
> > 2.13.5
> >
> >
> 
> 
> 
> -- 
> Marc-André Lureau
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [RFC v2 11/32] vhub: Open userfaultfd

2017-08-30 Thread Marc-André Lureau
I would rather use libvhost-user: message prefix (same for similar
libvhost-user patches)

On Thu, Aug 24, 2017 at 12:27 PM, Dr. David Alan Gilbert (git)
 wrote:
> From: "Dr. David Alan Gilbert" 
>
> Open a userfaultfd (on a postcopy_advise) and send it back in
> the reply to the qemu for it to monitor.
>
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  contrib/libvhost-user/libvhost-user.c | 26 +++---
>  contrib/libvhost-user/libvhost-user.h |  3 +++
>  2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c 
> b/contrib/libvhost-user/libvhost-user.c
> index 47884c0a15..f9b5b12b28 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -15,6 +15,7 @@
>
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #include "qemu/atomic.h"
> @@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
>  static bool
>  vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
>  {
> -/* TODO: Open ufd, pass it back in the request
> - * TODO: Add addresses
> - */
> +struct uffdio_api api_struct;
> +
> +dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);

This will likely fail to compile on !Linux, could you add some
appropriate #ifdef?

> +/* TODO: Add addresses */
>  vmsg->payload.u64 = 0xcafe;
>  vmsg->size = sizeof(vmsg->payload.u64);
> +
> +if (dev->postcopy_ufd == -1) {
> +vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
> +goto out;
> +}
> +api_struct.api = UFFD_API;
> +api_struct.features = 0;
> +if (ioctl(dev->postcopy_ufd, UFFDIO_API, _struct)) {
> +vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
> +close(dev->postcopy_ufd);
> +dev->postcopy_ufd = -1;
> +goto out;
> +}
> +/* TODO: Stash feature flags somewhere */
> +out:
> +/* Return a ufd to the QEMU */
> +vmsg->fd_num = 1;
> +vmsg->fds[0] = dev->postcopy_ufd;
>  return true; /* = send a reply */
>  }
>
> diff --git a/contrib/libvhost-user/libvhost-user.h 
> b/contrib/libvhost-user/libvhost-user.h
> index 3987ce643d..3e8efdd919 100644
> --- a/contrib/libvhost-user/libvhost-user.h
> +++ b/contrib/libvhost-user/libvhost-user.h
> @@ -234,6 +234,9 @@ struct VuDev {
>   * re-initialize */
>  vu_panic_cb panic;
>  const VuDevIface *iface;
> +
> +/* Postcopy data */
> +int postcopy_ufd;
>  };
>
>  typedef struct VuVirtqElement {
> --
> 2.13.5
>
>



-- 
Marc-André Lureau



Re: [Qemu-devel] [RFC v2 11/32] vhub: Open userfaultfd

2017-08-29 Thread Peter Xu
On Thu, Aug 24, 2017 at 08:27:09PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> Open a userfaultfd (on a postcopy_advise) and send it back in
> the reply to the qemu for it to monitor.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  contrib/libvhost-user/libvhost-user.c | 26 +++---
>  contrib/libvhost-user/libvhost-user.h |  3 +++
>  2 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/libvhost-user/libvhost-user.c 
> b/contrib/libvhost-user/libvhost-user.c
> index 47884c0a15..f9b5b12b28 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -15,6 +15,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include "qemu/atomic.h"
> @@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
>  static bool
>  vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
>  {
> -/* TODO: Open ufd, pass it back in the request
> - * TODO: Add addresses 
> - */
> +struct uffdio_api api_struct;
> +
> +dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> +/* TODO: Add addresses */
>  vmsg->payload.u64 = 0xcafe;
>  vmsg->size = sizeof(vmsg->payload.u64);
> +
> +if (dev->postcopy_ufd == -1) {
> +vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
> +goto out;

We got error but still goto out?  I feel like we should reply with
some kind of error code when any error happens.

> +}
> +api_struct.api = UFFD_API;
> +api_struct.features = 0;
> +if (ioctl(dev->postcopy_ufd, UFFDIO_API, _struct)) {
> +vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
> +close(dev->postcopy_ufd);
> +dev->postcopy_ufd = -1;
> +goto out;

Same here.

> +}
> +/* TODO: Stash feature flags somewhere */
> +out:
> +/* Return a ufd to the QEMU */
> +vmsg->fd_num = 1;
> +vmsg->fds[0] = dev->postcopy_ufd;
>  return true; /* = send a reply */
>  }
>  
> diff --git a/contrib/libvhost-user/libvhost-user.h 
> b/contrib/libvhost-user/libvhost-user.h
> index 3987ce643d..3e8efdd919 100644
> --- a/contrib/libvhost-user/libvhost-user.h
> +++ b/contrib/libvhost-user/libvhost-user.h
> @@ -234,6 +234,9 @@ struct VuDev {
>   * re-initialize */
>  vu_panic_cb panic;
>  const VuDevIface *iface;
> +
> +/* Postcopy data */
> +int postcopy_ufd;
>  };
>  
>  typedef struct VuVirtqElement {
> -- 
> 2.13.5
> 

-- 
Peter Xu



[Qemu-devel] [RFC v2 11/32] vhub: Open userfaultfd

2017-08-24 Thread Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" 

Open a userfaultfd (on a postcopy_advise) and send it back in
the reply to the qemu for it to monitor.

Signed-off-by: Dr. David Alan Gilbert 
---
 contrib/libvhost-user/libvhost-user.c | 26 +++---
 contrib/libvhost-user/libvhost-user.h |  3 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/contrib/libvhost-user/libvhost-user.c 
b/contrib/libvhost-user/libvhost-user.c
index 47884c0a15..f9b5b12b28 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -15,6 +15,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "qemu/atomic.h"
@@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg *vmsg)
 static bool
 vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
 {
-/* TODO: Open ufd, pass it back in the request
- * TODO: Add addresses 
- */
+struct uffdio_api api_struct;
+
+dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
+/* TODO: Add addresses */
 vmsg->payload.u64 = 0xcafe;
 vmsg->size = sizeof(vmsg->payload.u64);
+
+if (dev->postcopy_ufd == -1) {
+vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
+goto out;
+}
+api_struct.api = UFFD_API;
+api_struct.features = 0;
+if (ioctl(dev->postcopy_ufd, UFFDIO_API, _struct)) {
+vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
+close(dev->postcopy_ufd);
+dev->postcopy_ufd = -1;
+goto out;
+}
+/* TODO: Stash feature flags somewhere */
+out:
+/* Return a ufd to the QEMU */
+vmsg->fd_num = 1;
+vmsg->fds[0] = dev->postcopy_ufd;
 return true; /* = send a reply */
 }
 
diff --git a/contrib/libvhost-user/libvhost-user.h 
b/contrib/libvhost-user/libvhost-user.h
index 3987ce643d..3e8efdd919 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -234,6 +234,9 @@ struct VuDev {
  * re-initialize */
 vu_panic_cb panic;
 const VuDevIface *iface;
+
+/* Postcopy data */
+int postcopy_ufd;
 };
 
 typedef struct VuVirtqElement {
-- 
2.13.5