On 3.02.2025 19:20, Peter Xu wrote:
On Thu, Jan 30, 2025 at 11:08:29AM +0100, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero" <maciej.szmigi...@oracle.com>
Multifd send channels are terminated by calling
qio_channel_shutdown(QIO_CHANNEL_SHUTDOWN_BOTH) in
multifd_send_terminate_threads(), which in the TLS case essentially
calls shutdown(SHUT_RDWR) on the underlying raw socket.
Unfortunately, this does not terminate the TLS session properly and
the receive side sees this as a GNUTLS_E_PREMATURE_TERMINATION error.
The only reason why this wasn't causing migration failures is because
the current migration code apparently does not check for migration
error being set after the end of the multifd receive process.
However, this will change soon so the multifd receive code has to be
prepared to not return an error on such premature TLS session EOF.
Use the newly introduced QIOChannelTLS method for that.
It's worth noting that even if the sender were to be changed to terminate
the TLS connection properly the receive side still needs to remain
compatible with older QEMU bit stream which does not do this.
If this is an existing bug, we could add a Fixes.
It is an existing issue but only uncovered by this patch set.
As far as I can see it was always there, so it would need some
thought where to point that Fixes tag.
Two pure questions..
- What is the correct way to terminate the TLS session without this flag?
I guess one would need to call gnutls_bye() like in this GnuTLS example:
https://gitlab.com/gnutls/gnutls/-/blob/2b8c3e4c71ad380bbbffb32e6003b34ecad596e3/doc/examples/ex-client-anon.c#L102
- Why this is only needed by multifd sessions?
What uncovered the issue was switching the load threads to using
migrate_set_error() instead of their own result variable
(load_threads_ret) which you had requested during the previous
patch set version review:
https://lore.kernel.org/qemu-devel/Z1DbH5fwBaxtgrvH@x1n/
Turns out that the multifd receive code always returned
error in the TLS case, just nothing was previously checking for
that error presence.
Another option would be to simply return to using
load_threads_ret like the previous versions did and not
experiment with touching global migration state because
as we can see other places can unintentionally break.
If we go this route then these TLS EOF patches could be
dropped.
Thanks,
Thanks,
Maciej
