[PATCH 2/4] vfio/container: fails mdev hotplug if add migration blocker failed

Zhenzhong Duan posted 4 patches 4 months, 3 weeks ago
Maintainers: Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Steve Sistare <steven.sistare@oracle.com>
There is a newer version of this series
[PATCH 2/4] vfio/container: fails mdev hotplug if add migration blocker failed
Posted by Zhenzhong Duan 4 months, 3 weeks ago
It's aggressive to abort a running QEMU process when hotplug a mdev
and it fails migration blocker adding.

Fix by just failing mdev hotplug itself.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/container.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 2853f6f08b..68b4fdb401 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -992,12 +992,16 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
     if (vbasedev->mdev) {
         error_setg(&vbasedev->cpr.mdev_blocker,
                    "CPR does not support vfio mdev %s", vbasedev->name);
-        migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker, &error_fatal,
-                                  MIG_MODE_CPR_TRANSFER, -1);
+        if (migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker, errp,
+                                      MIG_MODE_CPR_TRANSFER, -1)) {
+            goto hiod_unref_exit;
+        }
     }
 
     return true;
 
+hiod_unref_exit:
+    object_unref(vbasedev->hiod);
 device_put_exit:
     vfio_device_put(vbasedev);
 group_put_exit:
-- 
2.34.1
Re: [PATCH 2/4] vfio/container: fails mdev hotplug if add migration blocker failed
Posted by Cédric Le Goater 4 months, 3 weeks ago
On 6/23/25 12:22, Zhenzhong Duan wrote:
> It's aggressive to abort a running QEMU process when hotplug a mdev
> and it fails migration blocker adding.
> 
> Fix by just failing mdev hotplug itself.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/vfio/container.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 2853f6f08b..68b4fdb401 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -992,12 +992,16 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
>       if (vbasedev->mdev) {
>           error_setg(&vbasedev->cpr.mdev_blocker,
>                      "CPR does not support vfio mdev %s", vbasedev->name);
> -        migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker, &error_fatal,
> -                                  MIG_MODE_CPR_TRANSFER, -1);
> +        if (migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker, errp,
> +                                      MIG_MODE_CPR_TRANSFER, -1)) {

migrate_add_blocker_modes() returns -errno. Testing with '< 0' would be
better.


Thanks,

C.



> +            goto hiod_unref_exit;
> +        }
>       }
>   
>       return true;
>   
> +hiod_unref_exit:
> +    object_unref(vbasedev->hiod);
>   device_put_exit:
>       vfio_device_put(vbasedev);
>   group_put_exit:
Re: [PATCH 2/4] vfio/container: fails mdev hotplug if add migration blocker failed
Posted by Cédric Le Goater 4 months, 3 weeks ago
On 6/24/25 11:21, Cédric Le Goater wrote:
> On 6/23/25 12:22, Zhenzhong Duan wrote:
>> It's aggressive to abort a running QEMU process when hotplug a mdev
>> and it fails migration blocker adding.
>>
>> Fix by just failing mdev hotplug itself.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/vfio/container.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 2853f6f08b..68b4fdb401 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -992,12 +992,16 @@ static bool vfio_legacy_attach_device(const char *name, VFIODevice *vbasedev,
>>       if (vbasedev->mdev) {
>>           error_setg(&vbasedev->cpr.mdev_blocker,
>>                      "CPR does not support vfio mdev %s", vbasedev->name);
>> -        migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker, &error_fatal,
>> -                                  MIG_MODE_CPR_TRANSFER, -1);
>> +        if (migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker, errp,
>> +                                      MIG_MODE_CPR_TRANSFER, -1)) {
> 
> migrate_add_blocker_modes() returns -errno. Testing with '< 0' would be
> better.
> 


Reviewed-by: Cédric Le Goater <clg@redhat.com>

I changed the test on the value returned by migrate_add_blocker_modes().


Thanks,

C.



> Thanks,
> 
> C.
> 
> 
> 
>> +            goto hiod_unref_exit;
>> +        }
>>       }
>>       return true;
>> +hiod_unref_exit:
>> +    object_unref(vbasedev->hiod);
>>   device_put_exit:
>>       vfio_device_put(vbasedev);
>>   group_put_exit:
> 


RE: [PATCH 2/4] vfio/container: fails mdev hotplug if add migration blocker failed
Posted by Duan, Zhenzhong 4 months, 3 weeks ago

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH 2/4] vfio/container: fails mdev hotplug if add migration
>blocker failed
>
>On 6/24/25 11:21, Cédric Le Goater wrote:
>> On 6/23/25 12:22, Zhenzhong Duan wrote:
>>> It's aggressive to abort a running QEMU process when hotplug a mdev
>>> and it fails migration blocker adding.
>>>
>>> Fix by just failing mdev hotplug itself.
>>>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>   hw/vfio/container.c | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>>> index 2853f6f08b..68b4fdb401 100644
>>> --- a/hw/vfio/container.c
>>> +++ b/hw/vfio/container.c
>>> @@ -992,12 +992,16 @@ static bool vfio_legacy_attach_device(const char
>*name, VFIODevice *vbasedev,
>>>       if (vbasedev->mdev) {
>>>           error_setg(&vbasedev->cpr.mdev_blocker,
>>>                      "CPR does not support vfio mdev %s", vbasedev->name);
>>> -        migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker,
>&error_fatal,
>>> -                                  MIG_MODE_CPR_TRANSFER, -1);
>>> +        if (migrate_add_blocker_modes(&vbasedev->cpr.mdev_blocker, errp,
>>> +                                      MIG_MODE_CPR_TRANSFER, -1)) {
>>
>> migrate_add_blocker_modes() returns -errno. Testing with '< 0' would be
>> better.
>>
>
>
>Reviewed-by: Cédric Le Goater <clg@redhat.com>
>

Thanks Cédric.
I ever planned to send an update after receiving comments for patch3,4.

BRs,
Zhenzhong