vfio/migration: Skip log_sync during migration SETUP state

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

Posted by Avihai Horon 1 year, 1 month ago

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(-)

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;
     }
 
-- 
2.21.3

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

Posted by Cédric Le Goater 1 year, 1 month ago

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;
>       }
>

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

Posted by Alex Williamson 1 year, 1 month ago

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;
> >       }
> >     
>

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

Posted by Avihai Horon 1 year, 1 month ago

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;
>>>        }
>>>