Re: [PATCH v3 18/21] migration: Make no compression operations into its own structure

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > * Juan Quintela (quintela@redhat.com) wrote:
> >> It will be used later.
> >> 
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> 
> 
> >> +int migrate_multifd_method(void)
> >> +{
> >> +    MigrationState *s;
> >> +
> >> +    s = migrate_get_current();
> >> +
> >> +    return s->parameters.multifd_compress;
> >> +}
> >
> > Shouldn't that be a MultifdCompress enum returned?
> 
> You are right here.
> >>  
> >>  #define MULTIFD_FLAG_SYNC (1 << 0)
> >> +#define MULTIFD_FLAG_NOCOMP (1 << 1)
> >
> > I don't think this should be a set of individual flags; in later patches
> > you define a flag for zlib and another for zstd etc etc - but you can't
> > combine them - you could never have FLAG_NOCOMP|FLAG_ZSTD|FLAG_ZLIB - so
> > this should be a 3 or 4 bit field which contains a compression id (0
> > being none).  The ID can't exactly be the migrate_multifd_method() enum
> > value - because I don't think that's defined to be stable (?).
> 
> The idea is to catch up if we got an incorrect packet with an incorrect
> flag.
> 
> But yes, I agree that it could be the same expecting a value here.
> The problem is that I already have the flags field.
> 
> Would it be ok for you if I reserve 3 bits for this?  (right now 2
> should be enough).

Yeh that's fine - I was going to suggest 4 bits, but 3 is OK;
just define something like:

  MULTIFD_FLAG_COMP_MASK (7 << 1)

Dave

> Thanks, Juan.
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK