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

[Dr. David Alan Gilbert]

* 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.

Dave

> Thanks,
> Jean
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK