qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/1] yank: Unregister function when using TLS migration
@ 2021-05-26 22:16 Leonardo Bras
  2021-05-26 22:17 ` Leonardo Brás
  2021-05-27  1:33 ` Peter Xu
  0 siblings, 2 replies; 3+ messages in thread
From: Leonardo Bras @ 2021-05-26 22:16 UTC (permalink / raw)
  To: Juan Quintela, Dr. David Alan Gilbert, peterx, lukasstraub2
  Cc: Leonardo Bras, qemu-devel

After yank feature was introduced, whenever migration is started using TLS,
the following error happens in both source and destination hosts:

(qemu) qemu-kvm: ../util/yank.c:107: yank_unregister_instance:
Assertion `QLIST_EMPTY(&entry->yankfns)' failed.

This happens because of a missing yank_unregister_function() when using
qio-channel-tls.

Fix this by also allowing TYPE_QIO_CHANNEL_TLS object type to perform
yank_unregister_function() in channel_close() and multifd_load_cleanup().

Fixes: 50186051f ("Introduce yank feature")
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1964326
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>

--
Changes since v1:
- Cast p->c to QIOChannelTLS into multifd_load_cleanup()
---
 migration/multifd.c           | 6 ++++++
 migration/qemu-file-channel.c | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/migration/multifd.c b/migration/multifd.c
index 0a4803cfcc..81de11c6fc 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -992,6 +992,12 @@ int multifd_load_cleanup(Error **errp)
             yank_unregister_function(MIGRATION_YANK_INSTANCE,
                                      migration_yank_iochannel,
                                      QIO_CHANNEL(p->c));
+        } else if (object_dynamic_cast(OBJECT(p->c), TYPE_QIO_CHANNEL_TLS)
+                   && OBJECT(p->c)->ref == 1) {
+            QIOChannelTLS *tioc = QIO_CHANNEL_TLS(p->c);
+            yank_unregister_function(MIGRATION_YANK_INSTANCE,
+                                     migration_yank_iochannel,
+                                     QIO_CHANNEL(tioc->master));
         }
 
         object_unref(OBJECT(p->c));
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index 876d05a540..4f79090f3f 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -26,6 +26,7 @@
 #include "qemu-file-channel.h"
 #include "qemu-file.h"
 #include "io/channel-socket.h"
+#include "io/channel-tls.h"
 #include "qemu/iov.h"
 #include "qemu/yank.h"
 #include "yank_functions.h"
@@ -111,6 +112,12 @@ static int channel_close(void *opaque, Error **errp)
         yank_unregister_function(MIGRATION_YANK_INSTANCE,
                                  migration_yank_iochannel,
                                  QIO_CHANNEL(ioc));
+    } else if (object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS)
+               && OBJECT(ioc)->ref == 1) {
+        QIOChannelTLS *tioc = opaque;
+        yank_unregister_function(MIGRATION_YANK_INSTANCE,
+                                 migration_yank_iochannel,
+                                 QIO_CHANNEL(tioc->master));
     }
     object_unref(OBJECT(ioc));
     return ret;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] yank: Unregister function when using TLS migration
  2021-05-26 22:16 [PATCH v2 1/1] yank: Unregister function when using TLS migration Leonardo Bras
@ 2021-05-26 22:17 ` Leonardo Brás
  2021-05-27  1:33 ` Peter Xu
  1 sibling, 0 replies; 3+ messages in thread
From: Leonardo Brás @ 2021-05-26 22:17 UTC (permalink / raw)
  To: Juan Quintela, Dr. David Alan Gilbert, peterx, lukasstraub2; +Cc: qemu-devel

CC: Daniel P. Berrange <berrange@redhat.com>

On Wed, 2021-05-26 at 19:16 -0300, Leonardo Bras wrote:
> After yank feature was introduced, whenever migration is started using
> TLS,
> the following error happens in both source and destination hosts:
> 
> (qemu) qemu-kvm: ../util/yank.c:107: yank_unregister_instance:
> Assertion `QLIST_EMPTY(&entry->yankfns)' failed.
> 
> This happens because of a missing yank_unregister_function() when using
> qio-channel-tls.
> 
> Fix this by also allowing TYPE_QIO_CHANNEL_TLS object type to perform
> yank_unregister_function() in channel_close() and
> multifd_load_cleanup().
> 
> Fixes: 50186051f ("Introduce yank feature")
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1964326
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> 
> --
> Changes since v1:
> - Cast p->c to QIOChannelTLS into multifd_load_cleanup()
> ---
>  migration/multifd.c           | 6 ++++++
>  migration/qemu-file-channel.c | 7 +++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 0a4803cfcc..81de11c6fc 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -992,6 +992,12 @@ int multifd_load_cleanup(Error **errp)
>              yank_unregister_function(MIGRATION_YANK_INSTANCE,
>                                       migration_yank_iochannel,
>                                       QIO_CHANNEL(p->c));
> +        } else if (object_dynamic_cast(OBJECT(p->c),
> TYPE_QIO_CHANNEL_TLS)
> +                   && OBJECT(p->c)->ref == 1) {
> +            QIOChannelTLS *tioc = QIO_CHANNEL_TLS(p->c);
> +            yank_unregister_function(MIGRATION_YANK_INSTANCE,
> +                                     migration_yank_iochannel,
> +                                     QIO_CHANNEL(tioc->master));
>          }
>  
>          object_unref(OBJECT(p->c));
> diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-
> channel.c
> index 876d05a540..4f79090f3f 100644
> --- a/migration/qemu-file-channel.c
> +++ b/migration/qemu-file-channel.c
> @@ -26,6 +26,7 @@
>  #include "qemu-file-channel.h"
>  #include "qemu-file.h"
>  #include "io/channel-socket.h"
> +#include "io/channel-tls.h"
>  #include "qemu/iov.h"
>  #include "qemu/yank.h"
>  #include "yank_functions.h"
> @@ -111,6 +112,12 @@ static int channel_close(void *opaque, Error
> **errp)
>          yank_unregister_function(MIGRATION_YANK_INSTANCE,
>                                   migration_yank_iochannel,
>                                   QIO_CHANNEL(ioc));
> +    } else if (object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_TLS)
> +               && OBJECT(ioc)->ref == 1) {
> +        QIOChannelTLS *tioc = opaque;
> +        yank_unregister_function(MIGRATION_YANK_INSTANCE,
> +                                 migration_yank_iochannel,
> +                                 QIO_CHANNEL(tioc->master));
>      }
>      object_unref(OBJECT(ioc));
>      return ret;




