| Message ID | 20221103161620.13120-11-avihaih@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/migration: Implement VFIO migration protocol v2 | expand |
On Thu, 3 Nov 2022 18:16:13 +0200 Avihai Horon <avihaih@nvidia.com> wrote: > Move vfio_dev_get_region_info() logic from vfio_migration_probe() to > vfio_migration_init(). This logic is specific to v1 protocol and moving > it will make it easier to add the v2 protocol implementation later. > No functional changes intended. > > Signed-off-by: Avihai Horon <avihaih@nvidia.com> > --- > hw/vfio/migration.c | 30 +++++++++++++++--------------- > hw/vfio/trace-events | 2 +- > 2 files changed, 16 insertions(+), 16 deletions(-) > > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > index 99ffb75782..0e3a950746 100644 > --- a/hw/vfio/migration.c > +++ b/hw/vfio/migration.c > @@ -785,14 +785,14 @@ static void vfio_migration_exit(VFIODevice *vbasedev) > vbasedev->migration = NULL; > } > > -static int vfio_migration_init(VFIODevice *vbasedev, > - struct vfio_region_info *info) > +static int vfio_migration_init(VFIODevice *vbasedev) > { > int ret; > Object *obj; > VFIOMigration *migration; > char id[256] = ""; > g_autofree char *path = NULL, *oid = NULL; > + struct vfio_region_info *info = NULL; Nit, I'm not spotting any cases where we need this initialization. The same is not true in the code the info handling was extracted from. Thanks, Alex > > if (!vbasedev->ops->vfio_get_object) { > return -EINVAL; > @@ -803,6 +803,14 @@ static int vfio_migration_init(VFIODevice *vbasedev, > return -EINVAL; > } > > + ret = vfio_get_dev_region_info(vbasedev, > + VFIO_REGION_TYPE_MIGRATION_DEPRECATED, > + VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED, > + &info); > + if (ret) { > + return ret; > + } > + > vbasedev->migration = g_new0(VFIOMigration, 1); > vbasedev->migration->device_state = VFIO_DEVICE_STATE_V1_RUNNING; > vbasedev->migration->vm_running = runstate_is_running(); > @@ -822,6 +830,8 @@ static int vfio_migration_init(VFIODevice *vbasedev, > goto err; > } > > + g_free(info); > + > migration = vbasedev->migration; > migration->vbasedev = vbasedev; > > @@ -844,6 +854,7 @@ static int vfio_migration_init(VFIODevice *vbasedev, > return 0; > > err: > + g_free(info); > vfio_migration_exit(vbasedev); > return ret; > } > @@ -857,34 +868,23 @@ int64_t vfio_mig_bytes_transferred(void) > > int vfio_migration_probe(VFIODevice *vbasedev, Error **errp) > { > - struct vfio_region_info *info = NULL; > int ret = -ENOTSUP; > > if (!vbasedev->enable_migration) { > goto add_blocker; > } > > - ret = vfio_get_dev_region_info(vbasedev, > - VFIO_REGION_TYPE_MIGRATION_DEPRECATED, > - VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED, > - &info); > + ret = vfio_migration_init(vbasedev); > if (ret) { > goto add_blocker; > } > > - ret = vfio_migration_init(vbasedev, info); > - if (ret) { > - goto add_blocker; > - } > - > - trace_vfio_migration_probe(vbasedev->name, info->index); > - g_free(info); > + trace_vfio_migration_probe(vbasedev->name); > return 0; > > add_blocker: > error_setg(&vbasedev->migration_blocker, > "VFIO device doesn't support migration"); > - g_free(info); > > ret = migrate_add_blocker(vbasedev->migration_blocker, errp); > if (ret < 0) { > diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events > index a21cbd2a56..27c059f96e 100644 > --- a/hw/vfio/trace-events > +++ b/hw/vfio/trace-events > @@ -148,7 +148,7 @@ vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u" > vfio_display_edid_write_error(void) "" > > # migration.c > -vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d" > +vfio_migration_probe(const char *name) " (%s)" > vfio_migration_set_state(const char *name, uint32_t state) " (%s) state %d" > vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t dev_state) " (%s) running %d reason %s device state %d" > vfio_migration_state_notifier(const char *name, const char *state) " (%s) state %s"
On 16/11/2022 1:56, Alex Williamson wrote: > External email: Use caution opening links or attachments > > > On Thu, 3 Nov 2022 18:16:13 +0200 > Avihai Horon <avihaih@nvidia.com> wrote: > >> Move vfio_dev_get_region_info() logic from vfio_migration_probe() to >> vfio_migration_init(). This logic is specific to v1 protocol and moving >> it will make it easier to add the v2 protocol implementation later. >> No functional changes intended. >> >> Signed-off-by: Avihai Horon <avihaih@nvidia.com> >> --- >> hw/vfio/migration.c | 30 +++++++++++++++--------------- >> hw/vfio/trace-events | 2 +- >> 2 files changed, 16 insertions(+), 16 deletions(-) >> >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c >> index 99ffb75782..0e3a950746 100644 >> --- a/hw/vfio/migration.c >> +++ b/hw/vfio/migration.c >> @@ -785,14 +785,14 @@ static void vfio_migration_exit(VFIODevice *vbasedev) >> vbasedev->migration = NULL; >> } >> >> -static int vfio_migration_init(VFIODevice *vbasedev, >> - struct vfio_region_info *info) >> +static int vfio_migration_init(VFIODevice *vbasedev) >> { >> int ret; >> Object *obj; >> VFIOMigration *migration; >> char id[256] = ""; >> g_autofree char *path = NULL, *oid = NULL; >> + struct vfio_region_info *info = NULL; > Nit, I'm not spotting any cases where we need this initialization. The > same is not true in the code the info handling was extracted from. > Thanks, You are right. I will drop the initialization in v4. Thanks! > Alex > >> if (!vbasedev->ops->vfio_get_object) { >> return -EINVAL; >> @@ -803,6 +803,14 @@ static int vfio_migration_init(VFIODevice *vbasedev, >> return -EINVAL; >> } >> >> + ret = vfio_get_dev_region_info(vbasedev, >> + VFIO_REGION_TYPE_MIGRATION_DEPRECATED, >> + VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED, >> + &info); >> + if (ret) { >> + return ret; >> + } >> + >> vbasedev->migration = g_new0(VFIOMigration, 1); >> vbasedev->migration->device_state = VFIO_DEVICE_STATE_V1_RUNNING; >> vbasedev->migration->vm_running = runstate_is_running(); >> @@ -822,6 +830,8 @@ static int vfio_migration_init(VFIODevice *vbasedev, >> goto err; >> } >> >> + g_free(info); >> + >> migration = vbasedev->migration; >> migration->vbasedev = vbasedev; >> >> @@ -844,6 +854,7 @@ static int vfio_migration_init(VFIODevice *vbasedev, >> return 0; >> >> err: >> + g_free(info); >> vfio_migration_exit(vbasedev); >> return ret; >> } >> @@ -857,34 +868,23 @@ int64_t vfio_mig_bytes_transferred(void) >> >> int vfio_migration_probe(VFIODevice *vbasedev, Error **errp) >> { >> - struct vfio_region_info *info = NULL; >> int ret = -ENOTSUP; >> >> if (!vbasedev->enable_migration) { >> goto add_blocker; >> } >> >> - ret = vfio_get_dev_region_info(vbasedev, >> - VFIO_REGION_TYPE_MIGRATION_DEPRECATED, >> - VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED, >> - &info); >> + ret = vfio_migration_init(vbasedev); >> if (ret) { >> goto add_blocker; >> } >> >> - ret = vfio_migration_init(vbasedev, info); >> - if (ret) { >> - goto add_blocker; >> - } >> - >> - trace_vfio_migration_probe(vbasedev->name, info->index); >> - g_free(info); >> + trace_vfio_migration_probe(vbasedev->name); >> return 0; >> >> add_blocker: >> error_setg(&vbasedev->migration_blocker, >> "VFIO device doesn't support migration"); >> - g_free(info); >> >> ret = migrate_add_blocker(vbasedev->migration_blocker, errp); >> if (ret < 0) { >> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events >> index a21cbd2a56..27c059f96e 100644 >> --- a/hw/vfio/trace-events >> +++ b/hw/vfio/trace-events >> @@ -148,7 +148,7 @@ vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u" >> vfio_display_edid_write_error(void) "" >> >> # migration.c >> -vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d" >> +vfio_migration_probe(const char *name) " (%s)" >> vfio_migration_set_state(const char *name, uint32_t state) " (%s) state %d" >> vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t dev_state) " (%s) running %d reason %s device state %d" >> vfio_migration_state_notifier(const char *name, const char *state) " (%s) state %s"
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index 99ffb75782..0e3a950746 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -785,14 +785,14 @@ static void vfio_migration_exit(VFIODevice *vbasedev) vbasedev->migration = NULL; } -static int vfio_migration_init(VFIODevice *vbasedev, - struct vfio_region_info *info) +static int vfio_migration_init(VFIODevice *vbasedev) { int ret; Object *obj; VFIOMigration *migration; char id[256] = ""; g_autofree char *path = NULL, *oid = NULL; + struct vfio_region_info *info = NULL; if (!vbasedev->ops->vfio_get_object) { return -EINVAL; @@ -803,6 +803,14 @@ static int vfio_migration_init(VFIODevice *vbasedev, return -EINVAL; } + ret = vfio_get_dev_region_info(vbasedev, + VFIO_REGION_TYPE_MIGRATION_DEPRECATED, + VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED, + &info); + if (ret) { + return ret; + } + vbasedev->migration = g_new0(VFIOMigration, 1); vbasedev->migration->device_state = VFIO_DEVICE_STATE_V1_RUNNING; vbasedev->migration->vm_running = runstate_is_running(); @@ -822,6 +830,8 @@ static int vfio_migration_init(VFIODevice *vbasedev, goto err; } + g_free(info); + migration = vbasedev->migration; migration->vbasedev = vbasedev; @@ -844,6 +854,7 @@ static int vfio_migration_init(VFIODevice *vbasedev, return 0; err: + g_free(info); vfio_migration_exit(vbasedev); return ret; } @@ -857,34 +868,23 @@ int64_t vfio_mig_bytes_transferred(void) int vfio_migration_probe(VFIODevice *vbasedev, Error **errp) { - struct vfio_region_info *info = NULL; int ret = -ENOTSUP; if (!vbasedev->enable_migration) { goto add_blocker; } - ret = vfio_get_dev_region_info(vbasedev, - VFIO_REGION_TYPE_MIGRATION_DEPRECATED, - VFIO_REGION_SUBTYPE_MIGRATION_DEPRECATED, - &info); + ret = vfio_migration_init(vbasedev); if (ret) { goto add_blocker; } - ret = vfio_migration_init(vbasedev, info); - if (ret) { - goto add_blocker; - } - - trace_vfio_migration_probe(vbasedev->name, info->index); - g_free(info); + trace_vfio_migration_probe(vbasedev->name); return 0; add_blocker: error_setg(&vbasedev->migration_blocker, "VFIO device doesn't support migration"); - g_free(info); ret = migrate_add_blocker(vbasedev->migration_blocker, errp); if (ret < 0) { diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index a21cbd2a56..27c059f96e 100644 --- a/hw/vfio/trace-events +++ b/hw/vfio/trace-events @@ -148,7 +148,7 @@ vfio_display_edid_update(uint32_t prefx, uint32_t prefy) "%ux%u" vfio_display_edid_write_error(void) "" # migration.c -vfio_migration_probe(const char *name, uint32_t index) " (%s) Region %d" +vfio_migration_probe(const char *name) " (%s)" vfio_migration_set_state(const char *name, uint32_t state) " (%s) state %d" vfio_vmstate_change(const char *name, int running, const char *reason, uint32_t dev_state) " (%s) running %d reason %s device state %d" vfio_migration_state_notifier(const char *name, const char *state) " (%s) state %s"
Move vfio_dev_get_region_info() logic from vfio_migration_probe() to vfio_migration_init(). This logic is specific to v1 protocol and moving it will make it easier to add the v2 protocol implementation later. No functional changes intended. Signed-off-by: Avihai Horon <avihaih@nvidia.com> --- hw/vfio/migration.c | 30 +++++++++++++++--------------- hw/vfio/trace-events | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-)