mirror of
https://gitlab.com/qemu-project/qemu
synced 2024-10-14 23:13:30 +00:00
migration/multifd: Unify multifd and TLS connection paths
During multifd channel creation (multifd_send_new_channel_async) when TLS is enabled, the multifd_channel_connect function is called twice, once to create the TLS handshake thread and another time after the asynchrounous TLS handshake has finished. This creates a slightly confusing call stack where multifd_channel_connect() is called more times than the number of channels. It also splits error handling between the two callers of multifd_channel_connect() causing some code duplication. Lastly, it gets in the way of having a single point to determine whether all channel creation tasks have been initiated. Refactor the code to move the reentrancy one level up at the multifd_new_send_channel_async() level, de-duplicating the error handling and allowing for the next patch to introduce a synchronization point common to all the multifd channel creation, regardless of TLS. Note that the previous code would never fail once p->c had been set. This patch changes this assumption, which affects refcounting, so add comments around object_unref to explain the situation. Reviewed-by: Peter Xu <peterx@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20240206215118.6171-6-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com>
This commit is contained in:
parent
dd904bc13f
commit
2576ae488e
|
@ -869,30 +869,7 @@ out:
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool multifd_channel_connect(MultiFDSendParams *p,
|
static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque);
|
||||||
QIOChannel *ioc,
|
|
||||||
Error **errp);
|
|
||||||
|
|
||||||
static void multifd_tls_outgoing_handshake(QIOTask *task,
|
|
||||||
gpointer opaque)
|
|
||||||
{
|
|
||||||
MultiFDSendParams *p = opaque;
|
|
||||||
QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
|
|
||||||
Error *err = NULL;
|
|
||||||
|
|
||||||
if (!qio_task_propagate_error(task, &err)) {
|
|
||||||
trace_multifd_tls_outgoing_handshake_complete(ioc);
|
|
||||||
if (multifd_channel_connect(p, ioc, &err)) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
trace_multifd_tls_outgoing_handshake_error(ioc, error_get_pretty(err));
|
|
||||||
|
|
||||||
multifd_send_set_error(err);
|
|
||||||
multifd_send_kick_main(p);
|
|
||||||
error_free(err);
|
|
||||||
}
|
|
||||||
|
|
||||||
static void *multifd_tls_handshake_thread(void *opaque)
|
static void *multifd_tls_handshake_thread(void *opaque)
|
||||||
{
|
{
|
||||||
|
@ -900,7 +877,7 @@ static void *multifd_tls_handshake_thread(void *opaque)
|
||||||
QIOChannelTLS *tioc = QIO_CHANNEL_TLS(p->c);
|
QIOChannelTLS *tioc = QIO_CHANNEL_TLS(p->c);
|
||||||
|
|
||||||
qio_channel_tls_handshake(tioc,
|
qio_channel_tls_handshake(tioc,
|
||||||
multifd_tls_outgoing_handshake,
|
multifd_new_send_channel_async,
|
||||||
p,
|
p,
|
||||||
NULL,
|
NULL,
|
||||||
NULL);
|
NULL);
|
||||||
|
@ -920,6 +897,10 @@ static bool multifd_tls_channel_connect(MultiFDSendParams *p,
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Ownership of the socket channel now transfers to the newly
|
||||||
|
* created TLS channel, which has already taken a reference.
|
||||||
|
*/
|
||||||
object_unref(OBJECT(ioc));
|
object_unref(OBJECT(ioc));
|
||||||
trace_multifd_tls_outgoing_handshake_start(ioc, tioc, hostname);
|
trace_multifd_tls_outgoing_handshake_start(ioc, tioc, hostname);
|
||||||
qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
|
qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
|
||||||
|
@ -936,18 +917,7 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
|
||||||
QIOChannel *ioc,
|
QIOChannel *ioc,
|
||||||
Error **errp)
|
Error **errp)
|
||||||
{
|
{
|
||||||
trace_multifd_set_outgoing_channel(
|
qio_channel_set_delay(ioc, false);
|
||||||
ioc, object_get_typename(OBJECT(ioc)),
|
|
||||||
migrate_get_current()->hostname);
|
|
||||||
|
|
||||||
if (migrate_channel_requires_tls_upgrade(ioc)) {
|
|
||||||
/*
|
|
||||||
* tls_channel_connect will call back to this
|
|
||||||
* function after the TLS handshake,
|
|
||||||
* so we mustn't call multifd_send_thread until then
|
|
||||||
*/
|
|
||||||
return multifd_tls_channel_connect(p, ioc, errp);
|
|
||||||
}
|
|
||||||
|
|
||||||
migration_ioc_register_yank(ioc);
|
migration_ioc_register_yank(ioc);
|
||||||
p->registered_yank = true;
|
p->registered_yank = true;
|
||||||
|
@ -959,24 +929,51 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* When TLS is enabled this function is called once to establish the
|
||||||
|
* TLS connection and a second time after the TLS handshake to create
|
||||||
|
* the multifd channel. Without TLS it goes straight into the channel
|
||||||
|
* creation.
|
||||||
|
*/
|
||||||
static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
|
static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
|
||||||
{
|
{
|
||||||
MultiFDSendParams *p = opaque;
|
MultiFDSendParams *p = opaque;
|
||||||
QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
|
QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
|
||||||
Error *local_err = NULL;
|
Error *local_err = NULL;
|
||||||
|
bool ret;
|
||||||
|
|
||||||
trace_multifd_new_send_channel_async(p->id);
|
trace_multifd_new_send_channel_async(p->id);
|
||||||
if (!qio_task_propagate_error(task, &local_err)) {
|
|
||||||
qio_channel_set_delay(ioc, false);
|
if (qio_task_propagate_error(task, &local_err)) {
|
||||||
if (multifd_channel_connect(p, ioc, &local_err)) {
|
ret = false;
|
||||||
return;
|
goto out;
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
trace_multifd_set_outgoing_channel(ioc, object_get_typename(OBJECT(ioc)),
|
||||||
|
migrate_get_current()->hostname);
|
||||||
|
|
||||||
|
if (migrate_channel_requires_tls_upgrade(ioc)) {
|
||||||
|
ret = multifd_tls_channel_connect(p, ioc, &local_err);
|
||||||
|
} else {
|
||||||
|
ret = multifd_channel_connect(p, ioc, &local_err);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (ret) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
out:
|
||||||
trace_multifd_new_send_channel_async_error(p->id, local_err);
|
trace_multifd_new_send_channel_async_error(p->id, local_err);
|
||||||
multifd_send_set_error(local_err);
|
multifd_send_set_error(local_err);
|
||||||
multifd_send_kick_main(p);
|
multifd_send_kick_main(p);
|
||||||
object_unref(OBJECT(ioc));
|
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));
|
||||||
|
}
|
||||||
error_free(local_err);
|
error_free(local_err);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue