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

From: Jason Gunthorpe <[email-masked]>
Date: Wed, 23 Feb 2022 20:46:22 -0400

On Wed, Feb 23, 2022 at 06:06:13PM +0100, Cornelia Huck wrote:
> On Sun, Feb 20 2022, Yishai Hadas <[email-masked]> wrote:
> 
> > diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> > index ca69516f869d..3bbadcdbc9c8 100644
> > +++ b/include/linux/vfio.h
> > @@ -56,6 +56,14 @@ struct vfio_device {
> >   *         match, -errno for abort (ex. match with insufficient or incorrect
> >   *         additional args)
> >   * @device_feature: Fill in the VFIO_DEVICE_FEATURE ioctl
> > + * @migration_set_state: Optional callback to change the migration state for
> > + *         devices that support migration. The returned FD is used for data
> > + *         transfer according to the FSM definition. The driver is responsible
> > + *         to ensure that FD reaches end of stream or error whenever the
> > + *         migration FSM leaves a data transfer state or before close_device()
> > + *         returns.
> > + * @migration_get_state: Optional callback to get the migration state for
> > + *         devices that support migration.
> 
> Nit: I'd add "mandatory for VFIO_DEVICE_FEATURE_MIGRATION migration
> support" to both descriptions to be a bit more explicit.

Ok

> > +/*
> > + * 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'

Thanks,
Jason