Re: [PATCH for-5.0 v11 18/20] virtio-iommu: Support migration

[Auger Eric <eric.auger@redhat.com>]

Hi Peter,

On 12/10/19 9:01 PM, Peter Xu wrote:
> On Fri, Nov 22, 2019 at 07:29:41PM +0100, Eric Auger wrote:
>> +static const VMStateDescription vmstate_virtio_iommu_device = {
>> +    .name = "virtio-iommu-device",
>> +    .minimum_version_id = 1,
>> +    .version_id = 1,
>> +    .post_load = iommu_post_load,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_GTREE_DIRECT_KEY_V(domains, VirtIOIOMMU, 1,
>> +                                   &vmstate_domain, viommu_domain),
>> +        VMSTATE_GTREE_DIRECT_KEY_V(endpoints, VirtIOIOMMU, 1,
>> +                                   &vmstate_endpoint, viommu_endpoint),
> 
> IIUC vmstate_domain already contains all the endpoint information (in
> endpoint_list of vmstate_domain), but here we migrate it twice. 

I migrated both because at that time I considered we could have
endpoints not attached to any domains but I think I can now simplify
based on the fact any EP is attached.


 I
> suppose that's why now we need reconstruct_ep_domain_link() to fixup
> the duplicated migration?

Even if I only migrate the domain gtree, I need to reconstruct the
ep->domain which was not migrated, on purpose, as it pointed to the old
domain in the origin.
> 
> Then I'll instead ask whether we can skip migrating here?  Then in
> post_load we simply:
> 
>   foreach(domain)
>     foreach(endpoint in domain)
>       g_tree_insert(s->endpoints);
> 
> It might help to avoid the reconstruct_ep_domain_link ugliness?
I agree that it is simpler. Also need to update the ep->domain as
mentionned above. Thank you for the suggestion.


> 
> And besides, I also agree with Jean that the endpoint data structure
> could be reused with IOMMUDevice somehow.

As I replied to Jean, I think it makes sense to keep both structures as
endpoints are not indexed by the same key and the bus number is resolved
later.

Thanks

Eric
> 
> Thanks,
>