Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-25 Thread Alexey Perevalov 

On 04/25/2017 02:14 PM, Dr. David Alan Gilbert wrote:

* Alexey (a.pereva...@samsung.com) wrote:

+ Andrea Arcangeli

On Mon, Apr 24, 2017 at 06:10:02PM +0100, Dr. David Alan Gilbert wrote:

* Alexey (a.pereva...@samsung.com) wrote:

On Mon, Apr 24, 2017 at 04:12:29PM +0800, Peter Xu wrote:

On Fri, Apr 21, 2017 at 06:22:12PM +0300, Alexey wrote:

On Fri, Apr 21, 2017 at 11:24:54AM +0100, Dr. David Alan Gilbert wrote:

* Alexey Perevalov (a.pereva...@samsung.com) wrote:

Userfaultfd mechanism is able to provide process thread id,
in case when client request it with UFDD_API ioctl.

Signed-off-by: Alexey Perevalov 

There seem to be two parts to this:
   a) Adding the mis parameter to ufd_version_check
   b) Asking for the feature

Please split it into two patches.

Also


---
  include/migration/postcopy-ram.h |  2 +-
  migration/migration.c|  2 +-
  migration/postcopy-ram.c | 12 ++--
  migration/savevm.c   |  2 +-
  4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/migration/postcopy-ram.h b/include/migration/postcopy-ram.h
index 8e036b9..809f6db 100644
--- a/include/migration/postcopy-ram.h
+++ b/include/migration/postcopy-ram.h
@@ -14,7 +14,7 @@
  #define QEMU_POSTCOPY_RAM_H
  
  /* Return true if the host supports everything we need to do postcopy-ram */

-bool postcopy_ram_supported_by_host(void);
+bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
  
  /*

   * Make all of RAM sensitive to accesses to areas that haven't yet been 
written
diff --git a/migration/migration.c b/migration/migration.c
index ad4036f..79f6425 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -802,7 +802,7 @@ void 
qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
   * special support.
   */
  if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) &&
