Re: [PATCH] vfio/migration: Skip log_sync during migration SETUP state

[Alex Williamson]

On Mon, 3 Apr 2023 22:36:42 +0200
Cédric Le Goater <clg@redhat.com> wrote:

> On 4/3/23 15:00, Avihai Horon wrote:
> > Currently, VFIO log_sync can be issued while migration is in SETUP
> > state. However, doing this log_sync is at best redundant and at worst
> > can fail.
> > 
> > Redundant -- all RAM is marked dirty in migration SETUP state and is
> > transferred only after migration is set to ACTIVE state, so doing
> > log_sync during migration SETUP is pointless.
> > 
> > Can fail -- there is a time window, between setting migration state to
> > SETUP and starting dirty tracking by RAM save_live_setup handler, during
> > which dirty tracking is still not started. Any VFIO log_sync call that
> > is issued during this time window will fail. For example, this error can
> > be triggered by migrating a VM when a GUI is active, which constantly
> > calls log_sync.
> > 
> > Fix it by skipping VFIO log_sync while migration is in SETUP state.
> > 
> > Fixes: 758b96b61d5c ("vfio/migrate: Move switch of dirty tracking into 
> > vfio_memory_listener")
> > Signed-off-by: Avihai Horon <avihaih@nvidia.com>  
> migration is still experimental, so this can wait 8.1. Correct me if not.

Agreed, this doesn't seem nearly catastrophic enough as an experimental
feature that it can't wait for the 8.1 devel cycle to open.  Thanks,

Alex

> > ---
> >   hw/vfio/common.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index 4d01ea3515..78358ede27 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -478,7 +478,8 @@ static bool 
> > vfio_devices_all_dirty_tracking(VFIOContainer *container)
> >       VFIODevice *vbasedev;
> >       MigrationState *ms = migrate_get_current();
> >   
> > -    if (!migration_is_setup_or_active(ms->state)) {
> > +    if (ms->state != MIGRATION_STATUS_ACTIVE &&
> > +        ms->state != MIGRATION_STATUS_DEVICE) {
> >           return false;
> >       }
> >     
>