Re: [PATCH v16 QEMU 08/16] vfio: Register SaveVMHandlers for VFIO device

[Kirti Wankhede]

On 5/6/2020 10:23 PM, Dr. David Alan Gilbert wrote:
> * Cornelia Huck (address@hidden) wrote:
> > On Wed, 6 May 2020 02:38:46 -0400
> > Yan Zhao <address@hidden> wrote:
> >
> > > On Tue, May 05, 2020 at 12:37:26PM +0800, Alex Williamson wrote:
> > > > It's been a long time, but that doesn't seem like what I was asking.
> > > > The sysfs version checking is used to select a target that is likely to
> > > > succeed, but the migration stream is still generated by a user and the
> > > > vendor driver is still ultimately responsible for validating that
> > > > stream.  I would hope that a vendor migration stream therefore starts
> > > > with information similar to that found in the sysfs interface, allowing
> > > > the receiving vendor driver to validate the source device and vendor
> > > > software version, such that we can fail an incoming migration that the
> > > > vendor driver deems incompatible.  Ideally the vendor driver might also
> > > > include consistency and sequence checking throughout the stream to
> > > > prevent a malicious user from exploiting the internal operation of the
> > > > vendor driver.  Thanks,
> >
> > Some kind of somewhat standardized marker for driver/version seems like
> > a good idea. Further checking is also a good idea, but I think the
> > details of that need to be left to the individual drivers.
>
> Standardised markers like that would be useful; although the rules of
> how to compare them might be a bit vendor specific; but still - it would
> be good for us to be able to dump something out when it all goes wrong.

Such checking should already there in vendor driver. Vendor driver might 
also support across version migration. I think checking in QEMU again 
would be redundant. Let vendor driver handle version checks.

Thanks,
Kirti

> > > >    
> > > maybe we can add a rw field migration_version in
> > > struct vfio_device_migration_info besides sysfs interface ?
> > >
> > > when reading it in src, it gets the same string as that from sysfs;
> > > when writing it in target, it returns success or not to check
> > > compatibility and fails the migration early in setup phase.
> >
> > Getting both populated from the same source seems like a good idea.
> >
> > Not sure if a string is the best value to put into a migration stream;
> > maybe the sysfs interface can derive a human-readable string from a
> > more compact value to be put into the migration region (and ultimately
> > the stream)? Might be overengineering, just thinking out aloud here.
>
> A string might be OK fi you specify a little about it.
>
> Dave
>
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK