From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>

DEFINE_PROP_ON_OFF_AUTO() property isn't runtime-mutable so using it
would mean that the source VM would need to decide upfront at startup
time whether it wants to do a multifd device state transfer at some
point.

Source VM can run for a long time before being migrated so it is
desirable to have a fallback mechanism to the old way of transferring
VFIO device state if it turns to be necessary.

This brings this property to the same mutability level as ordinary
migration parameters, which too can be adjusted at the run time.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
 hw/vfio/pci.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 184ff882f9d1..9111805ae06c 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3353,6 +3353,8 @@ static void vfio_instance_init(Object *obj)
     pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
 }
 
+static PropertyInfo qdev_prop_on_off_auto_mutable;
+
 static const Property vfio_pci_dev_properties[] = {
     DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
     DEFINE_PROP_UUID_NODEFAULT("vf-token", VFIOPCIDevice, vf_token),
@@ -3377,9 +3379,10 @@ static const Property vfio_pci_dev_properties[] = {
                     VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
     DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
                             vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
-    DEFINE_PROP_ON_OFF_AUTO("x-migration-multifd-transfer", VFIOPCIDevice,
-                            vbasedev.migration_multifd_transfer,
-                            ON_OFF_AUTO_AUTO),
+    DEFINE_PROP("x-migration-multifd-transfer", VFIOPCIDevice,
+                vbasedev.migration_multifd_transfer,
+                qdev_prop_on_off_auto_mutable, OnOffAuto,
+                .set_default = true, .defval.i = ON_OFF_AUTO_AUTO),
     DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
                      vbasedev.migration_events, false),
     DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
@@ -3475,6 +3478,9 @@ static const TypeInfo vfio_pci_nohotplug_dev_info = {
 
 static void register_vfio_pci_dev_type(void)
 {
+    qdev_prop_on_off_auto_mutable = qdev_prop_on_off_auto;
+    qdev_prop_on_off_auto_mutable.realized_set_allowed = true;
+
     type_register_static(&vfio_pci_dev_info);
     type_register_static(&vfio_pci_nohotplug_dev_info);
 }

On 2/19/25 21:34, Maciej S. Szmigiero wrote:
> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> 
> DEFINE_PROP_ON_OFF_AUTO() property isn't runtime-mutable so using it
> would mean that the source VM would need to decide upfront at startup
> time whether it wants to do a multifd device state transfer at some
> point.
> 
> Source VM can run for a long time before being migrated so it is
> desirable to have a fallback mechanism to the old way of transferring
> VFIO device state if it turns to be necessary.
> 
> This brings this property to the same mutability level as ordinary
> migration parameters, which too can be adjusted at the run time.
> 
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> ---
>   hw/vfio/pci.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 184ff882f9d1..9111805ae06c 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3353,6 +3353,8 @@ static void vfio_instance_init(Object *obj)
>       pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>   }
>   
> +static PropertyInfo qdev_prop_on_off_auto_mutable;

please use another name, like vfio_pci_migration_multifd_transfer_prop.
I wish we could define the property info all at once.

Thanks,

C.


> +
>   static const Property vfio_pci_dev_properties[] = {
>       DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host),
>       DEFINE_PROP_UUID_NODEFAULT("vf-token", VFIOPCIDevice, vf_token),
> @@ -3377,9 +3379,10 @@ static const Property vfio_pci_dev_properties[] = {
>                       VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
>       DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
>                               vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
> -    DEFINE_PROP_ON_OFF_AUTO("x-migration-multifd-transfer", VFIOPCIDevice,
> -                            vbasedev.migration_multifd_transfer,
> -                            ON_OFF_AUTO_AUTO),
> +    DEFINE_PROP("x-migration-multifd-transfer", VFIOPCIDevice,
> +                vbasedev.migration_multifd_transfer,
> +                qdev_prop_on_off_auto_mutable, OnOffAuto,
> +                .set_default = true, .defval.i = ON_OFF_AUTO_AUTO),
>       DEFINE_PROP_BOOL("migration-events", VFIOPCIDevice,
>                        vbasedev.migration_events, false),
>       DEFINE_PROP_BOOL("x-no-mmap", VFIOPCIDevice, vbasedev.no_mmap, false),
> @@ -3475,6 +3478,9 @@ static const TypeInfo vfio_pci_nohotplug_dev_info = {
>   
>   static void register_vfio_pci_dev_type(void)
>   {
> +    qdev_prop_on_off_auto_mutable = qdev_prop_on_off_auto;
> +    qdev_prop_on_off_auto_mutable.realized_set_allowed = true;
> +
>       type_register_static(&vfio_pci_dev_info);
>       type_register_static(&vfio_pci_nohotplug_dev_info);
>   }
>

On 26.02.2025 18:59, Cédric Le Goater wrote:
> On 2/19/25 21:34, Maciej S. Szmigiero wrote:
>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>
>> DEFINE_PROP_ON_OFF_AUTO() property isn't runtime-mutable so using it
>> would mean that the source VM would need to decide upfront at startup
>> time whether it wants to do a multifd device state transfer at some
>> point.
>>
>> Source VM can run for a long time before being migrated so it is
>> desirable to have a fallback mechanism to the old way of transferring
>> VFIO device state if it turns to be necessary.
>>
>> This brings this property to the same mutability level as ordinary
>> migration parameters, which too can be adjusted at the run time.
>>
>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>> ---
>>   hw/vfio/pci.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 184ff882f9d1..9111805ae06c 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -3353,6 +3353,8 @@ static void vfio_instance_init(Object *obj)
>>       pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>>   }
>> +static PropertyInfo qdev_prop_on_off_auto_mutable;
> 
> please use another name, like vfio_pci_migration_multifd_transfer_prop.

Done.

> I wish we could define the property info all at once.

I'm not sure what you mean here, could you please elaborate a bit more?

This property mutability patch was split out from the previous patch
adding the actual x-migration-multifd-transfer VFIO property upon
your request.

> Thanks,
> 
> C.

Thanks,
Maciej

On 2/26/25 22:05, Maciej S. Szmigiero wrote:
> On 26.02.2025 18:59, Cédric Le Goater wrote:
>> On 2/19/25 21:34, Maciej S. Szmigiero wrote:
>>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>>
>>> DEFINE_PROP_ON_OFF_AUTO() property isn't runtime-mutable so using it
>>> would mean that the source VM would need to decide upfront at startup
>>> time whether it wants to do a multifd device state transfer at some
>>> point.
>>>
>>> Source VM can run for a long time before being migrated so it is
>>> desirable to have a fallback mechanism to the old way of transferring
>>> VFIO device state if it turns to be necessary.
>>>
>>> This brings this property to the same mutability level as ordinary
>>> migration parameters, which too can be adjusted at the run time.
>>>
>>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>>> ---
>>>   hw/vfio/pci.c | 12 +++++++++---
>>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index 184ff882f9d1..9111805ae06c 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -3353,6 +3353,8 @@ static void vfio_instance_init(Object *obj)
>>>       pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>>>   }
>>> +static PropertyInfo qdev_prop_on_off_auto_mutable;
>>
>> please use another name, like vfio_pci_migration_multifd_transfer_prop.
> 
> Done.
> 
>> I wish we could define the property info all at once.
> 
> I'm not sure what you mean here, could you please elaborate a bit more?

I meant :

     static const PropertyInfo vfio_pci_migration_multifd_transfer_prop = {
         .name = "OnOffAuto",
         .description = "on/off/auto",
         .enum_table = &OnOffAuto_lookup,
         .get = qdev_propinfo_get_enum,
         .set = qdev_propinfo_set_enum,
         .set_default_value = qdev_propinfo_set_default_value_enum,
         .realized_set_allowed = true,
     };

which requires including "hw/core/qdev-prop-internal.h".

I think your method is preferable. Please add a little comment
before :

     qdev_prop_on_off_auto_mutable = qdev_prop_on_off_auto;
     qdev_prop_on_off_auto_mutable.realized_set_allowed = true;

Thanks,

C.

On 28.02.2025 09:44, Cédric Le Goater wrote:
> On 2/26/25 22:05, Maciej S. Szmigiero wrote:
>> On 26.02.2025 18:59, Cédric Le Goater wrote:
>>> On 2/19/25 21:34, Maciej S. Szmigiero wrote:
>>>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>>>
>>>> DEFINE_PROP_ON_OFF_AUTO() property isn't runtime-mutable so using it
>>>> would mean that the source VM would need to decide upfront at startup
>>>> time whether it wants to do a multifd device state transfer at some
>>>> point.
>>>>
>>>> Source VM can run for a long time before being migrated so it is
>>>> desirable to have a fallback mechanism to the old way of transferring
>>>> VFIO device state if it turns to be necessary.
>>>>
>>>> This brings this property to the same mutability level as ordinary
>>>> migration parameters, which too can be adjusted at the run time.
>>>>
>>>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>>>> ---
>>>>   hw/vfio/pci.c | 12 +++++++++---
>>>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>>> index 184ff882f9d1..9111805ae06c 100644
>>>> --- a/hw/vfio/pci.c
>>>> +++ b/hw/vfio/pci.c
>>>> @@ -3353,6 +3353,8 @@ static void vfio_instance_init(Object *obj)
>>>>       pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>>>>   }
>>>> +static PropertyInfo qdev_prop_on_off_auto_mutable;
>>>
>>> please use another name, like vfio_pci_migration_multifd_transfer_prop.
>>
>> Done.
>>
>>> I wish we could define the property info all at once.
>>
>> I'm not sure what you mean here, could you please elaborate a bit more?
> 
> I meant :
> 
>      static const PropertyInfo vfio_pci_migration_multifd_transfer_prop = {
>          .name = "OnOffAuto",
>          .description = "on/off/auto",
>          .enum_table = &OnOffAuto_lookup,
>          .get = qdev_propinfo_get_enum,
>          .set = qdev_propinfo_set_enum,
>          .set_default_value = qdev_propinfo_set_default_value_enum,
>          .realized_set_allowed = true,
>      };
> 
> which requires including "hw/core/qdev-prop-internal.h".
> 
> I think your method is preferable. Please add a little comment
> before :
> 
>      qdev_prop_on_off_auto_mutable = qdev_prop_on_off_auto;
>      qdev_prop_on_off_auto_mutable.realized_set_allowed = true;

Added a comment above these code lines describing why custom
property type is justified in this case.

> Thanks,
> 
> C.
> 

Thanks,
Maciej