[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/1] yank: Unregister function when using TLS migration
From: |
Lukas Straub |
Subject: |
Re: [PATCH v3 1/1] yank: Unregister function when using TLS migration |
Date: |
Tue, 1 Jun 2021 17:32:33 +0200 |
On Tue, 1 Jun 2021 10:50:24 -0400
Peter Xu <peterx@redhat.com> wrote:
> On Tue, Jun 01, 2021 at 02:40:31AM -0300, Leonardo Bras wrote:
> > After yank feature was introduced in migration, 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().
> >
> > Also, inside migration_channel_connect() and
> > migration_channel_process_incoming() move yank_register_function() so
> > it only runs once on a TLS migration.
>
> Slightly inaccurate I guess, as it was run once too before this patch, but not
> paired for tls?
>
> IIUC when we call the 2nd time at below chunk we won't register again:
>
> if (object_dynamic_cast(OBJECT(ioc), TYPE_QIO_CHANNEL_SOCKET)) {
> yank_register_function(MIGRATION_YANK_INSTANCE,
> migration_yank_iochannel,
> QIO_CHANNEL(ioc));
> }
>
> Because the 2nd call will be TYPE_QIO_CHANNEL_TLS, so object_dynamic_cast()
> will return with a failure, I think (note, TYPE_QIO_CHANNEL_TLS's parent is
> TYPE_QIO_CHANNEL, not TYPE_QIO_CHANNEL_SOCKET).
>
> >
> > Fixes: b5eea99ec2f ("migration: Add yank feature", 2021-01-13)
> > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1964326
> > Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
> Thanks Leo!
>
> I have one pure question not directly related to Leo's patch (probably for
> Lukas?): we check OBJECT(ioc)->ref == 1 when unregister each function. In
> what
> case will the ref be not one?
>
If a return path is opened with qemu_file_get_return_path(), it will
take additional references:
qemu_file_get_return_path() (qemu-file.c)
f->ops->get_return_path() -> channel_get_input_return_path()
(qemu-file-channel.c)
qemu_fopen_channel_input() (qemu-file-channel.c)
object_ref(OBJECT(ioc))
Regards,
Lukas Straub
--
pgpjVSpy9aUAl.pgp
Description: OpenPGP digital signature