| Message ID | 20240509090954.16447-4-avihaih@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi/vfio: Add VFIO migration QAPI event | expand |
On 5/9/24 11:09, Avihai Horon wrote: > When migrating a VFIO device that supports pre-copy, it is transitioned > to STOP_COPY twice: once in vfio_vmstate_change() and second time in > vfio_save_complete_precopy(). > > The second transition is harmless, as it's a STOP_COPY->STOP_COPY no-op > transition. However, with the newly added VFIO migration QAPI event, the > STOP_COPY event is undesirably emitted twice. > > Prevent this by returning early in vfio_migration_set_state() if > new_state is the same as current device state. > > Note that the STOP_COPY transition in vfio_save_complete_precopy() is > essential for VFIO devices that don't support pre-copy, for migrating an > already stopped guest and for snapshots. > > Signed-off-by: Avihai Horon <avihaih@nvidia.com> > --- > hw/vfio/migration.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > index 5a359c4c78..14ef9c924e 100644 > --- a/hw/vfio/migration.c > +++ b/hw/vfio/migration.c > @@ -143,6 +143,10 @@ static int vfio_migration_set_state(VFIODevice *vbasedev, > (struct vfio_device_feature_mig_state *)feature->data; > int ret; > I wonder if we should improve the trace events a little to track better the state transitions. May be move trace_vfio_migration_set_state() at the beginning of vfio_migration_set_state() and introduce a new event for the currently named routine set_state() ? This can come with followups. Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > + if (new_state == migration->device_state) { > + return 0; > + } > + > feature->argsz = sizeof(buf); > feature->flags = > VFIO_DEVICE_FEATURE_SET | VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE;
On 13/05/2024 17:13, Cédric Le Goater wrote: > External email: Use caution opening links or attachments > > > On 5/9/24 11:09, Avihai Horon wrote: >> When migrating a VFIO device that supports pre-copy, it is transitioned >> to STOP_COPY twice: once in vfio_vmstate_change() and second time in >> vfio_save_complete_precopy(). >> >> The second transition is harmless, as it's a STOP_COPY->STOP_COPY no-op >> transition. However, with the newly added VFIO migration QAPI event, the >> STOP_COPY event is undesirably emitted twice. >> >> Prevent this by returning early in vfio_migration_set_state() if >> new_state is the same as current device state. >> >> Note that the STOP_COPY transition in vfio_save_complete_precopy() is >> essential for VFIO devices that don't support pre-copy, for migrating an >> already stopped guest and for snapshots. >> >> Signed-off-by: Avihai Horon <avihaih@nvidia.com> >> --- >> hw/vfio/migration.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c >> index 5a359c4c78..14ef9c924e 100644 >> --- a/hw/vfio/migration.c >> +++ b/hw/vfio/migration.c >> @@ -143,6 +143,10 @@ static int vfio_migration_set_state(VFIODevice >> *vbasedev, >> (struct vfio_device_feature_mig_state *)feature->data; >> int ret; >> > > I wonder if we should improve the trace events a little to track better > the state transitions. May be move trace_vfio_migration_set_state() > at the beginning of vfio_migration_set_state() and introduce a new > event for the currently named routine set_state() ? > > This can come with followups. > Yes, this sounds good. Thanks. > > Reviewed-by: Cédric Le Goater <clg@redhat.com> > > Thanks, > > C. > > >> + if (new_state == migration->device_state) { >> + return 0; >> + } >> + >> feature->argsz = sizeof(buf); >> feature->flags = >> VFIO_DEVICE_FEATURE_SET | >> VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE; >
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index 5a359c4c78..14ef9c924e 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -143,6 +143,10 @@ static int vfio_migration_set_state(VFIODevice *vbasedev, (struct vfio_device_feature_mig_state *)feature->data; int ret; + if (new_state == migration->device_state) { + return 0; + } + feature->argsz = sizeof(buf); feature->flags = VFIO_DEVICE_FEATURE_SET | VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE;
When migrating a VFIO device that supports pre-copy, it is transitioned to STOP_COPY twice: once in vfio_vmstate_change() and second time in vfio_save_complete_precopy(). The second transition is harmless, as it's a STOP_COPY->STOP_COPY no-op transition. However, with the newly added VFIO migration QAPI event, the STOP_COPY event is undesirably emitted twice. Prevent this by returning early in vfio_migration_set_state() if new_state is the same as current device state. Note that the STOP_COPY transition in vfio_save_complete_precopy() is essential for VFIO devices that don't support pre-copy, for migrating an already stopped guest and for snapshots. Signed-off-by: Avihai Horon <avihaih@nvidia.com> --- hw/vfio/migration.c | 4 ++++ 1 file changed, 4 insertions(+)