^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] yank: Unregister function when using TLS migration
  2021-05-26 22:16 [PATCH v2 1/1] yank: Unregister function when using TLS migration Leonardo Bras
  2021-05-26 22:17 ` Leonardo Brás
@ 2021-05-27  1:33 ` Peter Xu
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Xu @ 2021-05-27  1:33 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: qemu-devel, lukasstraub2, Dr. David Alan Gilbert, Juan Quintela

On Wed, May 26, 2021 at 07:16:16PM -0300, Leonardo Bras wrote:
> After yank feature was introduced, whenever migration is started using TLS,
> the following error happens in both source and destination hosts:
> 
> (qemu) qemu-kvm: ../util/yank.c:107: yank_unregister_instance:
> Assertion `QLIST_EMPTY(&entry->yankfns)' failed.
> 
> This happens because of a missing yank_unregister_function() when using
> qio-channel-tls.
> 
> Fix this by also allowing TYPE_QIO_CHANNEL_TLS object type to perform
> yank_unregister_function() in channel_close() and multifd_load_cleanup().
> 
> Fixes: 50186051f ("Introduce yank feature")
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1964326
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>

Assuming you'll post a v3 soon trying to drop all references to ioc->master as
we've discussed. Meanwhile it should possibly be b5eea99ec2f ("migration: Add
yank feature", 2021-01-13) for the "fixes:"?

-- 
Peter Xu



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-27  1:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-26 22:16 [PATCH v2 1/1] yank: Unregister function when using TLS migration Leonardo Bras
2021-05-26 22:17 ` Leonardo Brás
2021-05-27  1:33 ` Peter Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).