Re: [PATCH 5/5] multifd: Only sync once each full round of memory

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Jul 05, 2022 at 06:13:40PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > On Tue, Jul 05, 2022 at 05:11:46PM +0200, Juan Quintela wrote:
> > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > > > * Juan Quintela (quintela@redhat.com) wrote:
> > > >> We need to add a new flag to mean to sync at that point.
> > > >> Notice that we still synchronize at the end of setup and at the end of
> > > >> complete stages.
> > > >> 
> > > >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> > > >> ---
> > > >>  migration/migration.c |  2 +-
> > > >>  migration/ram.c       | 42 ++++++++++++++++++++++++++++++------------
> > > >>  2 files changed, 31 insertions(+), 13 deletions(-)
> > > >> 
> > > >> diff --git a/migration/migration.c b/migration/migration.c
> > > >> index 3f79df0b70..6627787fc2 100644
> > > >> --- a/migration/migration.c
> > > >> +++ b/migration/migration.c
> > > >> @@ -4283,7 +4283,7 @@ static Property migration_properties[] = {
> > > >>                        DEFAULT_MIGRATE_ANNOUNCE_STEP),
> > > >>      /* We will change to false when we introduce the new mechanism */
> > > >>      DEFINE_PROP_BOOL("multifd-sync-each-iteration", MigrationState,
> > > >> -                      multifd_sync_each_iteration, true),
> > > >> +                      multifd_sync_each_iteration, false),
> > > >>  
> > > >>      /* Migration capabilities */
> > > >>      DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
> > > >> diff --git a/migration/ram.c b/migration/ram.c
> > > >> index 2c7289edad..6792986565 100644
> > > >> --- a/migration/ram.c
> > > >> +++ b/migration/ram.c
> > > >> @@ -81,6 +81,7 @@
> > > >>  #define RAM_SAVE_FLAG_XBZRLE   0x40
> > > >>  /* 0x80 is reserved in migration.h start with 0x100 next */
> > > >>  #define RAM_SAVE_FLAG_COMPRESS_PAGE    0x100
> > > >> +#define RAM_SAVE_FLAG_MULTIFD_SYNC     0x200
> > > >
> > > > Note this is the very last usable flag!
> > > 
> > > We can recover two flags right now:
> > > 
> > > RAM_SAVE_FLAG_FULL is not used anymore.
> > > 0x80 is free since years ago.
> > > 
> > > Once multifd is default, there are some other that could go.
> 
> I have suggested that a few times in the past.
> 
> > Non-multifd migration isn't likely to go away any time soon, given
> > distros desire to support migration between QEMU's with quite
> > significantly different versions. So feels like quite a long time
> > before we might reclaim more flags.
> > 
> > > > We could do with avoiding using them as flags where we dont need to.
> > > 
> > > I can't really think on another way to do it.  The other thing that I
> > > can do is just reuse one of the flags that don't make sense for multifd
> > > (RAM_SAVE_FLAG_ZERO after zero pages patch,
> > > RAM_SAVE_FLAG_XBZRLE/COMPRESS_PAGE).
> > 
> > Re-using flags based on use context differences feels like a recipe
> > to confuse people.
> 
> Note that most of these things aren't really 'flags'; in the sense that
> only a few of them are actually combinable; so we should start using
> combinations to mean things new.

IOW, treat the field as an enum of valid values instead, and just
define enum entries for the few valid combinations, giving us many
more values to play with ?


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|