RE: [PATCH v3 3/3] vfio/migration: vfio/migration: Refactor and fix print of "Migration disabled"

[Duan, Zhenzhong]

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Sent: Tuesday, June 27, 2023 6:57 PM
>To: Duan, Zhenzhong <zhenzhong.duan@intel.com>; Avihai Horon
><avihaih@nvidia.com>
>Cc: alex.williamson@redhat.com; clg@redhat.com; Peng, Chao P
><chao.p.peng@intel.com>; qemu-devel@nongnu.org
>Subject: Re: [PATCH v3 3/3] vfio/migration: vfio/migration: Refactor and fix
>print of "Migration disabled"
>
>On 27/06/2023 03:55, Duan, Zhenzhong wrote:
>>> I guess it makes sense -- the thing that was tieing him was the
>>> global migration blocker, which is now consolidated into the main
>migration blocker.
>>>
>>> My vIOMMU series will relax this condition yes (still same per-device 
>>> scope).
>>> And I will need to check the maximum IOVA in the vIOMMU. But it's new
>>> code I can adjust and Zhenzhong shouldn't worry about the vIOMMU
>>> future stuff
>> A bit confused, you agreed to move vfio_block_giommu_migration into
>> migration.c
>>
>>> but I don't expect to influence location, say if it gets moved into
>>> migration.c
>> and final decision is no move for influencing location reason? Do I
>misunderstand?
>
>Sorry for the confusion.
>
>The thing is that I will need 'similar code' to test if a vIOMMU is enabled or 
>not.
>The reason being that dirty tracking will need this to understand what to track
>meaning to decide whether we track the whole address space or just the
>linear map[0]. And all that code is in common, not migration.c and where I
>use it will have to look at all address spaces (because dirty tracking is 
>started
>for all devices, so there's no vbasedev to look at).
>
>Eventually after the vIOMMU stuff, the migration blocker condition will look
>more or less like this:
>
>       return (!vfio_viommu_preset(vbasedev) ||
>               (vfio_viommu_preset(vbasedev) &&
>                !vfio_viommu_get_max_iova(&max)))
>
>Whereby vfio_viommu_preset(vbasedev) is what you currently call
>vfio_block_giommu_migration(vbasedev) in current patch. So perhaps I would
>say to leave it in common.c and rename it to vfio_viommu_preset(vbasedev)
>with a comment on top for /* Block migration with a vIOMMU */
>
>Then when the time comes I can introduce in my vIOMMU series a
>vfio_viommu_possible() [or some other name] when starting dirty tracking
>which looks all VFIOAddressSpaces and reuses your
>vfio_viommu_preset(vbasedev). This should cover current and future needs,
>without going to great extents beyond the purpose of this patch?

Thanks for detailed explanation, clear, I'll update based on above suggestions.

Zhenzhong