Peter Xu <[email protected]> writes: > v3: > - Patch 1 > - Update qcrypto_tls_session_read() doc to reflect the new retval [Dan] > - Update commit message to explain the qatomic_read() change [Dan] > - Patch 2 (old) > - Dropped for now, more at the end > > This is v3 of the series. > > Fabiano fixed graceful shutdowns for multifd channels previously: > > https://lore.kernel.org/qemu-devel/[email protected]/ > > However we can still see an warning when running preempt unit test on TLS, > even though migration functionality will not be affected: > > QTEST_QEMU_BINARY=./qemu-system-x86_64 ./tests/qtest/migration-test --full -r > /x86_64/migration/postcopy/preempt/tls/psk > ... > qemu-kvm: Cannot read from TLS channel: The TLS connection was non-properly > terminated. > ... > > It turns out this is because the crypto code only passes the ->shutdown > field into the read function, however that value can change concurrently in > another thread by a concurrent qio_channel_shutdown(). > > Patch 1 should fix this issue. > > Patch 2 is something I found when looking at this problem, there's no known > issues I am aware of with them, however I still think they're logically > flawed, so I post them together here. > > Please review, thanks. > > ============= ABOUT OLD PATCH 2 =================== > > I dropped it for now to unblock almost patch 1, because patch 1 will fix a > real warning that can be triggered for not only qtest but also normal tls > postcopy migration. > > While I was looking at temporary settings for multifd send iochannels to be > blocking always, I found I cannot explain how migration_tls_channel_end() > currently works, because it writes to the multifd iochannels while the > channels should still be owned (and can be written at the same time?) by > the sender threads. It sounds like a thread-safety issue, or is it not? >
IIUC, the multifd channels will be stuck at p->sem because this is the success path so migration will have already finished when we reach migration_cleanup(). The ram/device state migration will hold the main thread until the multifd channels finish transferring. > Peter Xu (2): > io/crypto: Move tls premature termination handling into QIO layer > migration: Make migration_has_failed() work even for CANCELLING > > include/crypto/tlssession.h | 10 +++------- > crypto/tlssession.c | 7 ++----- > io/channel-tls.c | 21 +++++++++++++++++++-- > migration/migration.c | 3 ++- > 4 files changed, 26 insertions(+), 15 deletions(-)
