| Message ID | 20230403130000.6422-1-avihaih@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/migration: Skip log_sync during migration SETUP state | expand |
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. Thanks, C. > --- > 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; > } >
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; > > } > > >
On 04/04/2023 0:36, Alex Williamson wrote: > External email: Use caution opening links or attachments > > > 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. Sure, so let's wait for 8.1 cycle to open. Thanks! >>> --- >>> 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; >>> } >>>
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; }
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> --- hw/vfio/common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)