Re: [Qemu-devel] [PATCH v4 04/13] vfio: Add migration region initialization and finalize function

[Kirti Wankhede]

On 6/24/2019 7:30 PM, Cornelia Huck wrote:
> On Thu, 20 Jun 2019 20:07:32 +0530
> Kirti Wankhede <address@hidden> wrote:
> 
>> - Migration functions are implemented for VFIO_DEVICE_TYPE_PCI device in this
>>   patch series.
>> - VFIO device supports migration or not is decided based of migration region
>>   query. If migration region query is successful then migration is supported
>>   else migration is blocked.
>>
>> Signed-off-by: Kirti Wankhede <address@hidden>
>> Reviewed-by: Neo Jia <address@hidden>
>> ---
>>  hw/vfio/Makefile.objs         |   2 +-
>>  hw/vfio/migration.c           | 137 
>> ++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/vfio/vfio-common.h |  14 +++++
>>  3 files changed, 152 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/vfio/migration.c
> 
> (...)
> 
>> +static int vfio_migration_region_init(VFIODevice *vbasedev)
>> +{
>> +    VFIOMigration *migration = vbasedev->migration;
>> +    Object *obj = NULL;
>> +    int ret = -EINVAL;
>> +
>> +    if (!migration) {
>> +        return ret;
>> +    }
>> +
>> +    /* Migration support added for PCI device only */
>> +    if (vbasedev->type == VFIO_DEVICE_TYPE_PCI) {
>> +        obj = vfio_pci_get_object(vbasedev);
>> +    }
> 
> Hm... what about instead including an (optional) callback in
> VFIODeviceOps that returns the object embedding the VFIODevice? No need
> to adapt this code if we introduce support for a non-pci device, and the
> callback function also allows to support migration in a more
> finegrained way than by device type.
> 

That's good suggestion. I'm incorporating this change in next version.

Thanks,
Kirti


>> +
>> +    if (!obj) {
>> +        return ret;
>> +    }
>> +
>> +    ret = vfio_region_setup(obj, vbasedev, &migration->region.buffer,
>> +                            migration->region.index, "migration");
>> +    if (ret) {
>> +        error_report("Failed to setup VFIO migration region %d: %s",
>> +                      migration->region.index, strerror(-ret));
>> +        goto err;
>> +    }
>> +
>> +    if (!migration->region.buffer.size) {
>> +        ret = -EINVAL;
>> +        error_report("Invalid region size of VFIO migration region %d: %s",
>> +                     migration->region.index, strerror(-ret));
>> +        goto err;
>> +    }
>> +
>> +    return 0;
>> +
>> +err:
>> +    vfio_migration_region_exit(vbasedev);
>> +    return ret;
>> +}
> 
> (...)
>