Re: [PATCH v1] migration: fail the cap check if it requires the use of deferred incoming

[Peter Xu]

On Fri, May 19, 2023 at 11:30:31AM -0400, Peter Xu wrote:
> On Fri, May 19, 2023 at 02:34:57AM +0000, Wang, Wei W wrote:
> > On Friday, May 19, 2023 3:20 AM, Peter Xu wrote:
> > > On Fri, May 19, 2023 at 12:00:26AM +0800, Wei Wang wrote:
> > > > qemu_start_incoming_migration needs to check the number of multifd
> > > > channels or postcopy ram channels to configure the backlog parameter 
> > > > (i.e.
> > > > the maximum length to which the queue of pending connections for
> > > > sockfd may grow) of listen(). So multifd and postcopy-preempt caps
> > > > require the use of deferred incoming, that is, calling
> > > > qemu_start_incoming_migration should be deferred via qmp or hmp
> > > > commands after the cap of multifd and postcopy-preempt are configured.
> > > >
> > > > Check if deferred incoming is used when enabling multifd or
> > > > postcopy-preempt, and fail the check with error messages if not.
> > > >
> > > > Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> > > 
> > > IIUC this will unfortunately break things like:
> > > 
> > >   -global migration.x-postcopy-preempt=on
> > > 
> > > where the cap is actually applied before incoming starts even with !defer 
> > > so
> > > it should still work.
> > 
> > Actually the patch doesn’t check "!defer". It just checks if incoming has 
> > been started
> > or not. It allows the 2 caps to be set only before incoming starts. So I 
> > think the above
> > should work.
> 
> Ah yes indeed it keeps working, because we apply -global bits before setup
> sockets. Then it's fine by me since that's the only thing I would still
> like to keep it working. :)
> 
> If so, can we reword the error message a bit?  Obviously as you said we're
> not really checking against -defer, but established channels.  The problem
> is if something is established without knowing multifd being there it may
> not work for multifd or preempt, not strictly about defer.
> 
> How about:
> 
>   "Multifd/Preempt-Mode cannot be modified if incoming channel has setup"
> 
> ?

We may also want to trap the channel setups on num:

migrate_params_test_apply():

    if (params->has_multifd_channels) {
        dest->multifd_channels = params->multifd_channels;
    }

-- 
Peter Xu