Re: [PATCH 2/5] migration/postcopy: magic value for postcopy channel

[Peter Xu <peterx@redhat.com>]

On Tue, Nov 05, 2024 at 04:49:23PM +0530, Prasad Pandit wrote:
> On Mon, 4 Nov 2024 at 22:48, Peter Xu <peterx@redhat.com> wrote:
> > Firstly, we'll need a way to tell mgmt that the new qemu binary supports
> > enablement of both multifd + postcopy feature.  That can be done with a
> >
> >   "features": [ "postcopy-with-multifd-precopy" ]
> >
> > Flag attached to the QMP "migrate" command.
> 
> * IIUC, currently virsh(1)/libvirtd(8) sends the migration command to
> the destination to inform it of the migration features to use, whether
> to use multifd or postcopy or none etc. Based on that the destination
> QEMU prepares to accept incoming VM. Not sure how/what above flag
> shall benefit.

See:

https://www.qemu.org/docs/master/devel/qapi-code-gen.html

        Sometimes, the behaviour of QEMU changes compatibly, but without a
        change in the QMP syntax (usually by allowing values or operations
        that previously resulted in an error). QMP clients may still need
        to know whether the extension is available.

        For this purpose, a list of features can be specified for
        definitions, enumeration values, and struct members. Each feature
        list member can either be { 'name': STRING, '*if': COND }, or
        STRING, which is shorthand for { 'name': STRING }.

> 
> > Then, I think we don't need a magic for preempt channel, because new qemu
> > binaries (after 7.2) have no issue on out-of-order connections between main
> > / preempt channel.  Preempt channel is always connected later than main.
> >
> > It means in the test logic of "which channel is which", it should be:
> >
> >   - If it's a multifd channel (check multifd header match), it's a multifd
> >     channel, otherwise,
> >
> >     - if main channel is not present yet, it must be the main channel (and
> >       we can double check the main channel magic), otherwise,
> >
> >     - it's the preempt channel
> >
> > With that, I think we can drop the new magic sent here.
> 
> * Sending magic value on the postcopy channel only makes it consistent
> with other channels. There's no harm in sending it. Explicitly
> defining/sending the magic value is better than leaving it for the
> code/reader to figure it out. Is there a compelling reason to drop it?
> IMO, we should either define/send the magic value for all channels or
> none. Both ways are consistent. Defining it for few and not for others
> does not seem right.

It's a legacy issue as not all features are developed together, and that
was planned to be fixed together with handshake.  I think the handshake
could introduce one header on top to pair channels.

IMHO it is an overkill to add a feature now if it works even if tricky,
because it's not the 1st day it was tricky. And we're going to have another
header very soon..

Thanks,

-- 
Peter Xu