[RFC][PATCH v1 0/7] *** Add Multifd support for TLS migration ***

[RFC][PATCH v1 0/7] *** Add Multifd support for TLS migration ***

[Chuan Zheng]

TLS migration could easily reach bottleneck of cpu because of encryption
and decryption in migration thread.
In our test, the tls migration could only reach 300MB/s under bandwidth
of 500MB/s.

Inspired by multifd, we add multifd support for tls migration to make fully
use of given net bandwidth at the cost of multi-cpus and could reduce
at most of 100% migration time with 4U16G test vm.

Evaluate migration time of migration vm.
The VM specifications for migration are as follows:
- VM use 4-K page;
- the number of VCPU is 4;
- the total memory is 16Gigabit;
- use 'mempress' tool to pressurize VM(mempress 4096 100);
- migration flag is 73755 (8219 + 65536 (TLS)) vs 204827 (8219 + 65536 (TLS) + 131072(Multifd))

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
|                      |         TLS           |      MultiFD + TLS (2 channel)    |
--------------------------------------------------------t---------------------------
| mempress 1024 120    |       25.035s         |           15.067s                 |
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
| mempress 1024 200    |       48.798s         |           25.334s                 |
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
| mempress 1024 300    |   Migration Failed    |           25.617s                 |
------------------------------------------------------------------------------------

Chuan Zheng (7):
  migration/tls: save hostname into MigrationState
  migration/tls: extract migration_tls_client_create for common-use
  migration/tls: add MigrationState into MultiFDSendParams
  migration/tls: extract cleanup function for common-use
  migration/tls: add support for tls check
  migration/tls: add support for multifd tls-handshake
  migration/tls: add trace points for multifd-tls

 migration/channel.c    |   5 ++
 migration/migration.c  |   1 +
 migration/migration.h  |   5 ++
 migration/multifd.c    | 121 +++++++++++++++++++++++++++++++++++++++++++------
 migration/multifd.h    |   2 +
 migration/tls.c        |  26 +++++++----
 migration/tls.h        |   6 +++
 migration/trace-events |   5 ++
 8 files changed, 149 insertions(+), 22 deletions(-)

-- 
1.8.3.1

[PATCH v1 1/7] migration/tls: save hostname into MigrationState

[Chuan Zheng]

hostname is need in multifd-tls, save hostname into MigrationState

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Yan Jin <jinyan12@huawei.com>
---
 migration/channel.c   | 5 +++++
 migration/migration.c | 1 +
 migration/migration.h | 5 +++++
 3 files changed, 11 insertions(+)

