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

[Dr. David Alan Gilbert]

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

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