Currently it is not forbidden to use "-object memory-backend-file,share=on"
and together with "-incoming". But after incoming migration is finished,
the memory-backend-file will be definitely written if share=on. So the
memory-backend-file can only be used once, but failed in the 2nd time
incoming.

Thus it gives a warning and the users can run the qemu if they really
want to do it.

Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
---
 backends/hostmem-file.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 37ac6445d2..59429ee0b4 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -16,6 +16,7 @@
 #include "sysemu/hostmem.h"
 #include "sysemu/sysemu.h"
 #include "qom/object_interfaces.h"
+#include "migration/migration.h"
 
 /* hostmem-file.c */
 /**
@@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
         }
     }
 
+    /*
+     * In ignore shared case, if share=on for host memory backend file,
+     * the ram might be written after incoming process is finished. Thus
+     * the memory backend can't be reused for 2nd/3rd... incoming
+     */
+    if (backend->share && migrate_ignore_shared()
+                       && runstate_check(RUN_STATE_INMIGRATE))
+        warn_report("share=on for memory backend file might be "
+                        "conflicted with incoming in ignore shared case");
+
     backend->force_prealloc = mem_prealloc;
     name = host_memory_backend_get_name(backend);
     memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
-- 
2.17.1

On Sun,  7 Apr 2019 22:19:05 -0400
Catherine Ho <catherine.hecx@gmail.com> wrote:

> Currently it is not forbidden to use "-object memory-backend-file,share=on"
> and together with "-incoming". But after incoming migration is finished,
> the memory-backend-file will be definitely written if share=on. So the
> memory-backend-file can only be used once, but failed in the 2nd time
> incoming.
> 
> Thus it gives a warning and the users can run the qemu if they really
> want to do it.

Shouldn't we add a migration blocker in such a case instead of warning
and letting qemu run wild?


> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> ---
>  backends/hostmem-file.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 37ac6445d2..59429ee0b4 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -16,6 +16,7 @@
>  #include "sysemu/hostmem.h"
>  #include "sysemu/sysemu.h"
>  #include "qom/object_interfaces.h"
> +#include "migration/migration.h"
>  
>  /* hostmem-file.c */
>  /**
> @@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>          }
>      }
>  
> +    /*
> +     * In ignore shared case, if share=on for host memory backend file,
> +     * the ram might be written after incoming process is finished. Thus
> +     * the memory backend can't be reused for 2nd/3rd... incoming
> +     */
> +    if (backend->share && migrate_ignore_shared()
> +                       && runstate_check(RUN_STATE_INMIGRATE))
> +        warn_report("share=on for memory backend file might be "
> +                        "conflicted with incoming in ignore shared case");
> +
>      backend->force_prealloc = mem_prealloc;
>      name = host_memory_backend_get_name(backend);
>      memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),

Hi Igor


On Mon, 8 Apr 2019 at 18:35, Igor Mammedov <imammedo@redhat.com> wrote:

> On Sun,  7 Apr 2019 22:19:05 -0400
> Catherine Ho <catherine.hecx@gmail.com> wrote:
>
> > Currently it is not forbidden to use "-object
> memory-backend-file,share=on"
> > and together with "-incoming". But after incoming migration is finished,
> > the memory-backend-file will be definitely written if share=on. So the
> > memory-backend-file can only be used once, but failed in the 2nd time
> > incoming.
> >
> > Thus it gives a warning and the users can run the qemu if they really
> > want to do it.
>
> Shouldn't we add a migration blocker in such a case instead of warning
> and letting qemu run wild?
>

IMO, it doesn't need to block this. With share=on and -incoming, the user
can
still save the device memory state into memory-backend file again if
ignore-shared
capability is on.

If we block this, the user can't use the ignore-shared capability in
incoming
migration.

B.R.
Catherine

