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

2018-03-07 Thread Peter Xu 
On Tue, Mar 06, 2018 at 11:20:56AM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Mon, Mar 05, 2018 at 05:42:42PM +, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (pet...@redhat.com) wrote:
> > > > On Fri, Feb 16, 2018 at 01:16:07PM +, 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 

Thanks,

-- 
Peter Xu

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

2018-03-06 Thread Dr. David Alan Gilbert 
* Peter Xu (pet...@redhat.com) wrote:
> On Mon, Mar 05, 2018 at 05:42:42PM +, Dr. David Alan Gilbert wrote:
> > * Peter Xu (pet...@redhat.com) wrote:
> > > On Fri, Feb 16, 2018 at 01:16:07PM +, 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 / dgilb...@redhat.com / Manchester, UK
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

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

2018-03-05 Thread Peter Xu 
On Mon, Mar 05, 2018 at 05:42:42PM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Fri, Feb 16, 2018 at 01:16:07PM +, 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 / dgilb...@redhat.com / Manchester, UK

-- 
Peter Xu

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

2018-03-05 Thread Dr. David Alan Gilbert 
* Peter Xu (pet...@redhat.com) wrote:
> On Fri, Feb 16, 2018 at 01:16:07PM +, 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 / dgilb...@redhat.com / Manchester, UK

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

2018-02-28 Thread Peter Xu 
On Fri, Feb 16, 2018 at 01:16:07PM +, 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

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

2018-02-16 Thread Dr. David Alan Gilbert (git) 
From: "Dr. David Alan Gilbert" 

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

Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Marc-André Lureau 
---
 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 
 #include 
@@ -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