Re: [PATCH 10/14] vfio/migration: Add Error ** parameter to vfio_migration_init()

[Avihai Horon <avihaih@nvidia.com>]

On 5/7/2026 10:59 AM, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> Hello Avihai,
>
> On 5/5/26 10:14, Avihai Horon wrote:
>> vfio_migration_init() already has many failure points and a new one will
>> be added in next patch.
>>
>> Add Error ** parameter to vfio_migration_init() to report a detailed
>> error message through it.
>>
>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>> ---
>>   hw/vfio/migration.c | 29 +++++++++++++++++------------
>>   1 file changed, 17 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 243624b5fe..b7e929274a 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -1038,7 +1038,7 @@ static bool 
>> vfio_dma_logging_supported(VFIODevice *vbasedev)
>>       return !ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature);
>>   }
>>
>> -static int vfio_migration_init(VFIODevice *vbasedev)
>> +static int vfio_migration_init(VFIODevice *vbasedev, Error **errp)
>>   {
>>       int ret;
>>       Object *obj;
>> @@ -1047,23 +1047,38 @@ static int vfio_migration_init(VFIODevice 
>> *vbasedev)
>>       g_autofree char *path = NULL, *oid = NULL;
>>       uint64_t mig_flags = 0;
>>       VMChangeStateHandler *prepare_cb;
>> +    g_autofree char *error_prefix =
>> +        g_strdup_printf("%s: VFIO migration init failed:", 
>> vbasedev->name);
>
> We have error_prepend() for this purpose.

Right.
I was trying to avoid duplicating the prefix on each fail branch.

Do you suggest to add a common "err:" goto label at the bottom and put 
there a single:

   error_prepend(errp, "%s: VFIO migration init failed:", vbasedev->name);

?

Thanks.

>
>
>>
>>       if (!vbasedev->ops->vfio_get_object) {
>> +        error_setg(errp, "%s no vfio_get_object handler", 
>> error_prefix);
>>           return -EINVAL;
>>       }
>>
>>       obj = vbasedev->ops->vfio_get_object(vbasedev);
>>       if (!obj) {
>> +        error_setg(errp, "%s failed to get object", error_prefix);
>>           return -EINVAL;
>>       }
>>
>>       ret = vfio_migration_query_flags(vbasedev, &mig_flags);
>>       if (ret) {
>> +        if (ret == -ENOTTY) {
>> +            error_setg_errno(errp, -ret,
>> +                             "%s migration is not supported in kernel",
>> +                             error_prefix);
>> +        } else {
>> +            error_setg_errno(errp, -ret, "%s failed to query 
>> migration flags",
>> +                             error_prefix);
>> +        }
>> +
>>           return ret;
>>       }
>>
>>       /* Basic migration functionality must be supported */
>>       if (!(mig_flags & VFIO_MIGRATION_STOP_COPY)) {
>> +        error_setg(errp, "%s VFIO_MIGRATION_STOP_COPY is not 
>> supported",
>> +                   error_prefix);
>>           return -EOPNOTSUPP;
>>       }
>>
>> @@ -1261,18 +1276,8 @@ bool vfio_migration_realize(VFIODevice 
>> *vbasedev, Error **errp)
>>           return !vfio_block_migration(vbasedev, err, errp);
>>       }
>>
>> -    ret = vfio_migration_init(vbasedev);
>> +    ret = vfio_migration_init(vbasedev, &err);
>>       if (ret) {
>> -        if (ret == -ENOTTY) {
>> -            error_setg(&err, "%s: VFIO migration is not supported in 
>> kernel",
>> -                       vbasedev->name);
>> -        } else {
>> -            error_setg(&err,
>> -                       "%s: Migration couldn't be initialized for 
>> VFIO device, "
>> -                       "err: %d (%s)",
>> -                       vbasedev->name, ret, strerror(-ret));
>> -        }
>> -
>>           return !vfio_block_migration(vbasedev, err, errp);
>>       }
>>
>