>
>
> > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> > ---
> >  backends/hostmem-file.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > index 37ac6445d2..59429ee0b4 100644
> > --- a/backends/hostmem-file.c
> > +++ b/backends/hostmem-file.c
> > @@ -16,6 +16,7 @@
> >  #include "sysemu/hostmem.h"
> >  #include "sysemu/sysemu.h"
> >  #include "qom/object_interfaces.h"
> > +#include "migration/migration.h"
> >
> >  /* hostmem-file.c */
> >  /**
> > @@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
> Error **errp)
> >          }
> >      }
> >
> > +    /*
> > +     * In ignore shared case, if share=on for host memory backend file,
> > +     * the ram might be written after incoming process is finished. Thus
> > +     * the memory backend can't be reused for 2nd/3rd... incoming
> > +     */
> > +    if (backend->share && migrate_ignore_shared()
> > +                       && runstate_check(RUN_STATE_INMIGRATE))
> > +        warn_report("share=on for memory backend file might be "
> > +                        "conflicted with incoming in ignore shared
> case");
> > +
> >      backend->force_prealloc = mem_prealloc;
> >      name = host_memory_backend_get_name(backend);
> >      memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>
>

* Catherine Ho (catherine.hecx@gmail.com) wrote:
> Hi Igor
> 
> 
> On Mon, 8 Apr 2019 at 18:35, Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Sun,  7 Apr 2019 22:19:05 -0400
> > Catherine Ho <catherine.hecx@gmail.com> wrote:
> >
> > > Currently it is not forbidden to use "-object
> > memory-backend-file,share=on"
> > > and together with "-incoming". But after incoming migration is finished,
> > > the memory-backend-file will be definitely written if share=on. So the
> > > memory-backend-file can only be used once, but failed in the 2nd time
> > > incoming.
> > >
> > > Thus it gives a warning and the users can run the qemu if they really
> > > want to do it.
> >
> > Shouldn't we add a migration blocker in such a case instead of warning
> > and letting qemu run wild?
> >
> 
> IMO, it doesn't need to block this. With share=on and -incoming, the user
> can
> still save the device memory state into memory-backend file again if
> ignore-shared
> capability is on.
> 
> If we block this, the user can't use the ignore-shared capability in
> incoming
> migration.

-incomign with share=on is a perfectly normal thing to do - it just
depends who you are sharing the file with and the lifetime of that
shared file.

For example; if you're just running a qemu with vhost-user then you
use share=on - however wyou typically select the backend file as
a new file from /dev/shm - it's not a file that you previously migrated
to.

QEMU has no way to know about the provenance of the shared file it's
been given, so we can't really warn people about it.

Dave

> B.R.
> Catherine
> 
> >
> >
> > > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> > > ---
> > >  backends/hostmem-file.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > > index 37ac6445d2..59429ee0b4 100644
> > > --- a/backends/hostmem-file.c
> > > +++ b/backends/hostmem-file.c
> > > @@ -16,6 +16,7 @@
> > >  #include "sysemu/hostmem.h"
> > >  #include "sysemu/sysemu.h"
> > >  #include "qom/object_interfaces.h"
> > > +#include "migration/migration.h"
> > >
> > >  /* hostmem-file.c */
> > >  /**
> > > @@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
> > Error **errp)
> > >          }
> > >      }
> > >
> > > +    /*
> > > +     * In ignore shared case, if share=on for host memory backend file,
> > > +     * the ram might be written after incoming process is finished. Thus
> > > +     * the memory backend can't be reused for 2nd/3rd... incoming
> > > +     */
> > > +    if (backend->share && migrate_ignore_shared()
> > > +                       && runstate_check(RUN_STATE_INMIGRATE))
> > > +        warn_report("share=on for memory backend file might be "
> > > +                        "conflicted with incoming in ignore shared
> > case");
> > > +
> > >      backend->force_prealloc = mem_prealloc;
> > >      name = host_memory_backend_get_name(backend);
> > >      memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> >
> >
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Hi Dr. David

On Wed, 10 Apr 2019 at 22:59, Dr. David Alan Gilbert <dgilbert@redhat.com>
wrote:

> * Catherine Ho (catherine.hecx@gmail.com) wrote:
> > Hi Igor
> >
> >
> > On Mon, 8 Apr 2019 at 18:35, Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > > On Sun,  7 Apr 2019 22:19:05 -0400
> > > Catherine Ho <catherine.hecx@gmail.com> wrote:
> > >
> > > > Currently it is not forbidden to use "-object
> > > memory-backend-file,share=on"
> > > > and together with "-incoming". But after incoming migration is
> finished,
> > > > the memory-backend-file will be definitely written if share=on. So
> the
> > > > memory-backend-file can only be used once, but failed in the 2nd time
> > > > incoming.
> > > >
> > > > Thus it gives a warning and the users can run the qemu if they really
> > > > want to do it.
> > >
> > > Shouldn't we add a migration blocker in such a case instead of warning
> > > and letting qemu run wild?
> > >
> >
> > IMO, it doesn't need to block this. With share=on and -incoming, the user
> > can
> > still save the device memory state into memory-backend file again if
> > ignore-shared
> > capability is on.
> >
> > If we block this, the user can't use the ignore-shared capability in
> > incoming
> > migration.
>
> -incomign with share=on is a perfectly normal thing to do - it just
> depends who you are sharing the file with and the lifetime of that
> shared file.
>
> For example; if you're just running a qemu with vhost-user then you
> use share=on - however wyou typically select the backend file as
> a new file from /dev/shm - it's not a file that you previously migrated
> to.
>
Thanks,
but using a new file from /dev/shm means kernel will start from
start_kernel or early? Is it different from the x-ignore-shared case?
If we remove the share=on in incoming migration, all the writting
of ram will not be flush into the memory backend file. Thus we
can use the base memory backend file for ever.
e.g.
1) save the vm like a snapshot, current ram state is "kernel
has been started, systemd has been started"
2) restore it with -incoming and *no* share=on flag
3) restore it with -incoming and *no* share=on again...
In contrary, if we use share=on, the base backend file will
be written at once after 1st time incoming.

