Re: [PATCH v2 03/20] vfio/migration: Add VFIO migration pre-copy support

[Alex Williamson]

On Thu, 23 Feb 2023 17:25:12 +0200
Avihai Horon <avihaih@nvidia.com> wrote:

> On 22/02/2023 22:58, Alex Williamson wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Wed, 22 Feb 2023 19:48:58 +0200
> > Avihai Horon <avihaih@nvidia.com> wrote:
> >  
> >> @@ -302,23 +380,44 @@ static void vfio_save_cleanup(void *opaque)
> >>       trace_vfio_save_cleanup(vbasedev->name);
> >>   }
> >>
> >> +static void vfio_state_pending_estimate(void *opaque, uint64_t 
> >> threshold_size,
> >> +                                        uint64_t *must_precopy,
> >> +                                        uint64_t *can_postcopy)
> >> +{
> >> +    VFIODevice *vbasedev = opaque;
> >> +    VFIOMigration *migration = vbasedev->migration;
> >> +
> >> +    if (migration->device_state != VFIO_DEVICE_STATE_PRE_COPY) {
> >> +        return;
> >> +    }
> >> +
> >> +    /*
> >> +     * Initial size should be transferred during pre-copy phase so 
> >> stop-copy
> >> +     * phase will not be slowed down. Report threshold_size to force 
> >> another
> >> +     * pre-copy iteration.
> >> +     */
> >> +    *must_precopy += migration->precopy_init_size ?
> >> +                         threshold_size :
> >> +                         migration->precopy_dirty_size;  
> > This sure feels like we're feeding false data back to the iterator to
> > spoof it to run another iteration, when the vfio migration protocol
> > only recommends that initial_bytes reaches zero before proceeding to
> > stop-copy, it's not a requirement.  What benefit is actually observed
> > from this?  Why is this required for initial pre-copy support?  It
> > seems devious.  
> 
> As previously discussed in the thread that added the pre-copy uAPI [1], 
> the init_bytes can be used by drivers to reduce the downtime.
> For example, mlx5 transfers some metadata to the target so it will be 
> able to pre-allocate resources etc.
> 
> [1] 
> https://lore.kernel.org/kvm/ae4a6259-349d-0131-896c-7a6ea775cc9e@nvidia.com/

Yes, but how does that become a requirement to QEMU that it must
iterate until the initial segment is complete?  Especially when we need
to trigger that behavior via such nefarious means.  AIUI, QEMU should
be allowed to move to stop-copy at any point.  We should make efforts
that QEMU would never decide on its own to move from pre-copy to
stop-copy without completing the init_bytes (which sounds suspiciously
like the purpose of @must_precopy), but if, for instance a user forces a
transition to stop-copy, I don't see that we have any business to
impose a policy to delay that until the init_bytes is complete.  Thanks,

Alex