diff mbox series

[v6,07/13] migration: Add helpers to detect TLS capability

Message ID 20220517195730.32312-8-peterx@redhat.com
State New
Headers show
Series migration: Postcopy Preemption | expand

Commit Message

Peter Xu May 17, 2022, 7:57 p.m. UTC 
Add migrate_channel_requires_tls() to detect whether the specific channel
requires TLS, leveraging the recently introduced migrate_use_tls().  No
functional change intended.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/channel.c   | 9 ++-------
 migration/migration.c | 1 +
 migration/multifd.c   | 4 +---
 migration/tls.c       | 9 +++++++++
 migration/tls.h       | 4 ++++
 5 files changed, 17 insertions(+), 10 deletions(-)

Comments

Daniel P. Berrangé May 18, 2022, 8:57 a.m. UTC | #1 
On Tue, May 17, 2022 at 03:57:24PM -0400, Peter Xu wrote:
> Add migrate_channel_requires_tls() to detect whether the specific channel
> requires TLS, leveraging the recently introduced migrate_use_tls().  No
> functional change intended.
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/channel.c   | 9 ++-------
>  migration/migration.c | 1 +
>  migration/multifd.c   | 4 +---
>  migration/tls.c       | 9 +++++++++
>  migration/tls.h       | 4 ++++
>  5 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/migration/channel.c b/migration/channel.c
> index a162d00fea..36e59eaeec 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -38,9 +38,7 @@ void migration_channel_process_incoming(QIOChannel *ioc)
>      trace_migration_set_incoming_channel(
>          ioc, object_get_typename(OBJECT(ioc)));
>  
> -    if (migrate_use_tls() &&
> -        !object_dynamic_cast(OBJECT(ioc),
> -                             TYPE_QIO_CHANNEL_TLS)) {
> +    if (migrate_channel_requires_tls(ioc)) {
>          migration_tls_channel_process_incoming(s, ioc, &local_err);
>      } else {
>          migration_ioc_register_yank(ioc);
> @@ -70,10 +68,7 @@ void migration_channel_connect(MigrationState *s,
>          ioc, object_get_typename(OBJECT(ioc)), hostname, error);
>  
>      if (!error) {
> -        if (s->parameters.tls_creds &&
> -            *s->parameters.tls_creds &&
> -            !object_dynamic_cast(OBJECT(ioc),
> -                                 TYPE_QIO_CHANNEL_TLS)) {
> +        if (migrate_channel_requires_tls(ioc)) {
>              migration_tls_channel_connect(s, ioc, hostname, &error);
>  
>              if (!error) {
> diff --git a/migration/migration.c b/migration/migration.c
> index f5f7a0f91f..d17f435d08 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -49,6 +49,7 @@
>  #include "trace.h"
>  #include "exec/target_page.h"
>  #include "io/channel-buffer.h"
> +#include "io/channel-tls.h"
>  #include "migration/colo.h"
>  #include "hw/boards.h"
>  #include "hw/qdev-properties.h"
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 9282ab6aa4..849c116ce4 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -831,9 +831,7 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
>          migrate_get_current()->hostname, error);
>  
>      if (!error) {
> -        if (migrate_use_tls() &&
> -            !object_dynamic_cast(OBJECT(ioc),
> -                                 TYPE_QIO_CHANNEL_TLS)) {
> +        if (migrate_channel_requires_tls(ioc)) {
>              multifd_tls_channel_connect(p, ioc, &error);
>              if (!error) {
>                  /*
> diff --git a/migration/tls.c b/migration/tls.c
> index 32c384a8b6..1baa662489 100644
> --- a/migration/tls.c
> +++ b/migration/tls.c
> @@ -166,3 +166,12 @@ void migration_tls_channel_connect(MigrationState *s,
>                                NULL,
>                                NULL);
>  }
> +
> +bool migrate_channel_requires_tls(QIOChannel *ioc)
> +{
> +    if (!migrate_use_tls()) {
> +        return false;
> +    }
> +
> +    return !object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS);
> +}
> diff --git a/migration/tls.h b/migration/tls.h
> index de4fe2cafd..a54c1dcec7 100644
> --- a/migration/tls.h
> +++ b/migration/tls.h
> @@ -37,4 +37,8 @@ void migration_tls_channel_connect(MigrationState *s,
>                                     QIOChannel *ioc,
>                                     const char *hostname,
>                                     Error **errp);
> +
> +/* Whether the QIO channel requires further TLS handshake? */
> +bool migrate_channel_requires_tls(QIOChannel *ioc);

I find this name somewhat confusing, as 'requires tls' and
'uses tls' are just synonyms for the same thing IMHO.

What this method is actually checking is whether we still need
to upgrade the channel from plain text to TLS, by completing a
TLS handshake. So can we call this:

  migrate_channel_requires_tls_upgrade

With regards,
Daniel
Peter Xu May 18, 2022, 1:04 p.m. UTC | #2 
On Wed, May 18, 2022 at 09:57:12AM +0100, Daniel P. Berrangé wrote:
> > @@ -37,4 +37,8 @@ void migration_tls_channel_connect(MigrationState *s,
> >                                     QIOChannel *ioc,
> >                                     const char *hostname,
> >                                     Error **errp);
> > +
> > +/* Whether the QIO channel requires further TLS handshake? */
> > +bool migrate_channel_requires_tls(QIOChannel *ioc);
> 
> I find this name somewhat confusing, as 'requires tls' and
> 'uses tls' are just synonyms for the same thing IMHO.
> 
> What this method is actually checking is whether we still need
> to upgrade the channel from plain text to TLS, by completing a
> TLS handshake. So can we call this:
> 
>   migrate_channel_requires_tls_upgrade

Sounds good.  I'll wait for more comments on other patches.  Thanks,
diff mbox series

Patch

diff --git a/migration/channel.c b/migration/channel.c
index a162d00fea..36e59eaeec 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -38,9 +38,7 @@  void migration_channel_process_incoming(QIOChannel *ioc)
     trace_migration_set_incoming_channel(
         ioc, object_get_typename(OBJECT(ioc)));
 
-    if (migrate_use_tls() &&
-        !object_dynamic_cast(OBJECT(ioc),
-                             TYPE_QIO_CHANNEL_TLS)) {
+    if (migrate_channel_requires_tls(ioc)) {
         migration_tls_channel_process_incoming(s, ioc, &local_err);
     } else {
         migration_ioc_register_yank(ioc);
@@ -70,10 +68,7 @@  void migration_channel_connect(MigrationState *s,
         ioc, object_get_typename(OBJECT(ioc)), hostname, error);
 
     if (!error) {
-        if (s->parameters.tls_creds &&
-            *s->parameters.tls_creds &&
-            !object_dynamic_cast(OBJECT(ioc),
-                                 TYPE_QIO_CHANNEL_TLS)) {
+        if (migrate_channel_requires_tls(ioc)) {
             migration_tls_channel_connect(s, ioc, hostname, &error);
 
             if (!error) {
diff --git a/migration/migration.c b/migration/migration.c
index f5f7a0f91f..d17f435d08 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -49,6 +49,7 @@ 
 #include "trace.h"
 #include "exec/target_page.h"
 #include "io/channel-buffer.h"
+#include "io/channel-tls.h"
 #include "migration/colo.h"
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
diff --git a/migration/multifd.c b/migration/multifd.c
index 9282ab6aa4..849c116ce4 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -831,9 +831,7 @@  static bool multifd_channel_connect(MultiFDSendParams *p,
         migrate_get_current()->hostname, error);
 
     if (!error) {
-        if (migrate_use_tls() &&
-            !object_dynamic_cast(OBJECT(ioc),
-                                 TYPE_QIO_CHANNEL_TLS)) {
+        if (migrate_channel_requires_tls(ioc)) {
             multifd_tls_channel_connect(p, ioc, &error);
             if (!error) {
                 /*
diff --git a/migration/tls.c b/migration/tls.c
index 32c384a8b6..1baa662489 100644
--- a/migration/tls.c
+++ b/migration/tls.c
@@ -166,3 +166,12 @@  void migration_tls_channel_connect(MigrationState *s,
                               NULL,
                               NULL);
 }
+
+bool migrate_channel_requires_tls(QIOChannel *ioc)
+{
+    if (!migrate_use_tls()) {
+        return false;
+    }
+
+    return !object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS);
+}
diff --git a/migration/tls.h b/migration/tls.h
index de4fe2cafd..a54c1dcec7 100644
--- a/migration/tls.h
+++ b/migration/tls.h
@@ -37,4 +37,8 @@  void migration_tls_channel_connect(MigrationState *s,
                                    QIOChannel *ioc,
                                    const char *hostname,
                                    Error **errp);
+
+/* Whether the QIO channel requires further TLS handshake? */
+bool migrate_channel_requires_tls(QIOChannel *ioc);
+
 #endif