So, IMO, no "share=on" is the proper usage of incoming migration
when ignore-shared is on.
Please correct me if sth is wrong, thanks:)

B.R.
Catherine


> QEMU has no way to know about the provenance of the shared file it's
> been given, so we can't really warn people about it.
>
> Dave
>
> > B.R.
> > Catherine
> >
> > >
> > >
> > > > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> > > > ---
> > > >  backends/hostmem-file.c | 11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > > > index 37ac6445d2..59429ee0b4 100644
> > > > --- a/backends/hostmem-file.c
> > > > +++ b/backends/hostmem-file.c
> > > > @@ -16,6 +16,7 @@
> > > >  #include "sysemu/hostmem.h"
> > > >  #include "sysemu/sysemu.h"
> > > >  #include "qom/object_interfaces.h"
> > > > +#include "migration/migration.h"
> > > >
> > > >  /* hostmem-file.c */
> > > >  /**
> > > > @@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend
> *backend,
> > > Error **errp)
> > > >          }
> > > >      }
> > > >
> > > > +    /*
> > > > +     * In ignore shared case, if share=on for host memory backend
> file,
> > > > +     * the ram might be written after incoming process is finished.
> Thus
> > > > +     * the memory backend can't be reused for 2nd/3rd... incoming
> > > > +     */
> > > > +    if (backend->share && migrate_ignore_shared()
> > > > +                       && runstate_check(RUN_STATE_INMIGRATE))
> > > > +        warn_report("share=on for memory backend file might be "
> > > > +                        "conflicted with incoming in ignore shared
> > > case");
> > > > +
> > > >      backend->force_prealloc = mem_prealloc;
> > > >      name = host_memory_backend_get_name(backend);
> > > >      memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> > >
> > >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>

* Catherine Ho (catherine.hecx@gmail.com) wrote:
> Hi Dr. David
> 
> On Wed, 10 Apr 2019 at 22:59, Dr. David Alan Gilbert <dgilbert@redhat.com>
> wrote:
> 
> > * Catherine Ho (catherine.hecx@gmail.com) wrote:
> > > Hi Igor
> > >
> > >
> > > On Mon, 8 Apr 2019 at 18:35, Igor Mammedov <imammedo@redhat.com> wrote:
> > >
> > > > On Sun,  7 Apr 2019 22:19:05 -0400
> > > > Catherine Ho <catherine.hecx@gmail.com> wrote:
> > > >
> > > > > Currently it is not forbidden to use "-object
> > > > memory-backend-file,share=on"
> > > > > and together with "-incoming". But after incoming migration is
> > finished,
> > > > > the memory-backend-file will be definitely written if share=on. So
> > the
> > > > > memory-backend-file can only be used once, but failed in the 2nd time
> > > > > incoming.
> > > > >
> > > > > Thus it gives a warning and the users can run the qemu if they really
> > > > > want to do it.
> > > >
> > > > Shouldn't we add a migration blocker in such a case instead of warning
> > > > and letting qemu run wild?
> > > >
> > >
> > > IMO, it doesn't need to block this. With share=on and -incoming, the user
> > > can
> > > still save the device memory state into memory-backend file again if
> > > ignore-shared
> > > capability is on.
> > >
> > > If we block this, the user can't use the ignore-shared capability in
> > > incoming
> > > migration.
> >
> > -incomign with share=on is a perfectly normal thing to do - it just
> > depends who you are sharing the file with and the lifetime of that
> > shared file.
> >
> > For example; if you're just running a qemu with vhost-user then you
> > use share=on - however wyou typically select the backend file as
> > a new file from /dev/shm - it's not a file that you previously migrated
> > to.
> >
> Thanks,
> but using a new file from /dev/shm means kernel will start from
> start_kernel or early? Is it different from the x-ignore-shared case?
> If we remove the share=on in incoming migration, all the writting
> of ram will not be flush into the memory backend file. Thus we
> can use the base memory backend file for ever.
> e.g.
> 1) save the vm like a snapshot, current ram state is "kernel
> has been started, systemd has been started"
> 2) restore it with -incoming and *no* share=on flag
> 3) restore it with -incoming and *no* share=on again...
> In contrary, if we use share=on, the base backend file will
> be written at once after 1st time incoming.
> 
> So, IMO, no "share=on" is the proper usage of incoming migration
> when ignore-shared is on.
> Please correct me if sth is wrong, thanks:)

