Add migrate_tls_enabled() to detect whether TLS is configured.
Add migrate_channel_requires_tls() to detect whether the specific channel
requires TLS.
No functional change intended.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/channel.c | 10 ++--------
migration/migration.c | 17 +++++++++++++++++
migration/migration.h | 4 ++++
migration/multifd.c | 7 +------
4 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/migration/channel.c b/migration/channel.c
index c6a8dcf1d7..36e59eaeec 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -38,10 +38,7 @@ void migration_channel_process_incoming(QIOChannel *ioc)
trace_migration_set_incoming_channel(
ioc, object_get_typename(OBJECT(ioc)));
- 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_process_incoming(s, ioc, &local_err);
} else {
migration_ioc_register_yank(ioc);
@@ -71,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 ee3df9e229..899084f993 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"
@@ -4251,6 +4252,22 @@ void migration_global_dump(Monitor *mon)
ms->clear_bitmap_shift);
}
+bool migrate_tls_enabled(void)
+{
+ MigrationState *s = migrate_get_current();
+
+ return s->parameters.tls_creds && *s->parameters.tls_creds;
+}
+
+bool migrate_channel_requires_tls(QIOChannel *ioc)
+{
+ if (!migrate_tls_enabled()) {
+ return false;
+ }
+
+ return !object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS);
+}
+
#define DEFINE_PROP_MIG_CAP(name, x) \
DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
diff --git a/migration/migration.h b/migration/migration.h
index 6ee520642f..8b9ad7fe31 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -436,6 +436,10 @@ bool migrate_use_events(void);
bool migrate_postcopy_blocktime(void);
bool migrate_background_snapshot(void);
bool migrate_postcopy_preempt(void);
+/* Whether TLS is enabled for migration? */
+bool migrate_tls_enabled(void);
+/* Whether the QIO channel requires further TLS handshake? */
+bool migrate_channel_requires_tls(QIOChannel *ioc);
/* Sending on the return path - generic and then for each message type */
void migrate_send_rp_shut(MigrationIncomingState *mis,
diff --git a/migration/multifd.c b/migration/multifd.c
index 9ea4f581e2..19e3c44491 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -782,17 +782,12 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
QIOChannel *ioc,
Error *error)
{
- MigrationState *s = migrate_get_current();
-
trace_multifd_set_outgoing_channel(
ioc, object_get_typename(OBJECT(ioc)),
migrate_get_current()->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)) {
multifd_tls_channel_connect(p, ioc, &error);
if (!error) {
/*
--
2.32.0On Thu, Mar 31, 2022 at 11:08:52AM -0400, Peter Xu wrote:
> Add migrate_tls_enabled() to detect whether TLS is configured.
>
> Add migrate_channel_requires_tls() to detect whether the specific channel
> requires TLS.
>
> No functional change intended.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/channel.c | 10 ++--------
> migration/migration.c | 17 +++++++++++++++++
> migration/migration.h | 4 ++++
> migration/multifd.c | 7 +------
> 4 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/migration/channel.c b/migration/channel.c
> index c6a8dcf1d7..36e59eaeec 100644
> --- a/migration/channel.c
> +++ b/migration/channel.c
> @@ -38,10 +38,7 @@ void migration_channel_process_incoming(QIOChannel *ioc)
> trace_migration_set_incoming_channel(
> ioc, object_get_typename(OBJECT(ioc)));
>
> - 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_process_incoming(s, ioc, &local_err);
> } else {
> migration_ioc_register_yank(ioc);
> @@ -71,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 ee3df9e229..899084f993 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"
> @@ -4251,6 +4252,22 @@ void migration_global_dump(Monitor *mon)
> ms->clear_bitmap_shift);
> }
>
> +bool migrate_tls_enabled(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + return s->parameters.tls_creds && *s->parameters.tls_creds;
> +}
> +
> +bool migrate_channel_requires_tls(QIOChannel *ioc)
> +{
> + if (!migrate_tls_enabled()) {
This is the only place migrate_tls_enabled is called. Does it
really need to exist as an exported method, as opposed to
inlining it here ?
> + return false;
> + }
> +
> + return !object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS);
> +}
> +
> #define DEFINE_PROP_MIG_CAP(name, x) \
> DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
>
> diff --git a/migration/migration.h b/migration/migration.h
> index 6ee520642f..8b9ad7fe31 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -436,6 +436,10 @@ bool migrate_use_events(void);
> bool migrate_postcopy_blocktime(void);
> bool migrate_background_snapshot(void);
> bool migrate_postcopy_preempt(void);
> +/* Whether TLS is enabled for migration? */
> +bool migrate_tls_enabled(void);
> +/* Whether the QIO channel requires further TLS handshake? */
> +bool migrate_channel_requires_tls(QIOChannel *ioc);
How about having it in tls.{c,h} as 'migration_tls_channel_enabled()' ?
>
> /* Sending on the return path - generic and then for each message type */
> void migrate_send_rp_shut(MigrationIncomingState *mis,
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 9ea4f581e2..19e3c44491 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -782,17 +782,12 @@ static bool multifd_channel_connect(MultiFDSendParams *p,
> QIOChannel *ioc,
> Error *error)
> {
> - MigrationState *s = migrate_get_current();
> -
> trace_multifd_set_outgoing_channel(
> ioc, object_get_typename(OBJECT(ioc)),
> migrate_get_current()->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)) {
> multifd_tls_channel_connect(p, ioc, &error);
> if (!error) {
> /*
> --
> 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 :|
On Wed, Apr 20, 2022 at 12:10:14PM +0100, Daniel P. Berrangé wrote:
> On Thu, Mar 31, 2022 at 11:08:52AM -0400, Peter Xu wrote:
> > Add migrate_tls_enabled() to detect whether TLS is configured.
> >
> > Add migrate_channel_requires_tls() to detect whether the specific channel
> > requires TLS.
> >
> > No functional change intended.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > migration/channel.c | 10 ++--------
> > migration/migration.c | 17 +++++++++++++++++
> > migration/migration.h | 4 ++++
> > migration/multifd.c | 7 +------
> > 4 files changed, 24 insertions(+), 14 deletions(-)
> >
> > diff --git a/migration/channel.c b/migration/channel.c
> > index c6a8dcf1d7..36e59eaeec 100644
> > --- a/migration/channel.c
> > +++ b/migration/channel.c
> > @@ -38,10 +38,7 @@ void migration_channel_process_incoming(QIOChannel *ioc)
> > trace_migration_set_incoming_channel(
> > ioc, object_get_typename(OBJECT(ioc)));
> >
> > - 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_process_incoming(s, ioc, &local_err);
> > } else {
> > migration_ioc_register_yank(ioc);
> > @@ -71,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 ee3df9e229..899084f993 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"
> > @@ -4251,6 +4252,22 @@ void migration_global_dump(Monitor *mon)
> > ms->clear_bitmap_shift);
> > }
> >
> > +bool migrate_tls_enabled(void)
> > +{
> > + MigrationState *s = migrate_get_current();
> > +
> > + return s->parameters.tls_creds && *s->parameters.tls_creds;
> > +}
> > +
> > +bool migrate_channel_requires_tls(QIOChannel *ioc)
> > +{
> > + if (!migrate_tls_enabled()) {
>
> This is the only place migrate_tls_enabled is called. Does it
> really need to exist as an exported method, as opposed to
> inlining it here ?
IMHO the helper could help code readers to easier understand when TLS is
enabled, and it's not super obvious as TLS doesn't have a capability bit
bound to it. No strong opinions, though.
>
> > + return false;
> > + }
> > +
> > + return !object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS);
> > +}
> > +
> > #define DEFINE_PROP_MIG_CAP(name, x) \
> > DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
> >
> > diff --git a/migration/migration.h b/migration/migration.h
> > index 6ee520642f..8b9ad7fe31 100644
> > --- a/migration/migration.h
> > +++ b/migration/migration.h
> > @@ -436,6 +436,10 @@ bool migrate_use_events(void);
> > bool migrate_postcopy_blocktime(void);
> > bool migrate_background_snapshot(void);
> > bool migrate_postcopy_preempt(void);
> > +/* Whether TLS is enabled for migration? */
> > +bool migrate_tls_enabled(void);
> > +/* Whether the QIO channel requires further TLS handshake? */
> > +bool migrate_channel_requires_tls(QIOChannel *ioc);
>
> How about having it in tls.{c,h} as 'migration_tls_channel_enabled()' ?
I can do the movement, but the new name can be confusing when we read it in
the codes, it'll look like:
if (migration_tls_channel_enabled(ioc)) {
/* create the tls channel */
...
}
The thing is migration_tls_channel_enabled() on a TLS channel will return
false.. which seems to be against the gut feelings.
migrate_channel_requires_tls() feels better but maybe not so much..
Would migrate_channel_requires_tls_wrapper() be better (but longer..)?
Thanks,
--
Peter Xu
© 2016 - 2026 Red Hat, Inc.