On Thu, 3 Nov 2022 18:16:13 +0200
Avihai Horon <avih...@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 <avih...@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"