OK, I see what you're trying to do - you mean for the 'snapshotting'
case;  but that's not the only use.  Another use is for being able to
do a very quick upgrade of the running qemu to a new qemu binary
version; and in that case you want to be able to write to the shared
file so that you can repeatedly do the quick migrate.

Dave

> 
> B.R.
> Catherine
> 
> 
> > QEMU has no way to know about the provenance of the shared file it's
> > been given, so we can't really warn people about it.
> >
> > Dave
> >
> > > B.R.
> > > Catherine
> > >
> > > >
> > > >
> > > > > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> > > > > ---
> > > > >  backends/hostmem-file.c | 11 +++++++++++
> > > > >  1 file changed, 11 insertions(+)
> > > > >
> > > > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > > > > index 37ac6445d2..59429ee0b4 100644
> > > > > --- a/backends/hostmem-file.c
> > > > > +++ b/backends/hostmem-file.c
> > > > > @@ -16,6 +16,7 @@
> > > > >  #include "sysemu/hostmem.h"
> > > > >  #include "sysemu/sysemu.h"
> > > > >  #include "qom/object_interfaces.h"
> > > > > +#include "migration/migration.h"
> > > > >
> > > > >  /* hostmem-file.c */
> > > > >  /**
> > > > > @@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend
> > *backend,
> > > > Error **errp)
> > > > >          }
> > > > >      }
> > > > >
> > > > > +    /*
> > > > > +     * In ignore shared case, if share=on for host memory backend
> > file,
> > > > > +     * the ram might be written after incoming process is finished.
> > Thus
> > > > > +     * the memory backend can't be reused for 2nd/3rd... incoming
> > > > > +     */
> > > > > +    if (backend->share && migrate_ignore_shared()
> > > > > +                       && runstate_check(RUN_STATE_INMIGRATE))
> > > > > +        warn_report("share=on for memory backend file might be "
> > > > > +                        "conflicted with incoming in ignore shared
> > > > case");
> > > > > +
> > > > >      backend->force_prealloc = mem_prealloc;
> > > > >      name = host_memory_backend_get_name(backend);
> > > > >      memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> > > >
> > > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Hi Dr. David

On Thu, 11 Apr 2019 at 00:57, Dr. David Alan Gilbert <dgilbert@redhat.com>
wrote:

> * Catherine Ho (catherine.hecx@gmail.com) wrote:
> > Hi Dr. David
> >
> > On Wed, 10 Apr 2019 at 22:59, Dr. David Alan Gilbert <
> dgilbert@redhat.com>
> > wrote:
> >
> > > * Catherine Ho (catherine.hecx@gmail.com) wrote:
> > > > Hi Igor
> > > >
> > > >
> > > > On Mon, 8 Apr 2019 at 18:35, Igor Mammedov <imammedo@redhat.com>
> wrote:
> > > >
> > > > > On Sun,  7 Apr 2019 22:19:05 -0400
> > > > > Catherine Ho <catherine.hecx@gmail.com> wrote:
> > > > >
> > > > > > Currently it is not forbidden to use "-object
> > > > > memory-backend-file,share=on"
> > > > > > and together with "-incoming". But after incoming migration is
> > > finished,
> > > > > > the memory-backend-file will be definitely written if share=on.
> So
> > > the
> > > > > > memory-backend-file can only be used once, but failed in the 2nd
> time
> > > > > > incoming.
> > > > > >
> > > > > > Thus it gives a warning and the users can run the qemu if they
> really
> > > > > > want to do it.
> > > > >
> > > > > Shouldn't we add a migration blocker in such a case instead of
> warning
> > > > > and letting qemu run wild?
> > > > >
> > > >
> > > > IMO, it doesn't need to block this. With share=on and -incoming, the
> user
> > > > can
> > > > still save the device memory state into memory-backend file again if
> > > > ignore-shared
> > > > capability is on.
> > > >
> > > > If we block this, the user can't use the ignore-shared capability in
> > > > incoming
> > > > migration.
> > >
> > > -incomign with share=on is a perfectly normal thing to do - it just
> > > depends who you are sharing the file with and the lifetime of that
> > > shared file.
> > >
> > > For example; if you're just running a qemu with vhost-user then you
> > > use share=on - however wyou typically select the backend file as
> > > a new file from /dev/shm - it's not a file that you previously migrated
> > > to.
> > >
> > Thanks,
> > but using a new file from /dev/shm means kernel will start from
> > start_kernel or early? Is it different from the x-ignore-shared case?
> > If we remove the share=on in incoming migration, all the writting
> > of ram will not be flush into the memory backend file. Thus we
> > can use the base memory backend file for ever.
> > e.g.
> > 1) save the vm like a snapshot, current ram state is "kernel
> > has been started, systemd has been started"
> > 2) restore it with -incoming and *no* share=on flag
> > 3) restore it with -incoming and *no* share=on again...
> > In contrary, if we use share=on, the base backend file will
> > be written at once after 1st time incoming.
> >
> > So, IMO, no "share=on" is the proper usage of incoming migration
> > when ignore-shared is on.
> > Please correct me if sth is wrong, thanks:)
>
> OK, I see what you're trying to do - you mean for the 'snapshotting'
> case;  but that's not the only use.  Another use is for being able to
> do a very quick upgrade of the running qemu to a new qemu binary
> version; and in that case you want to be able to write to the shared
> file so that you can repeatedly do the quick migrate.
>
> Dave
>
> Ah, that quick upgrade example makes sense to me. Thanks for the
explanation.

