OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

migration/multifd: Cleanup TLS iochannel referencing

Browse Source 
Commit a1af605bd5 ("migration/multifd: fix hangup with TLS-Multifd due to
blocking handshake") introduced a thread for TLS channels, which will
resolve the issue on blocking the main thread.  However in the same commit
p->c is slightly abused just to be able to pass over the pointer "p" into
the thread.

That's the major reason we'll need to conditionally free the io channel in
the fault paths.

To clean it up, using a separate structure to pass over both "p" and "tioc"
in the tls handshake thread.  Then we can make it a rule that p->c will
never be set until the channel is completely setup.  With that, we can drop
the tricky conditional unref of the io channel in the error path.

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240222095301.171137-2-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
This commit is contained in:
Peter Xu 2024-02-22 17:52:57 +08:00
parent d13f0026c7
commit 9221e3c6a2
1 changed files with 23 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
migration/multifd.c
View File

@ -891,16 +891,22 @@ out:
static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque);
typedef struct {
MultiFDSendParams *p;
QIOChannelTLS *tioc;
} MultiFDTLSThreadArgs;
static void *multifd_tls_handshake_thread(void *opaque)
{
MultiFDSendParams *p = opaque;
QIOChannelTLS *tioc = QIO_CHANNEL_TLS(p->c);
MultiFDTLSThreadArgs *args = opaque;
qio_channel_tls_handshake(tioc,
qio_channel_tls_handshake(args->tioc,
multifd_new_send_channel_async,
p,
args->p,
NULL,
NULL);
g_free(args);
return NULL;
}
@ -910,6 +916,7 @@ static bool multifd_tls_channel_connect(MultiFDSendParams *p,
{
MigrationState *s = migrate_get_current();
const char *hostname = s->hostname;
MultiFDTLSThreadArgs *args;
QIOChannelTLS *tioc;
tioc = migration_tls_client_create(ioc, hostname, errp);
@ -924,11 +931,14 @@ static bool multifd_tls_channel_connect(MultiFDSendParams *p,
object_unref(OBJECT(ioc));
trace_multifd_tls_outgoing_handshake_start(ioc, tioc, hostname);
qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
p->c = QIO_CHANNEL(tioc);
args = g_new0(MultiFDTLSThreadArgs, 1);
args->tioc = tioc;
args->p = p;
p->tls_thread_created = true;
qemu_thread_create(&p->tls_thread, "multifd-tls-handshake-worker",
multifd_tls_handshake_thread, p,
multifd_tls_handshake_thread, args,
QEMU_THREAD_JOINABLE);
return true;
}
@ -941,6 +951,7 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
migration_ioc_register_yank(ioc);
p->registered_yank = true;
/* Setup p->c only if the channel is completely setup */
p->c = ioc;
p->thread_created = true;
@ -994,14 +1005,12 @@ out:
trace_multifd_new_send_channel_async_error(p->id, local_err);
multifd_send_set_error(local_err);
if (!p->c) {
/*
* If no channel has been created, drop the initial
* reference. Otherwise cleanup happens at
* multifd_send_channel_destroy()
*/
object_unref(OBJECT(ioc));
}
/*
* For error cases (TLS or non-TLS), IO channel is always freed here
* rather than when cleanup multifd: since p->c is not set, multifd
* cleanup code doesn't even know its existence.
*/
object_unref(OBJECT(ioc));
error_free(local_err);
}