[Qemu-devel] [PATCH v3 11/29] vhost+postcopy: Transmit 'listen' to client

Dr. David Alan Gilbert (git) posted 29 patches 7 years, 8 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v3 11/29] vhost+postcopy: Transmit 'listen' to client
Posted by Dr. David Alan Gilbert (git) 7 years, 8 months ago
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Notify the vhost-user client on reception of the 'postcopy-listen'
event from the source.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 contrib/libvhost-user/libvhost-user.c | 19 +++++++++++++++++++
 contrib/libvhost-user/libvhost-user.h |  2 ++
 docs/interop/vhost-user.txt           |  6 ++++++
 hw/virtio/trace-events                |  3 +++
 hw/virtio/vhost-user.c                | 34 ++++++++++++++++++++++++++++++++++
 migration/postcopy-ram.h              |  1 +
 migration/savevm.c                    |  7 +++++++
 7 files changed, 72 insertions(+)

diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index 0b563fc5ae..beec7695a8 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -98,6 +98,7 @@ vu_request_to_string(unsigned int req)
         REQ(VHOST_USER_GET_CONFIG),
         REQ(VHOST_USER_SET_CONFIG),
         REQ(VHOST_USER_POSTCOPY_ADVISE),
+        REQ(VHOST_USER_POSTCOPY_LISTEN),
         REQ(VHOST_USER_MAX),
     };
 #undef REQ
@@ -933,6 +934,22 @@ out:
     return true; /* = send a reply */
 }
 