B.R.
Catherine

On Sun, Apr 07, 2019 at 10:19:05PM -0400, Catherine Ho wrote:
> Currently it is not forbidden to use "-object memory-backend-file,share=on"
> and together with "-incoming". But after incoming migration is finished,
> the memory-backend-file will be definitely written if share=on. So the
> memory-backend-file can only be used once, but failed in the 2nd time
> incoming.
> 
> Thus it gives a warning and the users can run the qemu if they really
> want to do it.
> 
> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> ---
>  backends/hostmem-file.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 37ac6445d2..59429ee0b4 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -16,6 +16,7 @@
>  #include "sysemu/hostmem.h"
>  #include "sysemu/sysemu.h"
>  #include "qom/object_interfaces.h"
> +#include "migration/migration.h"
>  
>  /* hostmem-file.c */
>  /**
> @@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>          }
>      }
>  
> +    /*
> +     * In ignore shared case, if share=on for host memory backend file,
> +     * the ram might be written after incoming process is finished. Thus
> +     * the memory backend can't be reused for 2nd/3rd... incoming
> +     */
> +    if (backend->share && migrate_ignore_shared()
> +                       && runstate_check(RUN_STATE_INMIGRATE))
> +        warn_report("share=on for memory backend file might be "
> +                        "conflicted with incoming in ignore shared case");

