On Thu, Mar 31, 2022 at 11:08:54AM -0400, Peter Xu wrote:
> This patch is based on the async preempt channel creation. It continues
> wiring up the new channel with TLS handshake to destionation when enabled.
>
> Note that only the src QEMU needs such operation; the dest QEMU does not
> need any change for TLS support due to the fact that all channels are
> established synchronously there, so all the TLS magic is already properly
> handled by migration_tls_channel_process_incoming().
>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> migration/postcopy-ram.c | 60 +++++++++++++++++++++++++++++++++++-----
> migration/trace-events | 1 +
> 2 files changed, 54 insertions(+), 7 deletions(-)
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index ab2a50cf45..f5ba176862 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -36,6 +36,7 @@
> #include "socket.h"
> #include "qemu-file-channel.h"
> #include "yank_functions.h"
> +#include "tls.h"
>
> /* Arbitrary limit on size of each discard command,
> * keeps them around ~200 bytes
> @@ -1552,15 +1553,15 @@ bool
> postcopy_preempt_new_channel(MigrationIncomingState *mis, QEMUFile *file)
> return true;
> }
>
> +/*
> + * Setup the postcopy preempt channel with the IOC. If ERROR is specified,
> + * setup the error instead. This helper will free the ERROR if specified.
> + */
> static void
> -postcopy_preempt_send_channel_new(QIOTask *task, gpointer opaque)
> +postcopy_preempt_send_channel_done(MigrationState *s,
> + QIOChannel *ioc, Error *local_err)
> {
> - MigrationState *s = opaque;
> - QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
> - Error *local_err = NULL;
> -
> - if (qio_task_propagate_error(task, &local_err)) {
> - /* Something wrong happened.. */
> + if (local_err) {
> migrate_set_error(s, local_err);
> error_free(local_err);
> } else {
> @@ -1574,6 +1575,51 @@ postcopy_preempt_send_channel_new(QIOTask *task,
> gpointer opaque)
> * postcopy_qemufile_src to know whether it failed or not.
> */
> qemu_sem_post(&s->postcopy_qemufile_src_sem);
> +}
> +
> +static void
> +postcopy_preempt_tls_handshake(QIOTask *task, gpointer opaque)
> +{
> + MigrationState *s = opaque;
> + QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
If using g_autoptr(QIOChannel) ioc = ...
> + Error *err = NULL;
local_err is normal naming
> +
> + qio_task_propagate_error(task, &err);
> + postcopy_preempt_send_channel_done(s, ioc, err);
> + object_unref(OBJECT(ioc));
...not needed with g_autoptr
> +}
> +
> +static void
> +postcopy_preempt_send_channel_new(QIOTask *task, gpointer opaque)
> +{
> + MigrationState *s = opaque;
> + QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
If you use g_autoptr(QIOChannel)
> + QIOChannelTLS *tioc;
> + Error *local_err = NULL;
> +
> + if (qio_task_propagate_error(task, &local_err)) {
> + assert(local_err);
I don't think we really need to add these asserts everywhere we
handle a failure path do we ?
> + goto out;
> + }
> +
> + if (migrate_channel_requires_tls(ioc)) {
> + tioc = migration_tls_client_create(s, ioc, s->hostname, &local_err);
> + if (!tioc) {
> + assert(local_err);
> + goto out;
> + }
> + trace_postcopy_preempt_tls_handshake();
> + qio_channel_set_name(QIO_CHANNEL(tioc), "migration-tls-preempt");
> + qio_channel_tls_handshake(tioc, postcopy_preempt_tls_handshake,
> + s, NULL, NULL);
> + /* Setup the channel until TLS handshake finished */
> + object_unref(OBJECT(ioc));
...not needed with g_autoptr
> + return;
> + }
> +
> +out:
> + /* This handles both good and error cases */
> + postcopy_preempt_send_channel_done(s, ioc, local_err);
> object_unref(OBJECT(ioc));
...also not needed with g_autoptr
> }
>
> diff --git a/migration/trace-events b/migration/trace-events
> index b21d5f371f..00ab2e1b96 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -287,6 +287,7 @@ postcopy_request_shared_page(const char *sharer, const
> char *rb, uint64_t rb_off
> postcopy_request_shared_page_present(const char *sharer, const char *rb,
> uint64_t rb_offset) "%s already %s offset 0x%"PRIx64
> postcopy_wake_shared(uint64_t client_addr, const char *rb) "at 0x%"PRIx64"
> in %s"
> postcopy_page_req_del(void *addr, int count) "resolved page req %p total %d"
> +postcopy_preempt_tls_handshake(void) ""
> postcopy_preempt_new_channel(void) ""
> postcopy_preempt_thread_entry(void) ""
> postcopy_preempt_thread_exit(void) ""
> --
> 2.32.0
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
