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

[Fabiano Rosas <farosas@suse.de>]

Prasad Pandit <ppandit@redhat.com> writes:

> On Tue, 5 Nov 2024 at 18:30, Peter Xu <peterx@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() ... }
>
> 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
> 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?

What we're thinking is having an initial exchange of information between
src & dst as soon as migration starts and that would sync the
capabilities and parameters between both sides. Which would then be
followed by a channel establishment phase that would open each necessary
channel (according to caps) in order, removing the current ambiguity.

>
> Thank you.
> ---
>   - Prasad