I feel like this message wasn't really clear to me...  you want to
warn people these data might not match with each other, right?  How
about simply state it:

  NOTE: Please make sure the data on the shared memory backend file
  and the data from the incoming migration stream contains matching
  contents, otherwise...

Regards,

-- 
Peter Xu

Hi Peter Xu

On Mon, 8 Apr 2019 at 11:25, Peter Xu <peterx@redhat.com> wrote:

> On Sun, Apr 07, 2019 at 10:19:05PM -0400, Catherine Ho wrote:
> > Currently it is not forbidden to use "-object
> memory-backend-file,share=on"
> > and together with "-incoming". But after incoming migration is finished,
> > the memory-backend-file will be definitely written if share=on. So the
> > memory-backend-file can only be used once, but failed in the 2nd time
> > incoming.
> >
> > Thus it gives a warning and the users can run the qemu if they really
> > want to do it.
> >
> > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> > ---
> >  backends/hostmem-file.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > index 37ac6445d2..59429ee0b4 100644
> > --- a/backends/hostmem-file.c
> > +++ b/backends/hostmem-file.c
> > @@ -16,6 +16,7 @@
> >  #include "sysemu/hostmem.h"
> >  #include "sysemu/sysemu.h"
> >  #include "qom/object_interfaces.h"
> > +#include "migration/migration.h"
> >
> >  /* hostmem-file.c */
> >  /**
> > @@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
> Error **errp)
> >          }
> >      }
> >
> > +    /*
> > +     * In ignore shared case, if share=on for host memory backend file,
> > +     * the ram might be written after incoming process is finished. Thus
> > +     * the memory backend can't be reused for 2nd/3rd... incoming
> > +     */
> > +    if (backend->share && migrate_ignore_shared()
> > +                       && runstate_check(RUN_STATE_INMIGRATE))
> > +        warn_report("share=on for memory backend file might be "
> > +                        "conflicted with incoming in ignore shared
> case");
>
> I feel like this message wasn't really clear to me...  you want to
> warn people these data might not match with each other, right?  How
> about simply state it:
>
>   NOTE: Please make sure the data on the shared memory backend file
>   and the data from the incoming migration stream contains matching
>   contents, otherwise...
>

Sorry for my vague expression.
The background is [1]
I happened to use "-object memory-backend-file,share=on" and together with
"-incoming".
It worked fine in 1st incoming migration but failed in 2nd, 3rd incoming
migration.
Because qemu uses qemu_ram_mmap(..., MAP_SHARED,...) when share=on, the
memory-backend-file will be written after 1st incoming migration.
Finally I realized that this was caused by "share=on" flag, and after I
removed it,
the memory-backend-file will not be changed any more.

So do you think it will be better that qemu gives the user a clear warning
that
incoming migration will change the data in  memory-backend-file  with
"share=on" ?

[1] http://lists.nongnu.org/archive/html/qemu-devel/2019-04/msg00350.html


> Regards,
>
> --
> Peter Xu
>

On Mon, Apr 08, 2019 at 02:15:36PM +0800, Catherine Ho wrote:
> Hi Peter Xu
> 
> On Mon, 8 Apr 2019 at 11:25, Peter Xu <peterx@redhat.com> wrote:
> 
> > On Sun, Apr 07, 2019 at 10:19:05PM -0400, Catherine Ho wrote:
> > > Currently it is not forbidden to use "-object
> > memory-backend-file,share=on"
> > > and together with "-incoming". But after incoming migration is finished,
> > > the memory-backend-file will be definitely written if share=on. So the
> > > memory-backend-file can only be used once, but failed in the 2nd time
> > > incoming.
> > >
> > > Thus it gives a warning and the users can run the qemu if they really
> > > want to do it.
> > >
> > > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> > > ---
> > >  backends/hostmem-file.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > > index 37ac6445d2..59429ee0b4 100644
> > > --- a/backends/hostmem-file.c
> > > +++ b/backends/hostmem-file.c
> > > @@ -16,6 +16,7 @@
> > >  #include "sysemu/hostmem.h"
> > >  #include "sysemu/sysemu.h"
> > >  #include "qom/object_interfaces.h"
> > > +#include "migration/migration.h"
> > >
> > >  /* hostmem-file.c */
> > >  /**
> > > @@ -79,6 +80,16 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
> > Error **errp)
> > >          }
> > >      }
> > >
> > > +    /*
> > > +     * In ignore shared case, if share=on for host memory backend file,
> > > +     * the ram might be written after incoming process is finished. Thus
> > > +     * the memory backend can't be reused for 2nd/3rd... incoming
> > > +     */
> > > +    if (backend->share && migrate_ignore_shared()
> > > +                       && runstate_check(RUN_STATE_INMIGRATE))
> > > +        warn_report("share=on for memory backend file might be "
> > > +                        "conflicted with incoming in ignore shared
> > case");
> >
> > I feel like this message wasn't really clear to me...  you want to
> > warn people these data might not match with each other, right?  How
> > about simply state it:
> >
> >   NOTE: Please make sure the data on the shared memory backend file
> >   and the data from the incoming migration stream contains matching
> >   contents, otherwise...
> >
> 
> Sorry for my vague expression.
> The background is [1]
> I happened to use "-object memory-backend-file,share=on" and together with
> "-incoming".
> It worked fine in 1st incoming migration but failed in 2nd, 3rd incoming
> migration.
> Because qemu uses qemu_ram_mmap(..., MAP_SHARED,...) when share=on, the
> memory-backend-file will be written after 1st incoming migration.
> Finally I realized that this was caused by "share=on" flag, and after I
> removed it,
> the memory-backend-file will not be changed any more.
> 
> So do you think it will be better that qemu gives the user a clear warning
> that
> incoming migration will change the data in  memory-backend-file  with
> "share=on" ?

Hmm... "a shared memory backend file" would already be clear to me
that it will be modified, but it might be subjective.

Regards,

-- 
Peter Xu

Currently it is not forbidden to use "-object memory-backend-file,share=on"
and together with "-incoming". But after incoming migration is finished,
the memory-backend-file will be definitely written if share=on. So the
memory-backend-file can only be used once, but failed in the 2nd time
incoming.

Thus it gives a warning and the users can run the qemu if they really
want to do it.

Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
---
 backends/hostmem-file.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 37ac6445d2..ce03dc0a18 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -16,6 +16,7 @@
 #include "sysemu/hostmem.h"
 #include "sysemu/sysemu.h"
 #include "qom/object_interfaces.h"
+#include "migration/migration.h"
 
 /* hostmem-file.c */
 /**
@@ -79,6 +80,17 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
         }
     }
 
+    /*
+     * In ignore shared incoming migration, if share=on for host memory
+     * backend file, the ram might be modified after incoming process.
+     * The user should know this potential risk.
+     */
+    if (backend->share && migrate_ignore_shared()
+                       && runstate_check(RUN_STATE_INMIGRATE))
+        warn_report("NOTE: Please make sure the data on the shared memory "
+                    "backend file and the data from the incoming migration"
+                    " stream contains matching contents, otherwise...");
+
     backend->force_prealloc = mem_prealloc;
     name = host_memory_backend_get_name(backend);
     memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
-- 
2.17.1