Re: [PATCH 3/9] vfio/migration: Refactor vfio_devices_all_running_and_mig_active() logic

[Joao Martins]

On 17/12/2024 09:38, Avihai Horon wrote:
>>>>> +bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase 
>>>>> *bcontainer)
>>>>>    {
>>>>> -    VFIODevice *vbasedev;
>>>>> -
>>>>> -    if (!migration_is_active()) {
>>>>> +    if (!migration_is_running()) {
>>>>>            return false;
>>>>>        }
>>>>>
>>>>> -    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>>>>> -        VFIOMigration *migration = vbasedev->migration;
>>>>> -
>>>>> -        if (!migration) {
>>>>> -            return false;
>>>>> -        }
>>>>> -
>>>>> -        if (vfio_device_state_is_running(vbasedev) ||
>>>>> -            vfio_device_state_is_precopy(vbasedev)) {
>>>>> -            continue;
>>>>> -        } else {
>>>>> -            return false;
>>>>> -        }
>>>> Functionally the change implies that even if non-migratable VFIO devices 
>>>> behind
>>>> IOMMUs with dirty tracking would still sync DMA bitmap. I think this is OK
>>>> as it
>>>> increases the coverage for calc-dirty-rate (provided my comment in an 
>>>> earlier
>>>> patch) such that if you try to get a dirty rate included the IOMMU
>>>> invalidations
>>>> marking the bits accordingly.
>>> We still have the "if (!migration_is_running())" check above, so 
>>> non-migratable
>>> VFIO devices won't sync.
>>> But that's a valid point for when we'll allow VFIO log syncs for clac-dirty-
>>> rate.
>>>
>> It's the other way around :) This change helps calc-dirty-rate because you 
>> can
>> use it and still account for DMA unmap based dirties.
>>
>> migration_is_running just stops logs if migration is not running. And that
>> doesn't care about VFIO migation support.
>>
>> But if migration is running, whether the device supports migration or not...
> 
> If the device doesn't support migration then migration can't run, no?

/facepalm yes :D

We still have migration blockers

> But anyway, as we talked in the other thread, I can untie this from migration
> and then, as you said above, it may also dirty sync for non-migratable devices
> which will make calc-dirty-rate more accurate.
> 

Yeap