Re: [PATCH v28 04/17] vfio: Add migration region initialization and finalize function

[Kirti Wankhede]

On 10/23/2020 4:54 PM, Cornelia Huck wrote:
> On Fri, 23 Oct 2020 16:10:30 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>
> > Whether the VFIO device supports migration or not is decided based of
> > migration region query. If migration region query is successful and migration
> > region initialization is successful then migration is supported else
> > migration is blocked.
> >
> > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> > Reviewed-by: Neo Jia <cjia@nvidia.com>
> > Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >   hw/vfio/meson.build           |   1 +
> >   hw/vfio/migration.c           | 133 ++++++++++++++++++++++++++++++++++++++++++
> >   hw/vfio/trace-events          |   3 +
> >   include/hw/vfio/vfio-common.h |   9 +++
> >   4 files changed, 146 insertions(+)
> >   create mode 100644 hw/vfio/migration.c
>
> (...)
>
> > +static int vfio_migration_init(VFIODevice *vbasedev,
> > +                               struct vfio_region_info *info)
> > +{
> > +    int ret;
> > +    Object *obj;
> > +    VFIOMigration *migration;
> > +
> > +    if (!vbasedev->ops->vfio_get_object) {
> > +        return -EINVAL;
> > +    }
> > +
> > +    obj = vbasedev->ops->vfio_get_object(vbasedev);
> > +    if (!obj) {
> > +        return -EINVAL;
> > +    }
> > +
> > +    migration = g_new0(VFIOMigration, 1);
> > +
> > +    ret = vfio_region_setup(obj, vbasedev, &migration->region,
> > +                            info->index, "migration");
> > +    if (ret) {
> > +        error_report("%s: Failed to setup VFIO migration region %d: %s",
> > +                     vbasedev->name, info->index, strerror(-ret));
> > +        goto err;
> > +    }
> > +
> > +    vbasedev->migration = migration;
> > +
> > +    if (!migration->region.size) {
> > +        error_report("%s: Invalid zero-sized of VFIO migration region %d",
>
> s/of //
>
> > +                     vbasedev->name, info->index);
> > +        ret = -EINVAL;
> > +        goto err;
> > +    }
> > +    return 0;
> > +
> > +err:
> > +    vfio_migration_region_exit(vbasedev);
> > +    g_free(migration);
> > +    vbasedev->migration = NULL;
> > +    return ret;
> > +}
>
> (...)
>
> > +void vfio_migration_finalize(VFIODevice *vbasedev)
> > +{
> > +    VFIOMigration *migration = vbasedev->migration;
>
> I don't think you need this variable?

Removing it.

> > +
> > +    if (migration) {
> > +        vfio_migration_region_exit(vbasedev);
> > +        g_free(vbasedev->migration);
> > +        vbasedev->migration = NULL;
> > +    }
> > +
> > +    if (vbasedev->migration_blocker) {
> > +        migrate_del_blocker(vbasedev->migration_blocker);
> > +        error_free(vbasedev->migration_blocker);
> > +        vbasedev->migration_blocker = NULL;
> > +    }
> > +}
>
> (...)