On 6.02.2025 18:32, Fabiano Rosas wrote:
"Maciej S. Szmigiero" <m...@maciej.szmigiero.name> writes:
On 6.02.2025 16:20, Fabiano Rosas wrote:
"Maciej S. Szmigiero" <m...@maciej.szmigiero.name> writes:
(..)
Even if non-TLS behaved the same, why would we choose to port a bug to
the TLS implementation?
We could of course decide at this point to punt the problem forward and
when it shows up, we'd have to go implement gnutls_bye() to allow the
distinction between good-EOF/bad-EOF or maybe add an extra sync at the
end of migration to make sure the last recv() call is only started after
the source has already shutdown() the channel.
If we do some kind of a premature EOF detection then it should probably
work for the non-TLS case too (since that's probably by far the most
common use case).
So adding some MULTIFD_FLAG_EOS would make the most sense and would work
even with QIO_CHANNEL_READ_RELAXED_EOF being set.
Indeed, if MULTIFD_FLAG_EOS is not seen, the recv thread could treat any
EOF as an error. The question is whether we can add that without
disrupting multifd synchronization too much.
In any case we'd still need some kind of a compatibility behavior for
the TLS bit stream emitted by older QEMU versions (which is always
improperly terminated).
There is no compat issue. For <= 9.2, QEMU is still doing an extra
multifd_send_sync_main(), which results in an extra MULTIFD_FLAG_SYNC on
the destination and it gets stuck waiting for the
RAM_SAVE_FLAG_MULTIFD_FLUSH that never comes. Therefore the src always
closes the connection before dst reaches the extra recv().
I test migration both ways with 2 previous QEMU versions and the
gnutls_bye() series passes all tests. I also put an assert at
tlssession.c and never triggers for GNUTLS_E_PREMATURE_TERMINATION. The
MULTIFD_FLAG_EOS should behave the same.
If you are confident that properly terminating the TLS session by adding
gnutls_bye() is the way to go then I'm fine with this - I hope @Peter
and @Daniel are too.
It's now only matter of how soon you can have these gnutls_bye() patches
posted/merged since if I drop the premature_ok stuff the updated series
will depend on them for passing the TLS tests.
Thanks,
Maciej
