Re: [PATCH v1] docs/devel: Add VFIO device migration documentation

[Cornelia Huck <cohuck@redhat.com>]

On Thu, 29 Oct 2020 13:05:19 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Thu, 29 Oct 2020 23:11:16 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> 
> > Thanks for corrections Cornelia. I had done the corrections you 
> > suggested I had not replied, see my comments on couple of places where I 
> > disagree.
> > 
> > 
> > On 10/29/2020 5:22 PM, Cornelia Huck wrote:  
> > > On Thu, 29 Oct 2020 11:23:11 +0530
> > > Kirti Wankhede <kwankhede@nvidia.com> wrote:

> > >> +Detailed description of UAPI for VFIO device for migration is in the comment
> > >> +above ``vfio_device_migration_info`` structure definition in header file
> > >> +linux-headers/linux/vfio.h.    
> > > 
> > > I think I'd copy that to this file. If I'm looking at the
> > > documentation, I'd rather not go hunting for source code to find out
> > > what structure you are talking about. Plus, as it's UAPI, I don't
> > > expect it to change much, so it should be easy to keep the definitions
> > > in sync (famous last words).
> > >     
> > 
> > I feel its duplication of documentation. I would like to know others 
> > views as well.  
> 
> 
> TBH I don't think it's necessary here either, we're documenting the
> QEMU interaction with the uAPI, the uAPI itself is documented in the
> kernel header.  I don't think it would be unreasonable to ask someone
> trying to understand this to look at both sources together.

Ok, I can live with that. But let me correct some nits :)

"A detailed description of the UAPI for VFIO device migration can be
found in the comment for the ``vfio_device_migration_info`` structure
in the header file linux-headers/linux/vfio.h."

(...)

> > >> +remaining data for VFIO device till pending_bytes returned by vendor driver
> > >> +is zero.    
> > > 
> > > "...and interactively copies the remaining data for the VFIO device
> > > until the vendor driver indicates that no data remains (pending_bytes
> > > is zero)." ?  
> 
> 
> Connie, was that intentional to replace "iteratively" with
> "interactively"?  Iteratively seems correct to me.

Eh, should be "iteratively", of course. Too many meetings :/