[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/1] yank: Unregister function when using TLS migration
From: |
Peter Xu |
Subject: |
Re: [PATCH 1/1] yank: Unregister function when using TLS migration |
Date: |
Wed, 26 May 2021 17:58:58 -0400 |
On Wed, May 26, 2021 at 11:21:03PM +0200, Lukas Straub wrote:
> On Wed, 26 May 2021 16:40:35 -0400
> Peter Xu <peterx@redhat.com> wrote:
>
> > On Wed, May 26, 2021 at 05:05:40PM -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>
> >
> > Leo,
> >
> > Thanks for looking into it!
> >
> > So before looking int the fix... I do have a doubt on why we only enable
> > yank
> > on socket typed, as I think tls should also work with
> > qio_channel_shutdown().
> >
> > IIUC the confused thing here is we register only for qio-socket, however tls
> > will actually call migration_channel_connect() twice, first with a
> > qio-socket,
> > then with the real tls-socket. For tls I feel like we have registered with
> > the
> > wrong channel - instead of the wrapper socket ioc, we should register to the
> > final tls ioc?
> >
> > Lukas, is there a reason?
> >
>
> Hi,
> There is no specific reason. Both ways work equally well in preventing
> qemu from hanging. shutdown() for tls-channel just makes it abort a
> little sooner (by not attempting to encrypt and send data anymore).
>
> I don't lean either way. I guess registering it on the tls-channel
> makes is a bit more explicit and clearer.
Agreed, because IMHO logically the migration code should not be aware of
internals of IOChannels, e.g., we shouldn't need to know ioc->master is the
socket ioc of tls ioc to unregister.
--
Peter Xu
- [PATCH 1/1] yank: Unregister function when using TLS migration, Leonardo Bras, 2021/05/26
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Peter Xu, 2021/05/26
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Lukas Straub, 2021/05/26
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration,
Peter Xu <=
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Daniel P . Berrangé, 2021/05/27
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Peter Xu, 2021/05/27
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Daniel P . Berrangé, 2021/05/27
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Peter Xu, 2021/05/27
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Daniel P . Berrangé, 2021/05/27
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Dr. David Alan Gilbert, 2021/05/27
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Peter Xu, 2021/05/27
- Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Lukas Straub, 2021/05/27
Re: [PATCH 1/1] yank: Unregister function when using TLS migration, Lukas Straub, 2021/05/26