Re: [PATCH v2 3/4] virtio-iommu: Support bypass domain

[Eric Auger]

Hi Jean,

On 2/8/22 2:09 PM, Dr. David Alan Gilbert wrote:
> * Jean-Philippe Brucker (jean-philippe@linaro.org) wrote:
>> On Wed, Feb 02, 2022 at 02:21:37PM +0100, Eric Auger wrote:
>>>>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>>>>> index ec02029bb6..a112428c65 100644
>>>>>> --- a/hw/virtio/virtio-iommu.c
>>>>>> +++ b/hw/virtio/virtio-iommu.c
>>>>>> @@ -43,6 +43,7 @@
>>>>>>  
>>>>>>  typedef struct VirtIOIOMMUDomain {
>>>>>>      uint32_t id;
>>>>>> +    bool bypass;
>>>>> I am afraid this will break the migration if you don't change
>>>>> vmstate_domain.
>>>>>
>>>>> See static const VMStateDescription vmstate_domain.
>>>>> Also you need to migrate the new bypass field.
>>>>>
>>>>> Logically we should handle this with a vmstate subsection I think to
>>>>> handle migration of older devices. However I doubt the device has been
>>>>> used in production environment supporting migration so my guess is we
>>>>> may skip that burden and just add the missing field. Adding Juan, Dave &
>>>>> Peter for advices.
>>>> I'm not sure about users of this; if no one has used it then yeh; you
>>>> could bump up the version_id to make it a bit clearer.
>>> Thank you for your input. Yes to me it sounds OK to only bump the
>>> version_id while adding the new field.
>> Ok. Just to make sure we're on the same page, this means we don't support
>> migration from new->old or old->new instances, since the migration stream
>> doesn't carry a version ID for the virtio-iommu-device and domain
>> vmstates, as far as I understand. I also believe backward-incompatible
>> changes are fine this time around, though I don't have much visibility in
>> what's being used.
> I think the stream only has it for top level devices; I've not dug into
> this device.
Not sure I get what you meant:

vmstate_virtio_iommu has a version_id. Also vmstate_domain has one.

Thanks

Eric
>
> Dave
>
>> Thanks,
>> Jean
>>