diff --git a/migration/channel.c b/migration/channel.c
index 20e4c8e..2af3069 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -66,6 +66,11 @@ void migration_channel_connect(MigrationState *s,
     trace_migration_set_outgoing_channel(
         ioc, object_get_typename(OBJECT(ioc)), hostname, error);
 
+    /* Save hostname into MigrationState for handshake */
+    if (hostname) {
+        s->hostname = g_strdup(hostname);
+    }
+
     if (!error) {
         if (s->parameters.tls_creds &&
             *s->parameters.tls_creds &&
diff --git a/migration/migration.c b/migration/migration.c
index 58a5452..e20b778 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1883,6 +1883,7 @@ void migrate_init(MigrationState *s)
     s->migration_thread_running = false;
     error_free(s->error);
     s->error = NULL;
+    s->hostname = NULL;
 
     migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
 
diff --git a/migration/migration.h b/migration/migration.h
index ae497bd..758f803 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -261,6 +261,11 @@ struct MigrationState
      * (which is in 4M chunk).
      */
     uint8_t clear_bitmap_shift;
+
+    /*
+     * This save hostname when out-going migration starts
+     */
+    char *hostname;
 };
 
 void migrate_set_state(int *state, int old_state, int new_state);
-- 
1.8.3.1

[PATCH v1 2/7] migration/tls: extract migration_tls_client_create for common-use

[Chuan Zheng]

migration_tls_client_create will be used in multifd-tls, let's
extract it.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Yan Jin <jinyan12@huawei.com>
---
 migration/tls.c | 26 ++++++++++++++++++--------
 migration/tls.h |  6 ++++++
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/migration/tls.c b/migration/tls.c
index 7a02ec8..e888698 100644
--- a/migration/tls.c
+++ b/migration/tls.c
@@ -22,7 +22,6 @@
 #include "channel.h"
 #include "migration.h"
 #include "tls.h"
-#include "io/channel-tls.h"
 #include "crypto/tlscreds.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
@@ -125,11 +124,10 @@ static void migration_tls_outgoing_handshake(QIOTask *task,
     object_unref(OBJECT(ioc));
 }
 
-
-void migration_tls_channel_connect(MigrationState *s,
-                                   QIOChannel *ioc,
-                                   const char *hostname,
-                                   Error **errp)
+QIOChannelTLS *migration_tls_client_create(MigrationState *s,
+                                 QIOChannel *ioc,
+                                 const char *hostname,
+                                 Error **errp)
 {
     QCryptoTLSCreds *creds;
     QIOChannelTLS *tioc;
@@ -137,7 +135,7 @@ void migration_tls_channel_connect(MigrationState *s,
     creds = migration_tls_get_creds(
         s, QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, errp);
     if (!creds) {
-        return;
+        return NULL;
     }
 
     if (s->parameters.tls_hostname && *s->parameters.tls_hostname) {
@@ -145,11 +143,23 @@ void migration_tls_channel_connect(MigrationState *s,
     }
     if (!hostname) {
         error_setg(errp, "No hostname available for TLS");
-        return;
+        return NULL;
     }
 
     tioc = qio_channel_tls_new_client(
         ioc, creds, hostname, errp);
+
+    return tioc;
+}
+
+void migration_tls_channel_connect(MigrationState *s,
+                                   QIOChannel *ioc,
+                                   const char *hostname,
+                                   Error **errp)
+{
+    QIOChannelTLS *tioc;
+
+    tioc = migration_tls_client_create(s, ioc, hostname, errp);
     if (!tioc) {
         return;
     }
diff --git a/migration/tls.h b/migration/tls.h
index cdd7000..d4a0861 100644
--- a/migration/tls.h
+++ b/migration/tls.h
@@ -22,11 +22,17 @@
 #define QEMU_MIGRATION_TLS_H
 
 #include "io/channel.h"
+#include "io/channel-tls.h"
 
 void migration_tls_channel_process_incoming(MigrationState *s,
                                             QIOChannel *ioc,
                                             Error **errp);
 
+QIOChannelTLS *migration_tls_client_create(MigrationState *s,
+                                   QIOChannel *ioc,
+                                   const char *hostname,
+                                   Error **errp);
+
 void migration_tls_channel_connect(MigrationState *s,
                                    QIOChannel *ioc,
                                    const char *hostname,
-- 
1.8.3.1

[PATCH v1 3/7] migration/tls: add MigrationState into MultiFDSendParams

[Chuan Zheng]

MigrationState is need for tls session build, add MigrationState
into MultiFDSendParams.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Yan Jin <jinyan12@huawei.com>
---
 migration/multifd.c | 2 ++
 migration/multifd.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index d044120..2e04803 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -543,6 +543,7 @@ void multifd_save_cleanup(void)
 
         socket_send_channel_destroy(p->c);
         p->c = NULL;
+        p->s = NULL;
         qemu_mutex_destroy(&p->mutex);
         qemu_sem_destroy(&p->sem);
         qemu_sem_destroy(&p->sem_sync);
@@ -738,6 +739,7 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
         object_unref(OBJECT(sioc));
         error_free(local_err);
     } else {
+        p->s = migrate_get_current();
         p->c = QIO_CHANNEL(sioc);
         qio_channel_set_delay(p->c, false);
         p->running = true;
diff --git a/migration/multifd.h b/migration/multifd.h
index 448a03d..8175b3c 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -66,6 +66,8 @@ typedef struct {
 } MultiFDPages_t;
 
 typedef struct {
+    /* Migration State */
+    MigrationState *s;
     /* this fields are not changed once the thread is created */
     /* channel number */
     uint8_t id;
-- 
1.8.3.1

[PATCH v1 4/7] migration/tls: extract cleanup function for common-use

[Chuan Zheng]

multifd channel cleanup is need  if multifd handshake failed,
let's extract it.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Yan Jin <jinyan12@huawei.com>
---
 migration/multifd.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 2e04803..b2e741c 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -718,6 +718,23 @@ out:
     return NULL;
 }
 
+static void multifd_new_send_channel_cleanup(MultiFDSendParams *p,
+                                             QIOChannel *ioc, Error *err)
+{
+     migrate_set_error(migrate_get_current(), err);
+     /* Error happen, we need to tell who pay attention to me */
+     qemu_sem_post(&multifd_send_state->channels_ready);
+     qemu_sem_post(&p->sem_sync);
+     /*
+      * Although multifd_send_thread is not created, but main migration
+      * thread neet to judge whether it is running, so we need to mark
+      * its status.
+      */
+     p->quit = true;
+     object_unref(OBJECT(ioc));
+     error_free(err);
+}
+
 static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
 {
     MultiFDSendParams *p = opaque;
@@ -726,18 +743,7 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
 
     trace_multifd_new_send_channel_async(p->id);
     if (qio_task_propagate_error(task, &local_err)) {
-        migrate_set_error(migrate_get_current(), local_err);
-        /* Error happen, we need to tell who pay attention to me */
-        qemu_sem_post(&multifd_send_state->channels_ready);
-        qemu_sem_post(&p->sem_sync);
-        /*
-         * Although multifd_send_thread is not created, but main migration
-         * thread neet to judge whether it is running, so we need to mark
-         * its status.
-         */
-        p->quit = true;
-        object_unref(OBJECT(sioc));
-        error_free(local_err);
+        goto cleanup;
     } else {
         p->s = migrate_get_current();
         p->c = QIO_CHANNEL(sioc);
@@ -745,7 +751,11 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
         p->running = true;
         qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
                            QEMU_THREAD_JOINABLE);
+        return;
     }
+
+cleanup:
+    multifd_new_send_channel_cleanup(p, sioc, local_err);
 }
 
 int multifd_save_setup(Error **errp)
-- 
1.8.3.1

[PATCH v1 5/7] migration/tls: add support for tls check

[Chuan Zheng]

add multifd_channel_connect to support for tls check.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Yan Jin <jinyan12@huawei.com>
---
 migration/multifd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index b2e741c..b2076d7 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -20,6 +20,7 @@
 #include "ram.h"
 #include "migration.h"
 #include "socket.h"
+#include "tls.h"
 #include "qemu-file.h"
 #include "trace.h"
 #include "multifd.h"
@@ -718,6 +719,47 @@ out:
     return NULL;
 }
 
+static void multifd_tls_channel_connect(MultiFDSendParams *p,
+                                    QIOChannel *ioc,
+                                    Error **errp)
+{
+    /* TODO */
+}
+
+static bool multifd_channel_connect(MultiFDSendParams *p,
+                                    QIOChannel *ioc,
+                                    Error *error)
+{
+    MigrationState *s = p->s;
+
+    if (!error) {
+        if (s->parameters.tls_creds &&
+            *s->parameters.tls_creds &&
+            !object_dynamic_cast(OBJECT(ioc),
+                                 TYPE_QIO_CHANNEL_TLS)) {
+            multifd_tls_channel_connect(p, ioc, &error);
+            if (!error) {
+                /*
+                 * tls_channel_connect will call back to this
+                 * function after the TLS handshake,
+                 * so we mustn't call multifd_send_thread until then
+                 */
+                return false;
+            } else {
+                return true;
+            }
+        } else {
+            /* update for tls qio channel */
+            p->c = ioc;
+            qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
+                                   QEMU_THREAD_JOINABLE);
+       }
+       return false;
+    }
+
+    return true;
+}
+
 static void multifd_new_send_channel_cleanup(MultiFDSendParams *p,
                                              QIOChannel *ioc, Error *err)
 {
@@ -749,8 +791,9 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
         p->c = QIO_CHANNEL(sioc);
         qio_channel_set_delay(p->c, false);
         p->running = true;
-        qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
-                           QEMU_THREAD_JOINABLE);
+        if (multifd_channel_connect(p, sioc, local_err)) {
+            goto cleanup;
+        }
         return;
     }
 
-- 
1.8.3.1

[PATCH v1 6/7] migration/tls: add support for multifd tls-handshake

[Chuan Zheng]

add support for multifd tls-handshake

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Yan Jin <jinyan12@huawei.com>
---
 migration/multifd.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index b2076d7..2509187 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -719,11 +719,41 @@ out:
     return NULL;
 }
 
+static bool multifd_channel_connect(MultiFDSendParams *p,
+                                    QIOChannel *ioc,
+                                    Error *error);
+
+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;
+
+    qio_task_propagate_error(task, &err);
+    multifd_channel_connect(p, ioc, err);
+}
+
 static void multifd_tls_channel_connect(MultiFDSendParams *p,
                                     QIOChannel *ioc,
                                     Error **errp)
 {
-    /* TODO */
+    MigrationState *s = p->s;
+    const char *hostname = s->hostname;
+    QIOChannelTLS *tioc;
+
+    tioc = migration_tls_client_create(s, ioc, hostname, errp);
+    if (!tioc) {
+        return;
+    }
+
+    qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
+    qio_channel_tls_handshake(tioc,
+                              multifd_tls_outgoing_handshake,
+                              p,
+                              NULL,
+                              NULL);
+
 }
 
 static bool multifd_channel_connect(MultiFDSendParams *p,
-- 
1.8.3.1

[PATCH v1 7/7] migration/tls: add trace points for multifd-tls

[Chuan Zheng]

add trace points for multifd-tls for debug.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Yan Jin <jinyan12@huawei.com>
---
 migration/multifd.c    | 10 +++++++++-
 migration/trace-events |  5 +++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 2509187..26935b6 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -730,7 +730,11 @@ static void multifd_tls_outgoing_handshake(QIOTask *task,
     QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
     Error *err = NULL;
 
-    qio_task_propagate_error(task, &err);
+    if (qio_task_propagate_error(task, &err)) {
+        trace_multifd_tls_outgoing_handshake_error(error_get_pretty(err));
+    } else {
+        trace_multifd_tls_outgoing_handshake_complete();
+    }
     multifd_channel_connect(p, ioc, err);
 }
 
@@ -747,6 +751,7 @@ static void multifd_tls_channel_connect(MultiFDSendParams *p,
         return;
     }
 
+    trace_multifd_tls_outgoing_handshake_start(hostname);
     qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
     qio_channel_tls_handshake(tioc,
                               multifd_tls_outgoing_handshake,
@@ -762,6 +767,9 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
 {
     MigrationState *s = p->s;
 
+    trace_multifd_set_outgoing_channel(
+        ioc, object_get_typename(OBJECT(ioc)), s->hostname, error);
+
     if (!error) {
         if (s->parameters.tls_creds &&
             *s->parameters.tls_creds &&
diff --git a/migration/trace-events b/migration/trace-events
index 4ab0a50..860d2c4 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -109,6 +109,11 @@ multifd_send_sync_main_wait(uint8_t id) "channel %d"
 multifd_send_terminate_threads(bool error) "error %d"
 multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t pages) "channel %d packets %" PRIu64 " pages %"  PRIu64
 multifd_send_thread_start(uint8_t id) "%d"
+multifd_tls_outgoing_handshake_start(const char *hostname) "hostname=%s"
+multifd_tls_outgoing_handshake_error(const char *err) "err=%s"
+multifd_tls_outgoing_handshake_complete(void) ""
+multifd_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname, void *err)  "ioc=%p ioctype=%s hostname=%s err=%p"
+
 ram_discard_range(const char *rbname, uint64_t start, size_t len) "%s: start: %" PRIx64 " %zx"
 ram_load_loop(const char *rbname, uint64_t addr, int flags, void *host) "%s: addr: 0x%" PRIx64 " flags: 0x%x host: %p"
 ram_load_postcopy_loop(uint64_t addr, int flags) "@%" PRIx64 " %x"
-- 
1.8.3.1

Re: [PATCH v1 1/7] migration/tls: save hostname into MigrationState

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 10:52:51PM +0800, Chuan Zheng wrote:
> hostname is need in multifd-tls, save hostname into MigrationState
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: Yan Jin <jinyan12@huawei.com>
> ---
>  migration/channel.c   | 5 +++++
>  migration/migration.c | 1 +
>  migration/migration.h | 5 +++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/migration/channel.c b/migration/channel.c
> index 20e4c8e..2af3069 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -66,6 +66,11 @@ void migration_channel_connect(MigrationState *s,
>      trace_migration_set_outgoing_channel(
>          ioc, object_get_typename(OBJECT(ioc)), hostname, error);
>  
> +    /* Save hostname into MigrationState for handshake */
> +    if (hostname) {
> +        s->hostname = g_strdup(hostname);
> +    }
> +
>      if (!error) {
>          if (s->parameters.tls_creds &&
>              *s->parameters.tls_creds &&
> diff --git a/migration/migration.c b/migration/migration.c
> index 58a5452..e20b778 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1883,6 +1883,7 @@ void migrate_init(MigrationState *s)
>      s->migration_thread_running = false;
>      error_free(s->error);
>      s->error = NULL;
> +    s->hostname = NULL;
>  
>      migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
>  
> diff --git a/migration/migration.h b/migration/migration.h
> index ae497bd..758f803 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -261,6 +261,11 @@ struct MigrationState
>       * (which is in 4M chunk).
>       */
>      uint8_t clear_bitmap_shift;
> +
> +    /*
> +     * This save hostname when out-going migration starts
> +     */
> +    char *hostname;
>  };

Something needs to free(hostname) at the appropriate time, otherwise
well have a memory leak if we run migration multiple times.


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 :|

Re: [PATCH v1 2/7] migration/tls: extract migration_tls_client_create for common-use

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 10:52:52PM +0800, Chuan Zheng wrote:
> migration_tls_client_create will be used in multifd-tls, let's
> extract it.
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: Yan Jin <jinyan12@huawei.com>
> ---
>  migration/tls.c | 26 ++++++++++++++++++--------
>  migration/tls.h |  6 ++++++
>  2 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/migration/tls.c b/migration/tls.c
> index 7a02ec8..e888698 100644
> --- a/migration/tls.c
> +++ b/migration/tls.c
> @@ -22,7 +22,6 @@
>  #include "channel.h"
>  #include "migration.h"
>  #include "tls.h"
> -#include "io/channel-tls.h"
>  #include "crypto/tlscreds.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> @@ -125,11 +124,10 @@ static void migration_tls_outgoing_handshake(QIOTask *task,
>      object_unref(OBJECT(ioc));
>  }
>  
> -
> -void migration_tls_channel_connect(MigrationState *s,
> -                                   QIOChannel *ioc,
> -                                   const char *hostname,
> -                                   Error **errp)
> +QIOChannelTLS *migration_tls_client_create(MigrationState *s,
> +                                 QIOChannel *ioc,
> +                                 const char *hostname,
> +                                 Error **errp)

Alignment of parameters is messed up.

>  {
>      QCryptoTLSCreds *creds;
>      QIOChannelTLS *tioc;
> @@ -137,7 +135,7 @@ void migration_tls_channel_connect(MigrationState *s,
>      creds = migration_tls_get_creds(
>          s, QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, errp);
>      if (!creds) {
> -        return;
> +        return NULL;
>      }
>  
>      if (s->parameters.tls_hostname && *s->parameters.tls_hostname) {
> @@ -145,11 +143,23 @@ void migration_tls_channel_connect(MigrationState *s,
>      }
>      if (!hostname) {
>          error_setg(errp, "No hostname available for TLS");
> -        return;
> +        return NULL;
>      }
>  
>      tioc = qio_channel_tls_new_client(
>          ioc, creds, hostname, errp);
> +
> +    return tioc;
> +}
> +
> +void migration_tls_channel_connect(MigrationState *s,
> +                                   QIOChannel *ioc,
> +                                   const char *hostname,
> +                                   Error **errp)
> +{
> +    QIOChannelTLS *tioc;
> +
> +    tioc = migration_tls_client_create(s, ioc, hostname, errp);
>      if (!tioc) {
>          return;
>      }
> diff --git a/migration/tls.h b/migration/tls.h
> index cdd7000..d4a0861 100644
> --- a/migration/tls.h
> +++ b/migration/tls.h
> @@ -22,11 +22,17 @@
>  #define QEMU_MIGRATION_TLS_H
>  
>  #include "io/channel.h"
> +#include "io/channel-tls.h"
>  
>  void migration_tls_channel_process_incoming(MigrationState *s,
>                                              QIOChannel *ioc,
>                                              Error **errp);
>  
> +QIOChannelTLS *migration_tls_client_create(MigrationState *s,
> +                                   QIOChannel *ioc,
> +                                   const char *hostname,
> +                                   Error **errp);

Again alignment is messed up.

Assuming that's fixed

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



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 :|

Re: [PATCH v1 4/7] migration/tls: extract cleanup function for common-use

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 10:52:54PM +0800, Chuan Zheng wrote:
> multifd channel cleanup is need  if multifd handshake failed,
> let's extract it.
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: Yan Jin <jinyan12@huawei.com>
> ---
>  migration/multifd.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

Re: [PATCH v1 6/7] migration/tls: add support for multifd tls-handshake

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 10:52:56PM +0800, Chuan Zheng wrote:
> add support for multifd tls-handshake
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: Yan Jin <jinyan12@huawei.com>
> ---
>  migration/multifd.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index b2076d7..2509187 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -719,11 +719,41 @@ out:
>      return NULL;
>  }
>  
> +static bool multifd_channel_connect(MultiFDSendParams *p,
> +                                    QIOChannel *ioc,
> +                                    Error *error);
> +
> +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;
> +
> +    qio_task_propagate_error(task, &err);
> +    multifd_channel_connect(p, ioc, err);
> +}
> +
>  static void multifd_tls_channel_connect(MultiFDSendParams *p,
>                                      QIOChannel *ioc,
>                                      Error **errp)
>  {
> -    /* TODO */
> +    MigrationState *s = p->s;
> +    const char *hostname = s->hostname;
> +    QIOChannelTLS *tioc;
> +
> +    tioc = migration_tls_client_create(s, ioc, hostname, errp);
> +    if (!tioc) {
> +        return;
> +    }
> +
> +    qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
> +    qio_channel_tls_handshake(tioc,
> +                              multifd_tls_outgoing_handshake,
> +                              p,
> +                              NULL,
> +                              NULL);
> +
>  }


Please squash this back into the previous patch, and both are
inter-dependant on each other, and thus don't make sense to split

Assuming it is squashed in

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



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 :|

Re: [PATCH v1 5/7] migration/tls: add support for tls check

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 10:52:55PM +0800, Chuan Zheng wrote:
> add multifd_channel_connect to support for tls check.
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: Yan Jin <jinyan12@huawei.com>
> ---
>  migration/multifd.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index b2e741c..b2076d7 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -20,6 +20,7 @@
>  #include "ram.h"
>  #include "migration.h"
>  #include "socket.h"
> +#include "tls.h"
>  #include "qemu-file.h"
>  #include "trace.h"
>  #include "multifd.h"
> @@ -718,6 +719,47 @@ out:
>      return NULL;
>  }
>  
> +static void multifd_tls_channel_connect(MultiFDSendParams *p,
> +                                    QIOChannel *ioc,
> +                                    Error **errp)
> +{
> +    /* TODO */
> +}
> +
> +static bool multifd_channel_connect(MultiFDSendParams *p,
> +                                    QIOChannel *ioc,
> +                                    Error *error)
> +{
> +    MigrationState *s = p->s;
> +
> +    if (!error) {
> +        if (s->parameters.tls_creds &&
> +            *s->parameters.tls_creds &&
> +            !object_dynamic_cast(OBJECT(ioc),
> +                                 TYPE_QIO_CHANNEL_TLS)) {
> +            multifd_tls_channel_connect(p, ioc, &error);
> +            if (!error) {
> +                /*
> +                 * tls_channel_connect will call back to this
> +                 * function after the TLS handshake,
> +                 * so we mustn't call multifd_send_thread until then
> +                 */
> +                return false;
> +            } else {
> +                return true;
> +            }
> +        } else {
> +            /* update for tls qio channel */
> +            p->c = ioc;
> +            qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
> +                                   QEMU_THREAD_JOINABLE);
> +       }
> +       return false;
> +    }
> +
> +    return true;
> +}
> +
>  static void multifd_new_send_channel_cleanup(MultiFDSendParams *p,
>                                               QIOChannel *ioc, Error *err)
>  {
> @@ -749,8 +791,9 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
>          p->c = QIO_CHANNEL(sioc);
>          qio_channel_set_delay(p->c, false);
>          p->running = true;
> -        qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
> -                           QEMU_THREAD_JOINABLE);
> +        if (multifd_channel_connect(p, sioc, local_err)) {
> +            goto cleanup;
> +        }
>          return;
>      }

If this is squashed with the following patch then

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

Re: [PATCH v1 7/7] migration/tls: add trace points for multifd-tls

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 10:52:57PM +0800, Chuan Zheng wrote:
> add trace points for multifd-tls for debug.
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: Yan Jin <jinyan12@huawei.com>
> ---
>  migration/multifd.c    | 10 +++++++++-
>  migration/trace-events |  5 +++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 2509187..26935b6 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -730,7 +730,11 @@ static void multifd_tls_outgoing_handshake(QIOTask *task,
>      QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
>      Error *err = NULL;
>  
> -    qio_task_propagate_error(task, &err);
> +    if (qio_task_propagate_error(task, &err)) {
> +        trace_multifd_tls_outgoing_handshake_error(error_get_pretty(err));
> +    } else {
> +        trace_multifd_tls_outgoing_handshake_complete();
> +    }
>      multifd_channel_connect(p, ioc, err);
>  }
>  
> @@ -747,6 +751,7 @@ static void multifd_tls_channel_connect(MultiFDSendParams *p,
>          return;
>      }
>  
> +    trace_multifd_tls_outgoing_handshake_start(hostname);
>      qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
>      qio_channel_tls_handshake(tioc,
>                                multifd_tls_outgoing_handshake,
> @@ -762,6 +767,9 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
>  {
>      MigrationState *s = p->s;
>  
> +    trace_multifd_set_outgoing_channel(
> +        ioc, object_get_typename(OBJECT(ioc)), s->hostname, error);
> +
>      if (!error) {
>          if (s->parameters.tls_creds &&
>              *s->parameters.tls_creds &&
> diff --git a/migration/trace-events b/migration/trace-events
> index 4ab0a50..860d2c4 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -109,6 +109,11 @@ multifd_send_sync_main_wait(uint8_t id) "channel %d"
>  multifd_send_terminate_threads(bool error) "error %d"
>  multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t pages) "channel %d packets %" PRIu64 " pages %"  PRIu64
>  multifd_send_thread_start(uint8_t id) "%d"
> +multifd_tls_outgoing_handshake_start(const char *hostname) "hostname=%s"
> +multifd_tls_outgoing_handshake_error(const char *err) "err=%s"
> +multifd_tls_outgoing_handshake_complete(void) ""

I'd suggest adding 'void *ioc' for all of these to make it clearer to
correlate the traces.

> +multifd_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname, void *err)  "ioc=%p ioctype=%s hostname=%s err=%p"
> +
>  ram_discard_range(const char *rbname, uint64_t start, size_t len) "%s: start: %" PRIx64 " %zx"
>  ram_load_loop(const char *rbname, uint64_t addr, int flags, void *host) "%s: addr: 0x%" PRIx64 " flags: 0x%x host: %p"
>  ram_load_postcopy_loop(uint64_t addr, int flags) "@%" PRIx64 " %x"
> -- 
> 1.8.3.1
> 

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 :|

Re: [PATCH v1 3/7] migration/tls: add MigrationState into MultiFDSendParams

[Daniel P. Berrangé]

On Wed, Sep 09, 2020 at 10:52:53PM +0800, Chuan Zheng wrote:
> MigrationState is need for tls session build, add MigrationState
> into MultiFDSendParams.
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: Yan Jin <jinyan12@huawei.com>
> ---
>  migration/multifd.c | 2 ++
>  migration/multifd.h | 2 ++
>  2 files changed, 4 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

Re: [PATCH v1 1/7] migration/tls: save hostname into MigrationState

[Zheng Chuan]

On 2020/9/10 21:11, Daniel P. Berrangé wrote:
> On Wed, Sep 09, 2020 at 10:52:51PM +0800, Chuan Zheng wrote:
>> hostname is need in multifd-tls, save hostname into MigrationState
>>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> Signed-off-by: Yan Jin <jinyan12@huawei.com>
>> ---
>>  migration/channel.c   | 5 +++++
>>  migration/migration.c | 1 +
>>  migration/migration.h | 5 +++++
>>  3 files changed, 11 insertions(+)
>>
>> diff --git a/migration/channel.c b/migration/channel.c
>> index 20e4c8e..2af3069 100644
>> --- a/migration/channel.c
>> +++ b/migration/channel.c
>> @@ -66,6 +66,11 @@ void migration_channel_connect(MigrationState *s,
>>      trace_migration_set_outgoing_channel(
>>          ioc, object_get_typename(OBJECT(ioc)), hostname, error);
>>  
>> +    /* Save hostname into MigrationState for handshake */
>> +    if (hostname) {
>> +        s->hostname = g_strdup(hostname);
>> +    }
>> +
>>      if (!error) {
>>          if (s->parameters.tls_creds &&
>>              *s->parameters.tls_creds &&
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 58a5452..e20b778 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1883,6 +1883,7 @@ void migrate_init(MigrationState *s)
>>      s->migration_thread_running = false;
>>      error_free(s->error);
>>      s->error = NULL;
>> +    s->hostname = NULL;
>>  
>>      migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
>>  
>> diff --git a/migration/migration.h b/migration/migration.h
>> index ae497bd..758f803 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -261,6 +261,11 @@ struct MigrationState
>>       * (which is in 4M chunk).
>>       */
>>      uint8_t clear_bitmap_shift;
>> +
>> +    /*
>> +     * This save hostname when out-going migration starts
>> +     */
>> +    char *hostname;
>>  };
> 
> Something needs to free(hostname) at the appropriate time, otherwise
> well have a memory leak if we run migration multiple times.
> 
Hi, Daniel
Thank you for your review.

Yes, it will have a memory leak. Maybe i could just assign incoming parameters
when it is not NULL other than g_strdup it.

However, i am doubt if it is the best idea to save hostname into current_migration:(


> 
> Regards,
> Daniel
> 

Re: [PATCH v1 2/7] migration/tls: extract migration_tls_client_create for common-use

[Zheng Chuan]

On 2020/9/10 21:16, Daniel P. Berrangé wrote:
> On Wed, Sep 09, 2020 at 10:52:52PM +0800, Chuan Zheng wrote:
>> migration_tls_client_create will be used in multifd-tls, let's
>> extract it.
>>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> Signed-off-by: Yan Jin <jinyan12@huawei.com>
>> ---
>>  migration/tls.c | 26 ++++++++++++++++++--------
>>  migration/tls.h |  6 ++++++
>>  2 files changed, 24 insertions(+), 8 deletions(-)
>>
>> diff --git a/migration/tls.c b/migration/tls.c
>> index 7a02ec8..e888698 100644
>> --- a/migration/tls.c
>> +++ b/migration/tls.c
>> @@ -22,7 +22,6 @@
>>  #include "channel.h"
>>  #include "migration.h"
>>  #include "tls.h"
>> -#include "io/channel-tls.h"
>>  #include "crypto/tlscreds.h"
>>  #include "qemu/error-report.h"
>>  #include "qapi/error.h"
>> @@ -125,11 +124,10 @@ static void migration_tls_outgoing_handshake(QIOTask *task,
>>      object_unref(OBJECT(ioc));
>>  }
>>  
>> -
>> -void migration_tls_channel_connect(MigrationState *s,
>> -                                   QIOChannel *ioc,
>> -                                   const char *hostname,
>> -                                   Error **errp)
>> +QIOChannelTLS *migration_tls_client_create(MigrationState *s,
>> +                                 QIOChannel *ioc,
>> +                                 const char *hostname,
>> +                                 Error **errp)
> 
> Alignment of parameters is messed up.
> 
Sure, will fix in v2
>>  {
>>      QCryptoTLSCreds *creds;
>>      QIOChannelTLS *tioc;
>> @@ -137,7 +135,7 @@ void migration_tls_channel_connect(MigrationState *s,
>>      creds = migration_tls_get_creds(
>>          s, QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT, errp);
>>      if (!creds) {
>> -        return;
>> +        return NULL;
>>      }
>>  
>>      if (s->parameters.tls_hostname && *s->parameters.tls_hostname) {
>> @@ -145,11 +143,23 @@ void migration_tls_channel_connect(MigrationState *s,
>>      }
>>      if (!hostname) {
>>          error_setg(errp, "No hostname available for TLS");
>> -        return;
>> +        return NULL;
>>      }
>>  
>>      tioc = qio_channel_tls_new_client(
>>          ioc, creds, hostname, errp);
>> +
>> +    return tioc;
>> +}
>> +
>> +void migration_tls_channel_connect(MigrationState *s,
>> +                                   QIOChannel *ioc,
>> +                                   const char *hostname,
>> +                                   Error **errp)
>> +{
>> +    QIOChannelTLS *tioc;
>> +
>> +    tioc = migration_tls_client_create(s, ioc, hostname, errp);
>>      if (!tioc) {
>>          return;
>>      }
>> diff --git a/migration/tls.h b/migration/tls.h
>> index cdd7000..d4a0861 100644
>> --- a/migration/tls.h
>> +++ b/migration/tls.h
>> @@ -22,11 +22,17 @@
>>  #define QEMU_MIGRATION_TLS_H
>>  
>>  #include "io/channel.h"
>> +#include "io/channel-tls.h"
>>  
>>  void migration_tls_channel_process_incoming(MigrationState *s,
>>                                              QIOChannel *ioc,
>>                                              Error **errp);
>>  
>> +QIOChannelTLS *migration_tls_client_create(MigrationState *s,
>> +                                   QIOChannel *ioc,
>> +                                   const char *hostname,
>> +                                   Error **errp);
> 
> Again alignment is messed up.
> 
> Assuming that's fixed
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> 
> 
> Regards,
> Daniel
> 

Re: [PATCH v1 6/7] migration/tls: add support for multifd tls-handshake

[Zheng Chuan]

On 2020/9/10 21:25, Daniel P. Berrangé wrote:
> On Wed, Sep 09, 2020 at 10:52:56PM +0800, Chuan Zheng wrote:
>> add support for multifd tls-handshake
>>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> Signed-off-by: Yan Jin <jinyan12@huawei.com>
>> ---
>>  migration/multifd.c | 32 +++++++++++++++++++++++++++++++-
>>  1 file changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index b2076d7..2509187 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -719,11 +719,41 @@ out:
>>      return NULL;
>>  }
>>  
>> +static bool multifd_channel_connect(MultiFDSendParams *p,
>> +                                    QIOChannel *ioc,
>> +                                    Error *error);
>> +
>> +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;
>> +
>> +    qio_task_propagate_error(task, &err);
>> +    multifd_channel_connect(p, ioc, err);
>> +}
>> +
>>  static void multifd_tls_channel_connect(MultiFDSendParams *p,
>>                                      QIOChannel *ioc,
>>                                      Error **errp)
>>  {
>> -    /* TODO */
>> +    MigrationState *s = p->s;
>> +    const char *hostname = s->hostname;
>> +    QIOChannelTLS *tioc;
>> +
>> +    tioc = migration_tls_client_create(s, ioc, hostname, errp);
>> +    if (!tioc) {
>> +        return;
>> +    }
>> +
>> +    qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
>> +    qio_channel_tls_handshake(tioc,
>> +                              multifd_tls_outgoing_handshake,
>> +                              p,
>> +                              NULL,
>> +                              NULL);
>> +
>>  }
> 
> 
> Please squash this back into the previous patch, and both are
> inter-dependant on each other, and thus don't make sense to split
> 
> Assuming it is squashed in
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
OK, will squash it in v2
> 
> 
> Regards,
> Daniel
> 

Re: [PATCH v1 7/7] migration/tls: add trace points for multifd-tls

[Zheng Chuan]

On 2020/9/10 21:37, Daniel P. Berrangé wrote:
> On Wed, Sep 09, 2020 at 10:52:57PM +0800, Chuan Zheng wrote:
>> add trace points for multifd-tls for debug.
>>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> Signed-off-by: Yan Jin <jinyan12@huawei.com>
>> ---
>>  migration/multifd.c    | 10 +++++++++-
>>  migration/trace-events |  5 +++++
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 2509187..26935b6 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -730,7 +730,11 @@ static void multifd_tls_outgoing_handshake(QIOTask *task,
>>      QIOChannel *ioc = QIO_CHANNEL(qio_task_get_source(task));
>>      Error *err = NULL;
>>  
>> -    qio_task_propagate_error(task, &err);
>> +    if (qio_task_propagate_error(task, &err)) {
>> +        trace_multifd_tls_outgoing_handshake_error(error_get_pretty(err));
>> +    } else {
>> +        trace_multifd_tls_outgoing_handshake_complete();
>> +    }
>>      multifd_channel_connect(p, ioc, err);
>>  }
>>  
>> @@ -747,6 +751,7 @@ static void multifd_tls_channel_connect(MultiFDSendParams *p,
>>          return;
>>      }
>>  
>> +    trace_multifd_tls_outgoing_handshake_start(hostname);
>>      qio_channel_set_name(QIO_CHANNEL(tioc), "multifd-tls-outgoing");
>>      qio_channel_tls_handshake(tioc,
>>                                multifd_tls_outgoing_handshake,
>> @@ -762,6 +767,9 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
>>  {
>>      MigrationState *s = p->s;
>>  
>> +    trace_multifd_set_outgoing_channel(
>> +        ioc, object_get_typename(OBJECT(ioc)), s->hostname, error);
>> +
>>      if (!error) {
>>          if (s->parameters.tls_creds &&
>>              *s->parameters.tls_creds &&
>> diff --git a/migration/trace-events b/migration/trace-events
>> index 4ab0a50..860d2c4 100644
>> --- a/migration/trace-events
>> +++ b/migration/trace-events
>> @@ -109,6 +109,11 @@ multifd_send_sync_main_wait(uint8_t id) "channel %d"
>>  multifd_send_terminate_threads(bool error) "error %d"
>>  multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t pages) "channel %d packets %" PRIu64 " pages %"  PRIu64
>>  multifd_send_thread_start(uint8_t id) "%d"
>> +multifd_tls_outgoing_handshake_start(const char *hostname) "hostname=%s"
>> +multifd_tls_outgoing_handshake_error(const char *err) "err=%s"
>> +multifd_tls_outgoing_handshake_complete(void) ""
> 
> I'd suggest adding 'void *ioc' for all of these to make it clearer to
> correlate the traces.
> 
OK, will add it in v2

>> +multifd_set_outgoing_channel(void *ioc, const char *ioctype, const char *hostname, void *err)  "ioc=%p ioctype=%s hostname=%s err=%p"
>> +
>>  ram_discard_range(const char *rbname, uint64_t start, size_t len) "%s: start: %" PRIx64 " %zx"
>>  ram_load_loop(const char *rbname, uint64_t addr, int flags, void *host) "%s: addr: 0x%" PRIx64 " flags: 0x%x host: %p"
>>  ram_load_postcopy_loop(uint64_t addr, int flags) "@%" PRIx64 " %x"
>> -- 
>> 1.8.3.1
>>
> 
> Regards,
> Daniel
>