Re: [PATCH 3/3] migration: Remove _only suffix for res_postcopy/precopy

[Vladimir Sementsov-Ogievskiy]

On 14.02.23 21:22, Juan Quintela wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> > On 08.02.23 16:57, Juan Quintela wrote:
> > > Once that res_compatible is removed, they don't make sense anymore.
> > > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > > ---
> > >    include/migration/register.h   | 18 ++++++++----------
> > >    migration/savevm.h             |  8 ++++----
> > >    hw/s390x/s390-stattrib.c       |  7 +++----
> > >    hw/vfio/migration.c            | 10 ++++------
> > >    migration/block-dirty-bitmap.c |  6 +++---
> > >    migration/block.c              |  7 +++----
> > >    migration/ram.c                | 18 ++++++++----------
> > >    migration/savevm.c             | 24 ++++++++++--------------
> > >    8 files changed, 43 insertions(+), 55 deletions(-)
> > > diff --git a/include/migration/register.h
> > > b/include/migration/register.h
> > > index a958a92a0f..4a4a6d7174 100644
> > > --- a/include/migration/register.h
> > > +++ b/include/migration/register.h
> > > @@ -47,22 +47,20 @@ typedef struct SaveVMHandlers {
> > >        /* This runs outside the iothread lock!  */
> > >        int (*save_setup)(QEMUFile *f, void *opaque);
> > >        /* Note for save_live_pending:
> > > -     * - res_precopy_only is for data which must be migrated in precopy phase
> > > +     * - res_precopy is for data which must be migrated in precopy phase
> > >         *     or in stopped state, in other words - before target vm start
> > > -     * - res_postcopy_only is for data which must be migrated in postcopy phase
> > > +     * - res_postcopy is for data which must be migrated in postcopy phase
> > >         *     or in stopped state, in other words - after source vm stop
> >
> >
> > That's now wrong. "postcopy" is everything except "precopy", as it
> > includes "compat". Really, for RAM, it can be copied in precopy too,
> > and it is copied in precopy until user run command
> > migrate-start-postcopy. (In contrast: block-dirty-bitmap cannot
> > migrate in precopy at all, it migrate only in stopped state or in
> > postcopy).
> >
> > So, finally:
> >
> > "precopy"
> >
> >    definition:
> >    - must be migrated in precopy or in stopped state
> >    - in other words: must be migrated before target start
> >    - in other words: can't be migrated in postcopy
> >    - in other words: can't be migrated after target start
> >
> > "postcopy"
> >
> >    definition:
> >    - can migrate in postcopy
> >    - in other words: can migrate after target start
> >       some properties:
> >    - probably can be migrated in precopy (like RAM), or, may be not (like 
> > block-dirty-bitmap)
> >    - of course, can be migrated in stopped state
>
> What about (with latest naming)
>
> must_precopy:
> * must be migrated in precopy or in stopped state
> * i.e. must be migrated before target start
>
> can_postcopy:
> * can migrate in postcopy or in stopped state
> * i.e. can migrate after target start
> * some can also be migrated during precopy (RAM)
> * some must be migrated after source stops (block-dirty-bitmap)

Sounds very good, I'm for!

> > To be absolutely clear, we may rename them to "not_postcopyable" and 
> > "postcopyable".
>
> I hate negatives when naming variables.

Agree.

> What about: must_precopy and can_postcopy?

Sounds good!)

--
Best regards,
Vladimir