On Wed, Nov 06, 2024 at 05:49:23PM +0530, Prasad Pandit wrote:
> On Tue, 5 Nov 2024 at 18:30, Peter Xu <pet...@redhat.com> wrote:
> > 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 }.
>
> * I see, okay.
>
> > 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..
>
> * See, current (this series) 'if' conditional in the
> migration_ioc_process_incoming() function is simple as:
>
> if (qio_channel_has_feature(ioc,
> QIO_CHANNEL_FEATURE_READ_MSG_PEEK)) { peek_magic_bytes() ... }
IIUC we can't simply change it to this. We can do this with a compat
property and we can start sending a magic in the preempt channel, but then
the code still needs to keep working with old binaries of QEMU, so in all
cases we'll need to keep the old complexity for quite a while.
IOW, I think it may break migrations from old QEMUs when postcopy preempt
is enabled, because then the new QEMU (with your patch applied) will always
peek the byte assuming the magic is there, but old binaries don't have
those.
Handshake, in my mind, will use a totally separate path, then the hope is
we'll move to that with more machine types and finally obsolete / remove
this path.
>
> If we don't send magic value for the postcopy channel, then we avoid
> peeking into magic bytes when postcopy is enabled, because otherwise
> thread will block peeking into the magic bytes, so the 'if'
> conditional becomes:
>
> if (migrate_multifd() && !migrate_postcopy() &&
> qio_channel_has_feature (...) ) {
> peek_magic_bytes()
> ...
> } else {
> When migrate_postcopy() is true
> It'll reach here not only for the 'Postcopy' channel, but even
> for the 'default' and 'multifd' channels which send the magic bytes.
> Then here again we'll need to identify different channels, right?
> }
>
> * Let's not make it so complex. Let's send the magic value for the
Firstly, the complexity is there on migration, requiring it work with old
qemu binaries, bi-directional by default. In your case you're changing
receiving side, so it's even more important, because it's common old qemu
migrates to new ones.
What I want to avoid is we introduce two flags in a short period doing the
same thing. If we want we can merge the effort, I'll leave that to you and
Fabiano to decide, so that maybe you can work out the channel establishment
part of things.
But note again that I still think your goal of enabling multifd + postcopy
may not require that new flag yet, simply because after 7.2 qemu will
connect preempt channel later than the main channel. I think logically
QEMU can identify which channel is which: the preempt channel must be
established in this case after both main thread and multifd threads.
Meanwhile, as mentioned above, we still need to make pre-7.2 works in
general on migration in most cases (when there's network instability it
won't work.. so that's unavoidable).. it's indeed slightly complicated,
but hopefully could still work.
In all cases, we may want to test postcopy preempt from a 7.1 qemu to the
new qemu to keep it working when the patch is ready (and you can already
try that with current patch).
> postcopy channel and simplify it. If 'handshake' feature is going to
> redo it, so be it, what's the difference? OR maybe we can align it
> with the 'handshake' feature or as part of it or something like that.
>
> @Fabiano Rosas : may I know more about the 'handshake' feature? What
> it'll do and not do?
>
> Thank you.
> ---
> - Prasad
>
--
Peter Xu