+static bool
+vu_set_postcopy_listen(VuDev *dev, VhostUserMsg *vmsg)
+{
+    vmsg->payload.u64 = -1;
+    vmsg->size = sizeof(vmsg->payload.u64);
+
+    if (dev->nregions) {
+        vu_panic(dev, "Regions already registered at postcopy-listen");
+        return true;
+    }
+    dev->postcopy_listening = true;
+
+    vmsg->flags = VHOST_USER_VERSION |  VHOST_USER_REPLY_MASK;
+    vmsg->payload.u64 = 0; /* Success */
+    return true;
+}
 static bool
 vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
 {
@@ -1006,6 +1023,8 @@ vu_process_message(VuDev *dev, VhostUserMsg *vmsg)
         break;
     case VHOST_USER_POSTCOPY_ADVISE:
         return vu_set_postcopy_advise(dev, vmsg);
+    case VHOST_USER_POSTCOPY_LISTEN:
+        return vu_set_postcopy_listen(dev, vmsg);
     default:
         vmsg_close_fds(vmsg);
         vu_panic(dev, "Unhandled request: %d", vmsg->request);
diff --git a/contrib/libvhost-user/libvhost-user.h b/contrib/libvhost-user/libvhost-user.h
index bb33b33f3b..fcba53c3c3 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -83,6 +83,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_CONFIG = 24,
     VHOST_USER_SET_CONFIG = 25,
     VHOST_USER_POSTCOPY_ADVISE  = 26,
+    VHOST_USER_POSTCOPY_LISTEN  = 27,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -282,6 +283,7 @@ struct VuDev {
 
     /* Postcopy data */
     int postcopy_ufd;
+    bool postcopy_listening;
 };
 
 typedef struct VuVirtqElement {
diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index 621543e654..bdec9ec0e8 100644
--- a/docs/interop/vhost-user.txt
+++ b/docs/interop/vhost-user.txt
@@ -682,6 +682,12 @@ Master message types
       the slave must open a userfaultfd for later use.
       Note that at this stage the migration is still in precopy mode.
 
+ * VHOST_USER_POSTCOPY_LISTEN
+      Id: 27
+      Master payload: N/A
+
+      Master advises slave that a transition to postcopy mode has happened.
+
 Slave message types
 -------------------
 
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 742ff0f90b..06ec03d6e7 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -6,6 +6,9 @@ vhost_region_add_section(const char *name, uint64_t gpa, uint64_t size, uint64_t
 vhost_region_add_section_abut(const char *name, uint64_t new_size) "%s: 0x%"PRIx64
 vhost_section(const char *name, int r) "%s:%d"
 
+# hw/virtio/vhost-user.c
+vhost_user_postcopy_listen(void) ""
+
 # hw/virtio/virtio.c
 virtqueue_alloc_element(void *elem, size_t sz, unsigned in_num, unsigned out_num) "elem %p size %zd in_num %u out_num %u"
 virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u"
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index dd4eb50668..ec6a4a82fd 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -19,6 +19,7 @@
 #include "qemu/sockets.h"
 #include "migration/migration.h"
 #include "migration/postcopy-ram.h"
+#include "trace.h"
 
 #include <sys/ioctl.h>
 #include <sys/socket.h>
@@ -76,6 +77,7 @@ typedef enum VhostUserRequest {
     VHOST_USER_GET_CONFIG = 24,
     VHOST_USER_SET_CONFIG = 25,
     VHOST_USER_POSTCOPY_ADVISE  = 26,
+    VHOST_USER_POSTCOPY_LISTEN  = 27,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -157,6 +159,8 @@ struct vhost_user {
     int slave_fd;
     NotifierWithReturn postcopy_notifier;
     struct PostCopyFD  postcopy_fd;
+    /* True once we've entered postcopy_listen */
+    bool               postcopy_listen;
 };
 
 static bool ioeventfd_enabled(void)
@@ -843,6 +847,33 @@ static int vhost_user_postcopy_advise(struct vhost_dev *dev, Error **errp)
     return 0;
 }
 
+/*
+ * Called at the switch to postcopy on reception of the 'listen' command.
+ */
+static int vhost_user_postcopy_listen(struct vhost_dev *dev, Error **errp)
+{
+    struct vhost_user *u = dev->opaque;
+    int ret;
+    VhostUserMsg msg = {
+        .hdr.request = VHOST_USER_POSTCOPY_LISTEN,
+        .hdr.flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
+    };
+    u->postcopy_listen = true;
+    trace_vhost_user_postcopy_listen();
+    if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
+        error_setg(errp, "Failed to send postcopy_listen to vhost");
+        return -1;
+    }
+
+    ret = process_message_reply(dev, &msg);
+    if (ret) {
+        error_setg(errp, "Failed to receive reply to postcopy_listen");
+        return ret;
+    }
+
+    return 0;
+}
+
 static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
                                         void *opaque)
 {
@@ -865,6 +896,9 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier,
     case POSTCOPY_NOTIFY_INBOUND_ADVISE:
         return vhost_user_postcopy_advise(dev, pnd->errp);
 
+    case POSTCOPY_NOTIFY_INBOUND_LISTEN:
+        return vhost_user_postcopy_listen(dev, pnd->errp);
+
     default:
         /* We ignore notifications we don't know */
         break;
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index 23efbdf346..dbc2ee1f2b 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -129,6 +129,7 @@ void postcopy_infrastructure_init(void);
 enum PostcopyNotifyReason {
     POSTCOPY_NOTIFY_PROBE = 0,
     POSTCOPY_NOTIFY_INBOUND_ADVISE,
+    POSTCOPY_NOTIFY_INBOUND_LISTEN,
 };
 
 struct PostcopyNotifyData {
diff --git a/migration/savevm.c b/migration/savevm.c
index 9840bcaac9..fa779232cc 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1614,6 +1614,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
 {
     PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_LISTENING);
     trace_loadvm_postcopy_handle_listen();
+    Error *local_err = NULL;
+
     if (ps != POSTCOPY_INCOMING_ADVISE && ps != POSTCOPY_INCOMING_DISCARD) {
         error_report("CMD_POSTCOPY_LISTEN in wrong postcopy state (%d)", ps);
         return -1;
@@ -1639,6 +1641,11 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
         }
     }
 
+    if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_LISTEN, &local_err)) {
+        error_report_err(local_err);
+        return -1;
+    }
+
     if (mis->have_listen_thread) {
         error_report("CMD_POSTCOPY_RAM_LISTEN already has a listen thread");
         return -1;
-- 
2.14.3


Re: [Qemu-devel] [PATCH v3 11/29] vhost+postcopy: Transmit 'listen' to client
Posted by Peter Xu 7 years, 7 months ago
On Fri, Feb 16, 2018 at 01:16:07PM +0000, Dr. David Alan Gilbert (git) wrote:

[...]

>  typedef struct VuVirtqElement {
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index 621543e654..bdec9ec0e8 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -682,6 +682,12 @@ Master message types
>        the slave must open a userfaultfd for later use.
>        Note that at this stage the migration is still in precopy mode.
>  
> + * VHOST_USER_POSTCOPY_LISTEN
> +      Id: 27
> +      Master payload: N/A
> +
> +      Master advises slave that a transition to postcopy mode has happened.

Could we add something to explain why this listen needs to be
broadcasted to clients?  Since I failed to find it out quickly
myself. :(

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v3 11/29] vhost+postcopy: Transmit 'listen' to client
Posted by Dr. David Alan Gilbert 7 years, 7 months ago
* Peter Xu (peterx@redhat.com) wrote:
> On Fri, Feb 16, 2018 at 01:16:07PM +0000, Dr. David Alan Gilbert (git) wrote:
> 
> [...]
> 
> >  typedef struct VuVirtqElement {
> > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> > index 621543e654..bdec9ec0e8 100644
> > --- a/docs/interop/vhost-user.txt
> > +++ b/docs/interop/vhost-user.txt
> > @@ -682,6 +682,12 @@ Master message types
> >        the slave must open a userfaultfd for later use.
> >        Note that at this stage the migration is still in precopy mode.
> >  
> > + * VHOST_USER_POSTCOPY_LISTEN
> > +      Id: 27
> > +      Master payload: N/A
> > +
> > +      Master advises slave that a transition to postcopy mode has happened.
> 
> Could we add something to explain why this listen needs to be
> broadcasted to clients?  Since I failed to find it out quickly
> myself. :(

I've changed this to:

 * VHOST_USER_POSTCOPY_LISTEN
      Id: 29
      Master payload: N/A

      Master advises slave that a transition to postcopy mode has happened.
      The slave must ensure that shared memory is registered with userfaultfd
      to cause faulting of non-present pages.

      This is always sent sometime after a VHOST_USER_POSTCOPY_ADVISE, and
      thus only when VHOST_USER_PROTOCOL_F_PAGEFAULT is supported.

Dave

> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 11/29] vhost+postcopy: Transmit 'listen' to client
Posted by Peter Xu 7 years, 7 months ago
On Mon, Mar 05, 2018 at 05:42:42PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > On Fri, Feb 16, 2018 at 01:16:07PM +0000, Dr. David Alan Gilbert (git) wrote:
> > 
> > [...]
> > 
> > >  typedef struct VuVirtqElement {
> > > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> > > index 621543e654..bdec9ec0e8 100644
> > > --- a/docs/interop/vhost-user.txt
> > > +++ b/docs/interop/vhost-user.txt
> > > @@ -682,6 +682,12 @@ Master message types
> > >        the slave must open a userfaultfd for later use.
> > >        Note that at this stage the migration is still in precopy mode.
> > >  
> > > + * VHOST_USER_POSTCOPY_LISTEN
> > > +      Id: 27
> > > +      Master payload: N/A
> > > +
> > > +      Master advises slave that a transition to postcopy mode has happened.
> > 
> > Could we add something to explain why this listen needs to be
> > broadcasted to clients?  Since I failed to find it out quickly
> > myself. :(
> 
> I've changed this to:
> 
>  * VHOST_USER_POSTCOPY_LISTEN
>       Id: 29
>       Master payload: N/A
> 
>       Master advises slave that a transition to postcopy mode has happened.
>       The slave must ensure that shared memory is registered with userfaultfd
>       to cause faulting of non-present pages.

But shouldn't this be assured by the SET_MEM_TABLE call?

Sorry for being not that familiar with vhost-user protocol... but
what's the correct order of these commands?

  POSTCOPY_ADVISE
  POSTCOPY_LISTEN
  SET_MEM_TABLE

?  Thanks,

> 
>       This is always sent sometime after a VHOST_USER_POSTCOPY_ADVISE, and
>       thus only when VHOST_USER_PROTOCOL_F_PAGEFAULT is supported.
> 
> Dave
> 
> > -- 
> > Peter Xu
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v3 11/29] vhost+postcopy: Transmit 'listen' to client
Posted by Dr. David Alan Gilbert 7 years, 7 months ago
* Peter Xu (peterx@redhat.com) wrote:
> On Mon, Mar 05, 2018 at 05:42:42PM +0000, Dr. David Alan Gilbert wrote:
> > * Peter Xu (peterx@redhat.com) wrote:
> > > On Fri, Feb 16, 2018 at 01:16:07PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > 
> > > [...]
> > > 
> > > >  typedef struct VuVirtqElement {
> > > > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> > > > index 621543e654..bdec9ec0e8 100644
> > > > --- a/docs/interop/vhost-user.txt
> > > > +++ b/docs/interop/vhost-user.txt
> > > > @@ -682,6 +682,12 @@ Master message types
> > > >        the slave must open a userfaultfd for later use.
> > > >        Note that at this stage the migration is still in precopy mode.
> > > >  
> > > > + * VHOST_USER_POSTCOPY_LISTEN
> > > > +      Id: 27
> > > > +      Master payload: N/A
> > > > +
> > > > +      Master advises slave that a transition to postcopy mode has happened.
> > > 
> > > Could we add something to explain why this listen needs to be
> > > broadcasted to clients?  Since I failed to find it out quickly
> > > myself. :(
> > 
> > I've changed this to:
> > 
> >  * VHOST_USER_POSTCOPY_LISTEN
> >       Id: 29
> >       Master payload: N/A
> > 
> >       Master advises slave that a transition to postcopy mode has happened.
> >       The slave must ensure that shared memory is registered with userfaultfd
> >       to cause faulting of non-present pages.
> 
> But shouldn't this be assured by the SET_MEM_TABLE call?

Yes, it is the set_mem_table that does the register - but it only
registers it with userfaultfd if it's received a 'listen' notification,
otherwise it assumes it's a normal precopy migration.

> Sorry for being not that familiar with vhost-user protocol... but
> what's the correct order of these commands?
> 
>   POSTCOPY_ADVISE
>   POSTCOPY_LISTEN
>   SET_MEM_TABLE

Right.

Dave

> ?  Thanks,
> 
> > 
> >       This is always sent sometime after a VHOST_USER_POSTCOPY_ADVISE, and
> >       thus only when VHOST_USER_PROTOCOL_F_PAGEFAULT is supported.
> > 
> > Dave
> > 
> > > -- 
> > > Peter Xu
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 11/29] vhost+postcopy: Transmit 'listen' to client
Posted by Peter Xu 7 years, 7 months ago
On Tue, Mar 06, 2018 at 11:20:56AM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > On Mon, Mar 05, 2018 at 05:42:42PM +0000, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (peterx@redhat.com) wrote:
> > > > On Fri, Feb 16, 2018 at 01:16:07PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > > 
> > > > [...]
> > > > 
> > > > >  typedef struct VuVirtqElement {
> > > > > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> > > > > index 621543e654..bdec9ec0e8 100644
> > > > > --- a/docs/interop/vhost-user.txt
> > > > > +++ b/docs/interop/vhost-user.txt
> > > > > @@ -682,6 +682,12 @@ Master message types
> > > > >        the slave must open a userfaultfd for later use.
> > > > >        Note that at this stage the migration is still in precopy mode.
> > > > >  
> > > > > + * VHOST_USER_POSTCOPY_LISTEN
> > > > > +      Id: 27
> > > > > +      Master payload: N/A
> > > > > +
> > > > > +      Master advises slave that a transition to postcopy mode has happened.
> > > > 
> > > > Could we add something to explain why this listen needs to be
> > > > broadcasted to clients?  Since I failed to find it out quickly
> > > > myself. :(
> > > 
> > > I've changed this to:
> > > 
> > >  * VHOST_USER_POSTCOPY_LISTEN
> > >       Id: 29
> > >       Master payload: N/A
> > > 
> > >       Master advises slave that a transition to postcopy mode has happened.
> > >       The slave must ensure that shared memory is registered with userfaultfd
> > >       to cause faulting of non-present pages.
> > 
> > But shouldn't this be assured by the SET_MEM_TABLE call?
> 
> Yes, it is the set_mem_table that does the register - but it only
> registers it with userfaultfd if it's received a 'listen' notification,
> otherwise it assumes it's a normal precopy migration.

I think I got the picture now.  Please add this after the enhanced
document:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu