Add a task dispatcher for gnutls_bye similar to the

qio_channel_tls_handshake_task(). The gnutls_bye() call might be

interrupted and so it needs to be rescheduled.

The migration code will make use of this to help the migration

destination identify a premature EOF. Once the session termination is

in place, any EOF that happens before the source issued gnutls_bye()

will be considered an error.

Signed-off-by: Fabiano Rosas <farosas@suse.de>

---

include/io/channel-tls.h | 12 ++++++

io/channel-tls.c | 84 ++++++++++++++++++++++++++++++++++++++++

io/trace-events | 5 +++

3 files changed, 101 insertions(+)

diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h

index XXXXXXX..XXXXXXX 100644

--- a/include/io/channel-tls.h

+++ b/include/io/channel-tls.h

@@ -XXX,XX +XXX,XX @@ struct QIOChannelTLS {

QCryptoTLSSession *session;

QIOChannelShutdown shutdown;

guint hs_ioc_tag;

+ guint bye_ioc_tag;

};

+/**

+ * qio_channel_tls_bye:

+ * @ioc: the TLS channel object

+ * @errp: pointer to a NULL-initialized error object

+ *

+ * Perform the TLS session termination. This method will return

+ * immediately and the termination will continue in the background,

+ * provided the main loop is running.

+ */

+void qio_channel_tls_bye(QIOChannelTLS *ioc, Error **errp);

+

/**

* qio_channel_tls_new_server:

* @master: the underlying channel object

diff --git a/io/channel-tls.c b/io/channel-tls.c

index XXXXXXX..XXXXXXX 100644

--- a/io/channel-tls.c

+++ b/io/channel-tls.c

@@ -XXX,XX +XXX,XX @@ void qio_channel_tls_handshake(QIOChannelTLS *ioc,

qio_channel_tls_handshake_task(ioc, task, context);

}

+static gboolean qio_channel_tls_bye_io(QIOChannel *ioc, GIOCondition condition,

+ gpointer user_data);

+

+static void qio_channel_tls_bye_task(QIOChannelTLS *ioc, QIOTask *task,

+ GMainContext *context)

+{

+ GIOCondition condition;

+ QIOChannelTLSData *data;

+ int status;

+ Error *err = NULL;

+

+ status = qcrypto_tls_session_bye(ioc->session, &err);

+

+ if (status < 0) {

+ trace_qio_channel_tls_bye_fail(ioc);

+ qio_task_set_error(task, err);

+ qio_task_complete(task);

+ return;

+ }

+

+ if (status == QCRYPTO_TLS_BYE_COMPLETE) {

+ qio_task_complete(task);

+ return;

+ }

+

+ data = g_new0(typeof(*data), 1);

+ data->task = task;

+ data->context = context;

+

+ if (context) {

+ g_main_context_ref(context);

+ }

+

+ if (status == QCRYPTO_TLS_BYE_SENDING) {

+ condition = G_IO_OUT;

+ } else {

+ condition = G_IO_IN;

+ }

+

+ trace_qio_channel_tls_bye_pending(ioc, status);

+ ioc->bye_ioc_tag = qio_channel_add_watch_full(ioc->master, condition,

+ qio_channel_tls_bye_io,

+ data, NULL, context);

+}

+

+

+static gboolean qio_channel_tls_bye_io(QIOChannel *ioc, GIOCondition condition,

+ gpointer user_data)

+{

+ QIOChannelTLSData *data = user_data;

+ QIOTask *task = data->task;

+ GMainContext *context = data->context;

+ QIOChannelTLS *tioc = QIO_CHANNEL_TLS(qio_task_get_source(task));

+

+ tioc->bye_ioc_tag = 0;

+ g_free(data);

+ qio_channel_tls_bye_task(tioc, task, context);

+

+ if (context) {

+ g_main_context_unref(context);

+ }

+

+ return FALSE;

+}

+

+static void propagate_error(QIOTask *task, gpointer opaque)

+{

+ qio_task_propagate_error(task, opaque);

+}

+

+void qio_channel_tls_bye(QIOChannelTLS *ioc, Error **errp)

+{

+ QIOTask *task;

+

+ task = qio_task_new(OBJECT(ioc), propagate_error, errp, NULL);

+

+ trace_qio_channel_tls_bye_start(ioc);

+ qio_channel_tls_bye_task(ioc, task, NULL);

+}

static void qio_channel_tls_init(Object *obj G_GNUC_UNUSED)

{

@@ -XXX,XX +XXX,XX @@ static int qio_channel_tls_close(QIOChannel *ioc,

g_clear_handle_id(&tioc->hs_ioc_tag, g_source_remove);

}

+ if (tioc->bye_ioc_tag) {

+ trace_qio_channel_tls_bye_cancel(ioc);

+ g_clear_handle_id(&tioc->bye_ioc_tag, g_source_remove);

+ }

+

return qio_channel_close(tioc->master, errp);

}

diff --git a/io/trace-events b/io/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/io/trace-events

+++ b/io/trace-events

@@ -XXX,XX +XXX,XX @@ qio_channel_tls_handshake_pending(void *ioc, int status) "TLS handshake pending

qio_channel_tls_handshake_fail(void *ioc) "TLS handshake fail ioc=%p"

qio_channel_tls_handshake_complete(void *ioc) "TLS handshake complete ioc=%p"

qio_channel_tls_handshake_cancel(void *ioc) "TLS handshake cancel ioc=%p"

+qio_channel_tls_bye_start(void *ioc) "TLS termination start ioc=%p"

+qio_channel_tls_bye_pending(void *ioc, int status) "TLS termination pending ioc=%p status=%d"

+qio_channel_tls_bye_fail(void *ioc) "TLS termination fail ioc=%p"

+qio_channel_tls_bye_complete(void *ioc) "TLS termination complete ioc=%p"

+qio_channel_tls_bye_cancel(void *ioc) "TLS termination cancel ioc=%p"

qio_channel_tls_credentials_allow(void *ioc) "TLS credentials allow ioc=%p"

qio_channel_tls_credentials_deny(void *ioc) "TLS credentials deny ioc=%p"

--

2.35.3

The multifd recv side has been getting a TLS error of

GNUTLS_E_PREMATURE_TERMINATION at the end of migration when the send

side closes the sockets without ending the TLS session. This has been

masked by the code not checking the migration error after loadvm.

Start ending the TLS session at multifd_send_shutdown() so the recv

side always sees a clean termination (EOF) and we can start to

differentiate that from an actual premature termination that might

possibly happen in the middle of the migration.

There's nothing to be done if a previous migration error has already

broken the connection, so add a comment explaining it and ignore any

errors coming from gnutls_bye().

This doesn't break compat with older recv-side QEMUs because EOF has

always caused the recv thread to exit cleanly.

Signed-off-by: Fabiano Rosas <farosas@suse.de>

---

migration/multifd.c | 34 +++++++++++++++++++++++++++++++++-

migration/tls.c | 5 +++++

migration/tls.h | 2 +-

3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c

index XXXXXXX..XXXXXXX 100644

--- a/migration/multifd.c

+++ b/migration/multifd.c

@@ -XXX,XX +XXX,XX @@ void multifd_send_shutdown(void)

return;

}

+ for (i = 0; i < migrate_multifd_channels(); i++) {

+ MultiFDSendParams *p = &multifd_send_state->params[i];

+

+ /* thread_created implies the TLS handshake has succeeded */

+ if (p->tls_thread_created && p->thread_created) {

+ Error *local_err = NULL;

+ /*

+ * The destination expects the TLS session to always be

+ * properly terminated. This helps to detect a premature

+ * termination in the middle of the stream. Note that

+ * older QEMUs always break the connection on the source

+ * and the destination always sees

+ * GNUTLS_E_PREMATURE_TERMINATION.

+ */

+ migration_tls_channel_end(p->c, &local_err);

+

+ if (local_err) {

+ /*

+ * The above can fail with broken pipe due to a

+ * previous migration error, ignore the error.

+ */

+ assert(migration_has_failed(migrate_get_current()));

+ }

+ }

+ }

+

multifd_send_terminate_threads();

for (i = 0; i < migrate_multifd_channels(); i++) {

@@ -XXX,XX +XXX,XX @@ static void *multifd_recv_thread(void *opaque)

ret = qio_channel_read_all_eof(p->c, (void *)p->packet,

p->packet_len, &local_err);

- if (ret == 0 || ret == -1) { /* 0: EOF -1: Error */

+ if (!ret) {

+ /* EOF */

+ assert(!local_err);

+ break;

+ }

+

+ if (ret == -1) {

break;

}

diff --git a/migration/tls.c b/migration/tls.c

index XXXXXXX..XXXXXXX 100644

--- a/migration/tls.c

+++ b/migration/tls.c

@@ -XXX,XX +XXX,XX @@ void migration_tls_channel_connect(MigrationState *s,

NULL);

}

+void migration_tls_channel_end(QIOChannel *ioc, Error **errp)

+{

+ qio_channel_tls_bye(QIO_CHANNEL_TLS(ioc), errp);

+}

+

bool migrate_channel_requires_tls_upgrade(QIOChannel *ioc)

{

if (!migrate_tls()) {

diff --git a/migration/tls.h b/migration/tls.h

index XXXXXXX..XXXXXXX 100644

--- a/migration/tls.h

+++ b/migration/tls.h

@@ -XXX,XX +XXX,XX @@ void migration_tls_channel_connect(MigrationState *s,

QIOChannel *ioc,

const char *hostname,

Error **errp);

-

+void migration_tls_channel_end(QIOChannel *ioc, Error **errp);

/* Whether the QIO channel requires further TLS handshake? */

bool migrate_channel_requires_tls_upgrade(QIOChannel *ioc);

--

2.35.3

We're currently only checking the QEMUFile error after

qemu_loadvm_state(). Check the migration error as well to avoid

missing errors that might be set by the multifd recv thread.

This doesn't break compat between 9.2 and 10.0 because 9.2 still has

the multifd recv threads stuck at sync when the source channel shuts

down. I.e. it doesn't have commit 1d457daf86 ("migration/multifd:

Further remove the SYNC on complete"). QEMU versions with that commit

will have compat broken with versions containing this commit. This is

not an issue because both will be present in 10.0, but development

trees might see a migration error.

Signed-off-by: Fabiano Rosas <farosas@suse.de>

---

migration/savevm.c | 6 +++++-

1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/migration/savevm.c b/migration/savevm.c

index XXXXXXX..XXXXXXX 100644

--- a/migration/savevm.c

+++ b/migration/savevm.c

@@ -XXX,XX +XXX,XX @@ int qemu_loadvm_state(QEMUFile *f)

/* When reaching here, it must be precopy */

if (ret == 0) {

- ret = qemu_file_get_error(f);

+ if (migrate_has_error(migrate_get_current())) {

+ ret = -EINVAL;

+ } else {

+ ret = qemu_file_get_error(f);

+ }

}

/*

--

2.35.3