On Fri, Feb 07, 2025 at 02:55:57PM -0300, Fabiano Rosas wrote: > Peter Xu <pet...@redhat.com> writes: > > > On Fri, Feb 07, 2025 at 11:27:51AM -0300, Fabiano Rosas wrote: > >> QEMU's TLS session code provides no way to call gnutls_bye() to > >> terminate a TLS session. Callers of qcrypto_tls_session_read() can > >> choose to ignore a GNUTLS_E_PREMATURE_TERMINATION error by setting the > >> gracefulTermination argument. > >> > >> The QIOChannelTLS ignores the premature termination error whenever > >> shutdown() has already been issued. This is not enough anymore for the > >> migration code due to changes [1] in the synchronization between > >> migration source and destination. > > > > This sentence seems to say commit [1] changed something on the tls > > condition, but IMHO fundamentally the issue is multifd recv thread model > > that relies on blocking readv() rather than request-based (like what src > > multifd does). > > > > Now src uses either shutdown() or close() to kick dest multifd recv threads > > out from readv(). That has nothing to do with what we do during complete() > > with those sync messages.. referencing it is ok, but we'll need to > > reference also the other commit to be clear pre-9.0 can also be prone to > > this. To me, it's more important to mention the root cause on the multifd > > recv thread model, which requires explicit tls terminations. > > > > I didn't want to go into too much detail in a commit for crypto/. The
You already did so by referencing a multifd commit that changes how complete() works! > motivation for *this* patch is just: migration needs it. What about: > > The QIOChannelTLS ignores the premature termination error whenever > shutdown() has already been issued. This was found to be not enough for > the migration code because shutdown() might not have been issued before > the connection is terminated. Looks good to me, thanks. -- Peter Xu
