Re: [Qemu-devel] [PATCH] hostmem-file: warn when memory-backend-file, share=on and in incoming migration

[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 <address@hidden> 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 <address@hidden>
> > > ---
> > >  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