On Thu, Feb 24, 2022 at 11:41:44AM +0100, Cornelia Huck wrote: > On Wed, Feb 23 2022, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > > > On Wed, Feb 23, 2022 at 06:06:13PM +0100, Cornelia Huck wrote: > >> On Sun, Feb 20 2022, Yishai Hadas <yishaih@xxxxxxxxxx> 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