-!postcopy_ram_supported_by_host()) {
+!postcopy_ram_supported_by_host(NULL)) {
  /* postcopy_ram_supported_by_host will have emitted a more
   * detailed message
   */
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index dc80dbb..70f0480 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -60,13 +60,13 @@ struct PostcopyDiscardState {
  #include 
  #include 
  
-static bool ufd_version_check(int ufd)

+static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
  {
  struct uffdio_api api_struct;
  uint64_t ioctl_mask;
  
  api_struct.api = UFFD_API;

-api_struct.features = 0;
+api_struct.features = UFFD_FEATURE_THREAD_ID;
  if (ioctl(ufd, UFFDIO_API, _struct)) {
  error_report("postcopy_ram_supported_by_host: UFFDIO_API failed: %s",
   strerror(errno));

You're not actually using the 'mis' here - what I'd expected was
something that was going to check if the UFFDIO_API return said that it really
had the feature, and if so store a flag in the MIS somewhere.

Also, I'm not sure it's right to set 'api_struct.features' on the input - what
happens if this is run on an old kernel - we don't want postcopy to fail on
an old kernel without your feature.
I'm not 100% sure of the interface, but I think the way it works is you set
features = 0 before the call, and then check the api_struct.features in the
return - in the same way that I check for UFFD_FEATURE_MISSING_HUGETLBFS.


We need to ask kernel about that feature,
right,
kernel returns back available features
uffdio_api.features = UFFD_API_FEATURES
but it also stores requested features

I feel like this does not against Dave's comment, maybe we just need
to send the UFFDIO_API twice? Like:

yes, ioctl with UFFDIO_API will fail on old kernel if we will request
e.g. UFFD_FEATURE_THREAD_ID or other new feature.

So in general way need a per feature request, for better error handling.

No, we don't need to - I think the way the kernel works is that you pass
features = 0 in, and it sets api_struct.features on the way out;
so if you always pass 0 in, you can then just check the features that
it returns.


Without explicitly set UFFD_FEATURE_THREAD_ID, ptid will not sent back
to user space.

Also it's impossible to call ioctl UFFD_API more than one time, due to
internal state of userfault_ctx inside kernel is changing
UFFD_STATE_WAIT_API -> UFFD_STATE_RUNNING,
but ioctl UFFD_API expects UFFD_STATE_WAIT_API
^^^

So looks like no way to provide backward compatibility for old kernels.
I even don't know how to be with new kernels, because point of extension
should be for new kernels (e.g. I want to add new feature in future,
UFFD_FEATURE_ALLOW_PADDING which will allow UFFD_COPY for lesser page
size than was registered).
So how to be in this case, add new UFFD feature, like
UFFD_FEATURE_ALLOW_CALL_API_AGAIN (allow set not always/persistent feature,
like UFFD_FEATURE_THREAD_ID)

or just remove condition in kernel while

Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-25 Thread Dr. David Alan Gilbert 
* Alexey (a.pereva...@samsung.com) wrote:
> + Andrea Arcangeli
> 
> On Mon, Apr 24, 2017 at 06:10:02PM +0100, Dr. David Alan Gilbert wrote:
> > * Alexey (a.pereva...@samsung.com) wrote:
> > > On Mon, Apr 24, 2017 at 04:12:29PM +0800, Peter Xu wrote:
> > > > On Fri, Apr 21, 2017 at 06:22:12PM +0300, Alexey wrote:
> > > > > On Fri, Apr 21, 2017 at 11:24:54AM +0100, Dr. David Alan Gilbert 
> > > > > wrote:
> > > > > > * Alexey Perevalov (a.pereva...@samsung.com) wrote:
> > > > > > > Userfaultfd mechanism is able to provide process thread id,
> > > > > > > in case when client request it with UFDD_API ioctl.
> > > > > > > 
> > > > > > > Signed-off-by: Alexey Perevalov 
> > > > > > 
> > > > > > There seem to be two parts to this:
> > > > > >   a) Adding the mis parameter to ufd_version_check
> > > > > >   b) Asking for the feature
> > > > > > 
> > > > > > Please split it into two patches.
> > > > > > 
> > > > > > Also
> > > > > > 
> > > > > > > ---
> > > > > > >  include/migration/postcopy-ram.h |  2 +-
> > > > > > >  migration/migration.c|  2 +-
> > > > > > >  migration/postcopy-ram.c | 12 ++--
> > > > > > >  migration/savevm.c   |  2 +-
> > > > > > >  4 files changed, 9 insertions(+), 9 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/include/migration/postcopy-ram.h 
> > > > > > > b/include/migration/postcopy-ram.h
> > > > > > > index 8e036b9..809f6db 100644
> > > > > > > --- a/include/migration/postcopy-ram.h
> > > > > > > +++ b/include/migration/postcopy-ram.h
> > > > > > > @@ -14,7 +14,7 @@
> > > > > > >  #define QEMU_POSTCOPY_RAM_H
> > > > > > >  
> > > > > > >  /* Return true if the host supports everything we need to do 
> > > > > > > postcopy-ram */
> > > > > > > -bool postcopy_ram_supported_by_host(void);
> > > > > > > +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
> > > > > > >  
> > > > > > >  /*
> > > > > > >   * Make all of RAM sensitive to accesses to areas that haven't 
> > > > > > > yet been written
> > > > > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > > > > index ad4036f..79f6425 100644
> > > > > > > --- a/migration/migration.c
> > > > > > > +++ b/migration/migration.c
> > > > > > > @@ -802,7 +802,7 @@ void 
> > > > > > > qmp_migrate_set_capabilities(MigrationCapabilityStatusList 
> > > > > > > *params,
> > > > > > >   * special support.
> > > > > > >   */
> > > > > > >  if (!old_postcopy_cap && 
> > > > > > > runstate_check(RUN_STATE_INMIGRATE) &&
> > > > > > > -!postcopy_ram_supported_by_host()) {
> > > > > > > +!postcopy_ram_supported_by_host(NULL)) {
> > > > > > >  /* postcopy_ram_supported_by_host will have emitted 
> > > > > > > a more
> > > > > > >   * detailed message
> > > > > > >   */
> > > > > > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > > > > > > index dc80dbb..70f0480 100644
> > > > > > > --- a/migration/postcopy-ram.c
> > > > > > > +++ b/migration/postcopy-ram.c
> > > > > > > @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
> > > > > > >  #include 
> > > > > > >  #include 
> > > > > > >  
> > > > > > > -static bool ufd_version_check(int ufd)
> > > > > > > +static bool ufd_version_check(int ufd, MigrationIncomingState 
> > > > > > > *mis)
> > > > > > >  {
> > > > > > >  struct uffdio_api api_struct;
> > > > > > >  uint64_t ioctl_mask;
> > > > > > >  
> > > > > > >  api_struct.api = UFFD_API;
> > > > > > > -api_struct.features = 0;
> > > > > > > +api_struct.features = UFFD_FEATURE_THREAD_ID;
> > > > > > >  if (ioctl(ufd, UFFDIO_API, _struct)) {
> > > > > > >  error_report("postcopy_ram_supported_by_host: UFFDIO_API 
> > > > > > > failed: %s",
> > > > > > >   strerror(errno));
> > > > > > 
> > > > > > You're not actually using the 'mis' here - what I'd expected was
> > > > > > something that was going to check if the UFFDIO_API return said 
> > > > > > that it really
> > > > > > had the feature, and if so store a flag in the MIS somewhere.
> > > > > > 
> > > > > > Also, I'm not sure it's right to set 'api_struct.features' on the 
> > > > > > input - what
> > > > > > happens if this is run on an old kernel - we don't want postcopy to 
> > > > > > fail on
> > > > > > an old kernel without your feature.
> > > > > > I'm not 100% sure of the interface, but I think the way it works is 
> > > > > > you set
> > > > > > features = 0 before the call, and then check the 
> > > > > > api_struct.features in the
> > > > > > return - in the same way that I check for 
> > > > > > UFFD_FEATURE_MISSING_HUGETLBFS.
> > > > > > 
> > > > > We need to ask kernel about that feature,
> > > > > right,
> > > > > kernel returns back available features
> > > > > uffdio_api.features = UFFD_API_FEATURES
> > > > > but it also stores requested features
> > > > 
> > > > I feel like this does not against Dave's comment, maybe we just

Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-25 Thread Alexey 
+ Andrea Arcangeli

On Mon, Apr 24, 2017 at 06:10:02PM +0100, Dr. David Alan Gilbert wrote:
> * Alexey (a.pereva...@samsung.com) wrote:
> > On Mon, Apr 24, 2017 at 04:12:29PM +0800, Peter Xu wrote:
> > > On Fri, Apr 21, 2017 at 06:22:12PM +0300, Alexey wrote:
> > > > On Fri, Apr 21, 2017 at 11:24:54AM +0100, Dr. David Alan Gilbert wrote:
> > > > > * Alexey Perevalov (a.pereva...@samsung.com) wrote:
> > > > > > Userfaultfd mechanism is able to provide process thread id,
> > > > > > in case when client request it with UFDD_API ioctl.
> > > > > > 
> > > > > > Signed-off-by: Alexey Perevalov 
> > > > > 
> > > > > There seem to be two parts to this:
> > > > >   a) Adding the mis parameter to ufd_version_check
> > > > >   b) Asking for the feature
> > > > > 
> > > > > Please split it into two patches.
> > > > > 
> > > > > Also
> > > > > 
> > > > > > ---
> > > > > >  include/migration/postcopy-ram.h |  2 +-
> > > > > >  migration/migration.c|  2 +-
> > > > > >  migration/postcopy-ram.c | 12 ++--
> > > > > >  migration/savevm.c   |  2 +-
> > > > > >  4 files changed, 9 insertions(+), 9 deletions(-)
> > > > > > 
> > > > > > diff --git a/include/migration/postcopy-ram.h 
> > > > > > b/include/migration/postcopy-ram.h
> > > > > > index 8e036b9..809f6db 100644
> > > > > > --- a/include/migration/postcopy-ram.h
> > > > > > +++ b/include/migration/postcopy-ram.h
> > > > > > @@ -14,7 +14,7 @@
> > > > > >  #define QEMU_POSTCOPY_RAM_H
> > > > > >  
> > > > > >  /* Return true if the host supports everything we need to do 
> > > > > > postcopy-ram */
> > > > > > -bool postcopy_ram_supported_by_host(void);
> > > > > > +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
> > > > > >  
> > > > > >  /*
> > > > > >   * Make all of RAM sensitive to accesses to areas that haven't yet 
> > > > > > been written
> > > > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > > > index ad4036f..79f6425 100644
> > > > > > --- a/migration/migration.c
> > > > > > +++ b/migration/migration.c
> > > > > > @@ -802,7 +802,7 @@ void 
> > > > > > qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> > > > > >   * special support.
> > > > > >   */
> > > > > >  if (!old_postcopy_cap && 
> > > > > > runstate_check(RUN_STATE_INMIGRATE) &&
> > > > > > -!postcopy_ram_supported_by_host()) {
> > > > > > +!postcopy_ram_supported_by_host(NULL)) {
> > > > > >  /* postcopy_ram_supported_by_host will have emitted a 
> > > > > > more
> > > > > >   * detailed message
> > > > > >   */
> > > > > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > > > > > index dc80dbb..70f0480 100644
> > > > > > --- a/migration/postcopy-ram.c
> > > > > > +++ b/migration/postcopy-ram.c
> > > > > > @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
> > > > > >  #include 
> > > > > >  #include 
> > > > > >  
> > > > > > -static bool ufd_version_check(int ufd)
> > > > > > +static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
> > > > > >  {
> > > > > >  struct uffdio_api api_struct;
> > > > > >  uint64_t ioctl_mask;
> > > > > >  
> > > > > >  api_struct.api = UFFD_API;
> > > > > > -api_struct.features = 0;
> > > > > > +api_struct.features = UFFD_FEATURE_THREAD_ID;
> > > > > >  if (ioctl(ufd, UFFDIO_API, _struct)) {
> > > > > >  error_report("postcopy_ram_supported_by_host: UFFDIO_API 
> > > > > > failed: %s",
> > > > > >   strerror(errno));
> > > > > 
> > > > > You're not actually using the 'mis' here - what I'd expected was
> > > > > something that was going to check if the UFFDIO_API return said that 
> > > > > it really
> > > > > had the feature, and if so store a flag in the MIS somewhere.
> > > > > 
> > > > > Also, I'm not sure it's right to set 'api_struct.features' on the 
> > > > > input - what
> > > > > happens if this is run on an old kernel - we don't want postcopy to 
> > > > > fail on
> > > > > an old kernel without your feature.
> > > > > I'm not 100% sure of the interface, but I think the way it works is 
> > > > > you set
> > > > > features = 0 before the call, and then check the api_struct.features 
> > > > > in the
> > > > > return - in the same way that I check for 
> > > > > UFFD_FEATURE_MISSING_HUGETLBFS.
> > > > > 
> > > > We need to ask kernel about that feature,
> > > > right,
> > > > kernel returns back available features
> > > > uffdio_api.features = UFFD_API_FEATURES
> > > > but it also stores requested features
> > > 
> > > I feel like this does not against Dave's comment, maybe we just need
> > > to send the UFFDIO_API twice? Like:
> > yes, ioctl with UFFDIO_API will fail on old kernel if we will request
> > e.g. UFFD_FEATURE_THREAD_ID or other new feature.
> > 
> > So in general way need a per feature request, for better error handling.
> 
> No, we don't need to - I think

Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-24 Thread Dr. David Alan Gilbert 
* Alexey (a.pereva...@samsung.com) wrote:
> On Mon, Apr 24, 2017 at 04:12:29PM +0800, Peter Xu wrote:
> > On Fri, Apr 21, 2017 at 06:22:12PM +0300, Alexey wrote:
> > > On Fri, Apr 21, 2017 at 11:24:54AM +0100, Dr. David Alan Gilbert wrote:
> > > > * Alexey Perevalov (a.pereva...@samsung.com) wrote:
> > > > > Userfaultfd mechanism is able to provide process thread id,
> > > > > in case when client request it with UFDD_API ioctl.
> > > > > 
> > > > > Signed-off-by: Alexey Perevalov 
> > > > 
> > > > There seem to be two parts to this:
> > > >   a) Adding the mis parameter to ufd_version_check
> > > >   b) Asking for the feature
> > > > 
> > > > Please split it into two patches.
> > > > 
> > > > Also
> > > > 
> > > > > ---
> > > > >  include/migration/postcopy-ram.h |  2 +-
> > > > >  migration/migration.c|  2 +-
> > > > >  migration/postcopy-ram.c | 12 ++--
> > > > >  migration/savevm.c   |  2 +-
> > > > >  4 files changed, 9 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/include/migration/postcopy-ram.h 
> > > > > b/include/migration/postcopy-ram.h
> > > > > index 8e036b9..809f6db 100644
> > > > > --- a/include/migration/postcopy-ram.h
> > > > > +++ b/include/migration/postcopy-ram.h
> > > > > @@ -14,7 +14,7 @@
> > > > >  #define QEMU_POSTCOPY_RAM_H
> > > > >  
> > > > >  /* Return true if the host supports everything we need to do 
> > > > > postcopy-ram */
> > > > > -bool postcopy_ram_supported_by_host(void);
> > > > > +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
> > > > >  
> > > > >  /*
> > > > >   * Make all of RAM sensitive to accesses to areas that haven't yet 
> > > > > been written
> > > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > > index ad4036f..79f6425 100644
> > > > > --- a/migration/migration.c
> > > > > +++ b/migration/migration.c
> > > > > @@ -802,7 +802,7 @@ void 
> > > > > qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> > > > >   * special support.
> > > > >   */
> > > > >  if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) 
> > > > > &&
> > > > > -!postcopy_ram_supported_by_host()) {
> > > > > +!postcopy_ram_supported_by_host(NULL)) {
> > > > >  /* postcopy_ram_supported_by_host will have emitted a 
> > > > > more
> > > > >   * detailed message
> > > > >   */
> > > > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > > > > index dc80dbb..70f0480 100644
> > > > > --- a/migration/postcopy-ram.c
> > > > > +++ b/migration/postcopy-ram.c
> > > > > @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
> > > > >  #include 
> > > > >  #include 
> > > > >  
> > > > > -static bool ufd_version_check(int ufd)
> > > > > +static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
> > > > >  {
> > > > >  struct uffdio_api api_struct;
> > > > >  uint64_t ioctl_mask;
> > > > >  
> > > > >  api_struct.api = UFFD_API;
> > > > > -api_struct.features = 0;
> > > > > +api_struct.features = UFFD_FEATURE_THREAD_ID;
> > > > >  if (ioctl(ufd, UFFDIO_API, _struct)) {
> > > > >  error_report("postcopy_ram_supported_by_host: UFFDIO_API 
> > > > > failed: %s",
> > > > >   strerror(errno));
> > > > 
> > > > You're not actually using the 'mis' here - what I'd expected was
> > > > something that was going to check if the UFFDIO_API return said that it 
> > > > really
> > > > had the feature, and if so store a flag in the MIS somewhere.
> > > > 
> > > > Also, I'm not sure it's right to set 'api_struct.features' on the input 
> > > > - what
> > > > happens if this is run on an old kernel - we don't want postcopy to 
> > > > fail on
> > > > an old kernel without your feature.
> > > > I'm not 100% sure of the interface, but I think the way it works is you 
> > > > set
> > > > features = 0 before the call, and then check the api_struct.features in 
> > > > the
> > > > return - in the same way that I check for 
> > > > UFFD_FEATURE_MISSING_HUGETLBFS.
> > > > 
> > > We need to ask kernel about that feature,
> > > right,
> > > kernel returns back available features
> > > uffdio_api.features = UFFD_API_FEATURES
> > > but it also stores requested features
> > 
> > I feel like this does not against Dave's comment, maybe we just need
> > to send the UFFDIO_API twice? Like:
> yes, ioctl with UFFDIO_API will fail on old kernel if we will request
> e.g. UFFD_FEATURE_THREAD_ID or other new feature.
> 
> So in general way need a per feature request, for better error handling.

No, we don't need to - I think the way the kernel works is that you pass
features = 0 in, and it sets api_struct.features on the way out;
so if you always pass 0 in, you can then just check the features that
it returns.

Dave

> 
> > 
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 85fd8d7..fd0905f 100644
> > ---

Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-24 Thread Alexey 
On Mon, Apr 24, 2017 at 04:12:29PM +0800, Peter Xu wrote:
> On Fri, Apr 21, 2017 at 06:22:12PM +0300, Alexey wrote:
> > On Fri, Apr 21, 2017 at 11:24:54AM +0100, Dr. David Alan Gilbert wrote:
> > > * Alexey Perevalov (a.pereva...@samsung.com) wrote:
> > > > Userfaultfd mechanism is able to provide process thread id,
> > > > in case when client request it with UFDD_API ioctl.
> > > > 
> > > > Signed-off-by: Alexey Perevalov 
> > > 
> > > There seem to be two parts to this:
> > >   a) Adding the mis parameter to ufd_version_check
> > >   b) Asking for the feature
> > > 
> > > Please split it into two patches.
> > > 
> > > Also
> > > 
> > > > ---
> > > >  include/migration/postcopy-ram.h |  2 +-
> > > >  migration/migration.c|  2 +-
> > > >  migration/postcopy-ram.c | 12 ++--
> > > >  migration/savevm.c   |  2 +-
> > > >  4 files changed, 9 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/include/migration/postcopy-ram.h 
> > > > b/include/migration/postcopy-ram.h
> > > > index 8e036b9..809f6db 100644
> > > > --- a/include/migration/postcopy-ram.h
> > > > +++ b/include/migration/postcopy-ram.h
> > > > @@ -14,7 +14,7 @@
> > > >  #define QEMU_POSTCOPY_RAM_H
> > > >  
> > > >  /* Return true if the host supports everything we need to do 
> > > > postcopy-ram */
> > > > -bool postcopy_ram_supported_by_host(void);
> > > > +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
> > > >  
> > > >  /*
> > > >   * Make all of RAM sensitive to accesses to areas that haven't yet 
> > > > been written
> > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > index ad4036f..79f6425 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -802,7 +802,7 @@ void 
> > > > qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> > > >   * special support.
> > > >   */
> > > >  if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) &&
> > > > -!postcopy_ram_supported_by_host()) {
> > > > +!postcopy_ram_supported_by_host(NULL)) {
> > > >  /* postcopy_ram_supported_by_host will have emitted a more
> > > >   * detailed message
> > > >   */
> > > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > > > index dc80dbb..70f0480 100644
> > > > --- a/migration/postcopy-ram.c
> > > > +++ b/migration/postcopy-ram.c
> > > > @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
> > > >  #include 
> > > >  #include 
> > > >  
> > > > -static bool ufd_version_check(int ufd)
> > > > +static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
> > > >  {
> > > >  struct uffdio_api api_struct;
> > > >  uint64_t ioctl_mask;
> > > >  
> > > >  api_struct.api = UFFD_API;
> > > > -api_struct.features = 0;
> > > > +api_struct.features = UFFD_FEATURE_THREAD_ID;
> > > >  if (ioctl(ufd, UFFDIO_API, _struct)) {
> > > >  error_report("postcopy_ram_supported_by_host: UFFDIO_API 
> > > > failed: %s",
> > > >   strerror(errno));
> > > 
> > > You're not actually using the 'mis' here - what I'd expected was
> > > something that was going to check if the UFFDIO_API return said that it 
> > > really
> > > had the feature, and if so store a flag in the MIS somewhere.
> > > 
> > > Also, I'm not sure it's right to set 'api_struct.features' on the input - 
> > > what
> > > happens if this is run on an old kernel - we don't want postcopy to fail 
> > > on
> > > an old kernel without your feature.
> > > I'm not 100% sure of the interface, but I think the way it works is you 
> > > set
> > > features = 0 before the call, and then check the api_struct.features in 
> > > the
> > > return - in the same way that I check for UFFD_FEATURE_MISSING_HUGETLBFS.
> > > 
> > We need to ask kernel about that feature,
> > right,
> > kernel returns back available features
> > uffdio_api.features = UFFD_API_FEATURES
> > but it also stores requested features
> 
> I feel like this does not against Dave's comment, maybe we just need
> to send the UFFDIO_API twice? Like:
yes, ioctl with UFFDIO_API will fail on old kernel if we will request
e.g. UFFD_FEATURE_THREAD_ID or other new feature.

So in general way need a per feature request, for better error handling.

> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 85fd8d7..fd0905f 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -64,6 +64,7 @@ static bool ufd_version_check(int ufd)
>  {
>  struct uffdio_api api_struct;
>  uint64_t ioctl_mask;
> +uint64_t features = 0;
> 
>  api_struct.api = UFFD_API;
>  api_struct.features = 0;
> @@ -92,6 +93,27 @@ static bool ufd_version_check(int ufd)
>  return false;
>  }
>  }
> +
> +#ifdef UFFD_FEATURE_THREAD_ID
> +if (api_struct.features & UFFD_FEATURE_THREAD_ID) {
> +features |=

Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-24 Thread Peter Xu 
On Fri, Apr 21, 2017 at 06:22:12PM +0300, Alexey wrote:
> On Fri, Apr 21, 2017 at 11:24:54AM +0100, Dr. David Alan Gilbert wrote:
> > * Alexey Perevalov (a.pereva...@samsung.com) wrote:
> > > Userfaultfd mechanism is able to provide process thread id,
> > > in case when client request it with UFDD_API ioctl.
> > > 
> > > Signed-off-by: Alexey Perevalov 
> > 
> > There seem to be two parts to this:
> >   a) Adding the mis parameter to ufd_version_check
> >   b) Asking for the feature
> > 
> > Please split it into two patches.
> > 
> > Also
> > 
> > > ---
> > >  include/migration/postcopy-ram.h |  2 +-
> > >  migration/migration.c|  2 +-
> > >  migration/postcopy-ram.c | 12 ++--
> > >  migration/savevm.c   |  2 +-
> > >  4 files changed, 9 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/include/migration/postcopy-ram.h 
> > > b/include/migration/postcopy-ram.h
> > > index 8e036b9..809f6db 100644
> > > --- a/include/migration/postcopy-ram.h
> > > +++ b/include/migration/postcopy-ram.h
> > > @@ -14,7 +14,7 @@
> > >  #define QEMU_POSTCOPY_RAM_H
> > >  
> > >  /* Return true if the host supports everything we need to do 
> > > postcopy-ram */
> > > -bool postcopy_ram_supported_by_host(void);
> > > +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
> > >  
> > >  /*
> > >   * Make all of RAM sensitive to accesses to areas that haven't yet been 
> > > written
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index ad4036f..79f6425 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -802,7 +802,7 @@ void 
> > > qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> > >   * special support.
> > >   */
> > >  if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) &&
> > > -!postcopy_ram_supported_by_host()) {
> > > +!postcopy_ram_supported_by_host(NULL)) {
> > >  /* postcopy_ram_supported_by_host will have emitted a more
> > >   * detailed message
> > >   */
> > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > > index dc80dbb..70f0480 100644
> > > --- a/migration/postcopy-ram.c
> > > +++ b/migration/postcopy-ram.c
> > > @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
> > >  #include 
> > >  #include 
> > >  
> > > -static bool ufd_version_check(int ufd)
> > > +static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
> > >  {
> > >  struct uffdio_api api_struct;
> > >  uint64_t ioctl_mask;
> > >  
> > >  api_struct.api = UFFD_API;
> > > -api_struct.features = 0;
> > > +api_struct.features = UFFD_FEATURE_THREAD_ID;
> > >  if (ioctl(ufd, UFFDIO_API, _struct)) {
> > >  error_report("postcopy_ram_supported_by_host: UFFDIO_API failed: 
> > > %s",
> > >   strerror(errno));
> > 
> > You're not actually using the 'mis' here - what I'd expected was
> > something that was going to check if the UFFDIO_API return said that it 
> > really
> > had the feature, and if so store a flag in the MIS somewhere.
> > 
> > Also, I'm not sure it's right to set 'api_struct.features' on the input - 
> > what
> > happens if this is run on an old kernel - we don't want postcopy to fail on
> > an old kernel without your feature.
> > I'm not 100% sure of the interface, but I think the way it works is you set
> > features = 0 before the call, and then check the api_struct.features in the
> > return - in the same way that I check for UFFD_FEATURE_MISSING_HUGETLBFS.
> > 
> We need to ask kernel about that feature,
> right,
> kernel returns back available features
> uffdio_api.features = UFFD_API_FEATURES
> but it also stores requested features

I feel like this does not against Dave's comment, maybe we just need
to send the UFFDIO_API twice? Like:

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 85fd8d7..fd0905f 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -64,6 +64,7 @@ static bool ufd_version_check(int ufd)
 {
 struct uffdio_api api_struct;
 uint64_t ioctl_mask;
+uint64_t features = 0;

 api_struct.api = UFFD_API;
 api_struct.features = 0;
@@ -92,6 +93,27 @@ static bool ufd_version_check(int ufd)
 return false;
 }
 }
+
+#ifdef UFFD_FEATURE_THREAD_ID
+if (api_struct.features & UFFD_FEATURE_THREAD_ID) {
+features |= UFFD_FEATURE_THREAD_ID;
+}
+#endif
+
+if (features) {
+/*
+ * If there are new features to be enabled from userspace,
+ * trigger another UFFDIO_API ioctl.
+ */
+api_struct.api = UFFD_API;
+api_struct.features = features;
+if (ioctl(ufd, UFFDIO_API, _struct)) {
+error_report("UFFDIO_API failed to setup features: 0x%"PRIx64,
+ features);
+return false;
+}
+}
+
 return true;
 }

Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-24 Thread Peter Xu 
On Fri, Apr 21, 2017 at 06:22:12PM +0300, Alexey wrote:
> On Fri, Apr 21, 2017 at 11:24:54AM +0100, Dr. David Alan Gilbert wrote:
> > * Alexey Perevalov (a.pereva...@samsung.com) wrote:
> > > Userfaultfd mechanism is able to provide process thread id,
> > > in case when client request it with UFDD_API ioctl.
> > > 
> > > Signed-off-by: Alexey Perevalov 
> > 
> > There seem to be two parts to this:
> >   a) Adding the mis parameter to ufd_version_check
> >   b) Asking for the feature
> > 
> > Please split it into two patches.
> > 
> > Also
> > 
> > > ---
> > >  include/migration/postcopy-ram.h |  2 +-
> > >  migration/migration.c|  2 +-
> > >  migration/postcopy-ram.c | 12 ++--
> > >  migration/savevm.c   |  2 +-
> > >  4 files changed, 9 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/include/migration/postcopy-ram.h 
> > > b/include/migration/postcopy-ram.h
> > > index 8e036b9..809f6db 100644
> > > --- a/include/migration/postcopy-ram.h
> > > +++ b/include/migration/postcopy-ram.h
> > > @@ -14,7 +14,7 @@
> > >  #define QEMU_POSTCOPY_RAM_H
> > >  
> > >  /* Return true if the host supports everything we need to do 
> > > postcopy-ram */
> > > -bool postcopy_ram_supported_by_host(void);
> > > +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
> > >  
> > >  /*
> > >   * Make all of RAM sensitive to accesses to areas that haven't yet been 
> > > written
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index ad4036f..79f6425 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -802,7 +802,7 @@ void 
> > > qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> > >   * special support.
> > >   */
> > >  if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) &&
> > > -!postcopy_ram_supported_by_host()) {
> > > +!postcopy_ram_supported_by_host(NULL)) {
> > >  /* postcopy_ram_supported_by_host will have emitted a more
> > >   * detailed message
> > >   */
> > > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > > index dc80dbb..70f0480 100644
> > > --- a/migration/postcopy-ram.c
> > > +++ b/migration/postcopy-ram.c
> > > @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
> > >  #include 
> > >  #include 
> > >  
> > > -static bool ufd_version_check(int ufd)
> > > +static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
> > >  {
> > >  struct uffdio_api api_struct;
> > >  uint64_t ioctl_mask;
> > >  
> > >  api_struct.api = UFFD_API;
> > > -api_struct.features = 0;
> > > +api_struct.features = UFFD_FEATURE_THREAD_ID;
> > >  if (ioctl(ufd, UFFDIO_API, _struct)) {
> > >  error_report("postcopy_ram_supported_by_host: UFFDIO_API failed: 
> > > %s",
> > >   strerror(errno));
> > 
> > You're not actually using the 'mis' here - what I'd expected was
> > something that was going to check if the UFFDIO_API return said that it 
> > really
> > had the feature, and if so store a flag in the MIS somewhere.
> > 
> > Also, I'm not sure it's right to set 'api_struct.features' on the input - 
> > what
> > happens if this is run on an old kernel - we don't want postcopy to fail on
> > an old kernel without your feature.
> > I'm not 100% sure of the interface, but I think the way it works is you set
> > features = 0 before the call, and then check the api_struct.features in the
> > return - in the same way that I check for UFFD_FEATURE_MISSING_HUGETLBFS.
> > 
> We need to ask kernel about that feature,
> right,
> kernel returns back available features
> uffdio_api.features = UFFD_API_FEATURES
> but it also stores requested features

I feel like this does not against Dave's comment, maybe we just need
to send the UFFDIO_API twice? Like:

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 85fd8d7..fd0905f 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -64,6 +64,7 @@ static bool ufd_version_check(int ufd)
 {
 struct uffdio_api api_struct;
 uint64_t ioctl_mask;
+uint64_t features = 0;

 api_struct.api = UFFD_API;
 api_struct.features = 0;
@@ -92,6 +93,27 @@ static bool ufd_version_check(int ufd)
 return false;
 }
 }
+
+#ifdef UFFD_FEATURE_THREAD_ID
+if (api_struct.features & UFFD_FEATURE_THREAD_ID) {
+features |= UFFD_FEATURE_THREAD_ID;
+}
+#endif
+
+if (features) {
+/*
+ * If there are new features to be enabled from userspace,
+ * trigger another UFFDIO_API ioctl.
+ */
+api_struct.api = UFFD_API;
+api_struct.features = features;
+if (ioctl(ufd, UFFDIO_API, _struct)) {
+error_report("UFFDIO_API failed to setup features: 0x%"PRIx64,
+ features);
+return false;
+}
+}
+
 return true;
 }

Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-21 Thread Alexey 
On Fri, Apr 21, 2017 at 11:24:54AM +0100, Dr. David Alan Gilbert wrote:
> * Alexey Perevalov (a.pereva...@samsung.com) wrote:
> > Userfaultfd mechanism is able to provide process thread id,
> > in case when client request it with UFDD_API ioctl.
> > 
> > Signed-off-by: Alexey Perevalov 
> 
> There seem to be two parts to this:
>   a) Adding the mis parameter to ufd_version_check
>   b) Asking for the feature
> 
> Please split it into two patches.
> 
> Also
> 
> > ---
> >  include/migration/postcopy-ram.h |  2 +-
> >  migration/migration.c|  2 +-
> >  migration/postcopy-ram.c | 12 ++--
> >  migration/savevm.c   |  2 +-
> >  4 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/migration/postcopy-ram.h 
> > b/include/migration/postcopy-ram.h
> > index 8e036b9..809f6db 100644
> > --- a/include/migration/postcopy-ram.h
> > +++ b/include/migration/postcopy-ram.h
> > @@ -14,7 +14,7 @@
> >  #define QEMU_POSTCOPY_RAM_H
> >  
> >  /* Return true if the host supports everything we need to do postcopy-ram 
> > */
> > -bool postcopy_ram_supported_by_host(void);
> > +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
> >  
> >  /*
> >   * Make all of RAM sensitive to accesses to areas that haven't yet been 
> > written
> > diff --git a/migration/migration.c b/migration/migration.c
> > index ad4036f..79f6425 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -802,7 +802,7 @@ void 
> > qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> >   * special support.
> >   */
> >  if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) &&
> > -!postcopy_ram_supported_by_host()) {
> > +!postcopy_ram_supported_by_host(NULL)) {
> >  /* postcopy_ram_supported_by_host will have emitted a more
> >   * detailed message
> >   */
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index dc80dbb..70f0480 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
> >  #include 
> >  #include 
> >  
> > -static bool ufd_version_check(int ufd)
> > +static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
> >  {
> >  struct uffdio_api api_struct;
> >  uint64_t ioctl_mask;
> >  
> >  api_struct.api = UFFD_API;
> > -api_struct.features = 0;
> > +api_struct.features = UFFD_FEATURE_THREAD_ID;
> >  if (ioctl(ufd, UFFDIO_API, _struct)) {
> >  error_report("postcopy_ram_supported_by_host: UFFDIO_API failed: 
> > %s",
> >   strerror(errno));
> 
> You're not actually using the 'mis' here - what I'd expected was
> something that was going to check if the UFFDIO_API return said that it really
> had the feature, and if so store a flag in the MIS somewhere.
> 
> Also, I'm not sure it's right to set 'api_struct.features' on the input - what
> happens if this is run on an old kernel - we don't want postcopy to fail on
> an old kernel without your feature.
> I'm not 100% sure of the interface, but I think the way it works is you set
> features = 0 before the call, and then check the api_struct.features in the
> return - in the same way that I check for UFFD_FEATURE_MISSING_HUGETLBFS.
> 
We need to ask kernel about that feature,
right,
kernel returns back available features
uffdio_api.features = UFFD_API_FEATURES
but it also stores requested features
/* only enable the requested features for this uffd context */
 ctx->features = uffd_ctx_features(features);

so, at the time when process thread id is going to be sent
kernel checks if it was requested
+   if (features & UFFD_FEATURE_THREAD_ID)
+   msg.arg.pagefault.ptid = task_pid_vnr(current);

from patch message:

 Process's thread id is being provided when user requeste it
by setting UFFD_FEATURE_THREAD_ID bit into uffdio_api.features.

UFFD_FEATURE_MISSING_HUGETLBFS - look like default, unconditional
behavior (I didn't find any usage of that define in kernel).


> Dave
> 
> > @@ -113,7 +113,7 @@ static int test_range_shared(const char *block_name, 
> > void *host_addr,
> >   * normally fine since if the postcopy succeeds it gets turned back on at 
> > the
> >   * end.
> >   */
> > -bool postcopy_ram_supported_by_host(void)
> > +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
> >  {
> >  long pagesize = getpagesize();
> >  int ufd = -1;
> > @@ -136,7 +136,7 @@ bool postcopy_ram_supported_by_host(void)
> >  }
> >  
> >  /* Version and features check */
> > -if (!ufd_version_check(ufd)) {
> > +if (!ufd_version_check(ufd, mis)) {
> >  goto out;
> >  }
> >  
> > @@ -515,7 +515,7 @@ int postcopy_ram_enable_notify(MigrationIncomingState 
> > *mis)
> >   * Although the host check already tested the API, we need to
> >   * do the check again as an

Re: [Qemu-devel] [PATCH 3/6] migration: add UFFD_FEATURE_THREAD_ID feature support

2017-04-21 Thread Dr. David Alan Gilbert 
* Alexey Perevalov (a.pereva...@samsung.com) wrote:
> Userfaultfd mechanism is able to provide process thread id,
> in case when client request it with UFDD_API ioctl.
> 
> Signed-off-by: Alexey Perevalov 

There seem to be two parts to this:
  a) Adding the mis parameter to ufd_version_check
  b) Asking for the feature

Please split it into two patches.

Also

> ---
>  include/migration/postcopy-ram.h |  2 +-
>  migration/migration.c|  2 +-
>  migration/postcopy-ram.c | 12 ++--
>  migration/savevm.c   |  2 +-
>  4 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/migration/postcopy-ram.h 
> b/include/migration/postcopy-ram.h
> index 8e036b9..809f6db 100644
> --- a/include/migration/postcopy-ram.h
> +++ b/include/migration/postcopy-ram.h
> @@ -14,7 +14,7 @@
>  #define QEMU_POSTCOPY_RAM_H
>  
>  /* Return true if the host supports everything we need to do postcopy-ram */
> -bool postcopy_ram_supported_by_host(void);
> +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
>  
>  /*
>   * Make all of RAM sensitive to accesses to areas that haven't yet been 
> written
> diff --git a/migration/migration.c b/migration/migration.c
> index ad4036f..79f6425 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -802,7 +802,7 @@ void 
> qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
>   * special support.
>   */
>  if (!old_postcopy_cap && runstate_check(RUN_STATE_INMIGRATE) &&
> -!postcopy_ram_supported_by_host()) {
> +!postcopy_ram_supported_by_host(NULL)) {
>  /* postcopy_ram_supported_by_host will have emitted a more
>   * detailed message
>   */
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index dc80dbb..70f0480 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -60,13 +60,13 @@ struct PostcopyDiscardState {
>  #include 
>  #include 
>  
> -static bool ufd_version_check(int ufd)
> +static bool ufd_version_check(int ufd, MigrationIncomingState *mis)
>  {
>  struct uffdio_api api_struct;
>  uint64_t ioctl_mask;
>  
>  api_struct.api = UFFD_API;
> -api_struct.features = 0;
> +api_struct.features = UFFD_FEATURE_THREAD_ID;
>  if (ioctl(ufd, UFFDIO_API, _struct)) {
>  error_report("postcopy_ram_supported_by_host: UFFDIO_API failed: %s",
>   strerror(errno));

You're not actually using the 'mis' here - what I'd expected was
something that was going to check if the UFFDIO_API return said that it really
had the feature, and if so store a flag in the MIS somewhere.

Also, I'm not sure it's right to set 'api_struct.features' on the input - what
happens if this is run on an old kernel - we don't want postcopy to fail on
an old kernel without your feature.
I'm not 100% sure of the interface, but I think the way it works is you set
features = 0 before the call, and then check the api_struct.features in the
return - in the same way that I check for UFFD_FEATURE_MISSING_HUGETLBFS.

Dave

> @@ -113,7 +113,7 @@ static int test_range_shared(const char *block_name, void 
> *host_addr,
>   * normally fine since if the postcopy succeeds it gets turned back on at the
>   * end.
>   */
> -bool postcopy_ram_supported_by_host(void)
> +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
>  {
>  long pagesize = getpagesize();
>  int ufd = -1;
> @@ -136,7 +136,7 @@ bool postcopy_ram_supported_by_host(void)
>  }
>  
>  /* Version and features check */
> -if (!ufd_version_check(ufd)) {
> +if (!ufd_version_check(ufd, mis)) {
>  goto out;
>  }
>  
> @@ -515,7 +515,7 @@ int postcopy_ram_enable_notify(MigrationIncomingState 
> *mis)
>   * Although the host check already tested the API, we need to
>   * do the check again as an ABI handshake on the new fd.
>   */
> -if (!ufd_version_check(mis->userfault_fd)) {
> +if (!ufd_version_check(mis->userfault_fd, mis)) {
>  return -1;
>  }
>  
> @@ -653,7 +653,7 @@ void *postcopy_get_tmp_page(MigrationIncomingState *mis)
>  
>  #else
>  /* No target OS support, stubs just fail */
> -bool postcopy_ram_supported_by_host(void)
> +bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
>  {
>  error_report("%s: No OS support", __func__);
>  return false;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 3b19a4a..f01e418 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1360,7 +1360,7 @@ static int 
> loadvm_postcopy_handle_advise(MigrationIncomingState *mis)
>  return -1;
>  }
>  
> -if (!postcopy_ram_supported_by_host()) {
> +if (!postcopy_ram_supported_by_host(mis)) {
>  postcopy_state_set(POSTCOPY_INCOMING_NONE);
>  return -1;
>  }
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK