Subject: Re: [PATCH V8 mlx5-next 09/15] vfio: Define device migration protocol v2

From: Jason Gunthorpe <[email-masked]>
Date: Thu, 24 Feb 2022 08:39:29 -0400

On Thu, Feb 24, 2022 at 11:41:44AM +0100, Cornelia Huck wrote:
> On Wed, Feb 23 2022, Jason Gunthorpe <[email-masked]> wrote:
> 
> > On Wed, Feb 23, 2022 at 06:06:13PM +0100, Cornelia Huck wrote:
> >> On Sun, Feb 20 2022, Yishai Hadas <[email-masked]> wrote:
> 
> >> > +/*
> >> > + * Indicates the device can support the migration API through
> >> > + * VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE. If present flags must be non-zero and
> >> > + * VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE is supported. The RUNNING and
> >> 
> >> I'm having trouble parsing this. I think what it tries to say is that at
> >> least one of the flags defined below must be set?
> >> 
> >> > + * ERROR states are always supported if this GET succeeds.
> >> 
> >> What about the following instead:
> >> 
> >> "Indicates device support for the migration API through
> >> VFIO_DEVICE_FEATURE_MIG_DEVICE_STATE. If present, the RUNNING and ERROR
> >> states are always supported. Support for additional states is indicated
> >> via the flags field; at least one of the flags defined below must be
> >> set."
> >
> > Almost, 'at least VFIO_MIGRATION_STOP_COPY must be set'
> 
> It feels a bit odd to split the mandatory states between a base layer
> (RUNNING/ERROR) and the ones governed by VFIO_MIGRATION_STOP_COPY. Do we
> want to keep the possibility of a future implementation that does not
> use the semantics indicated by VFIO_MIGRATION_STOP_COPY? 

Yes we do, and when we do that the documentation can reflect that
world. Today, as is, it is mandatory.

Jason