[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3] nbd/server: do not poll within a coroutine context
From: |
Eric Blake |
Subject: |
Re: [PATCH v3] nbd/server: do not poll within a coroutine context |
Date: |
Fri, 5 Apr 2024 09:59:06 -0500 |
User-agent: |
NeoMutt/20240201 |
On Fri, Apr 05, 2024 at 05:10:16PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 04.04.24 04:42, Eric Blake wrote:
> > From: Zhu Yangyang <zhuyangyang14@huawei.com>
> >
> > Coroutines are not supposed to block. Instead, they should yield.
> >
> > The client performs TLS upgrade outside of an AIOContext, during
> > synchronous handshake; this still requires g_main_loop. But the
> > server responds to TLS upgrade inside a coroutine, so a nested
> > g_main_loop is wrong. Since the two callbacks no longer share more
> > than the setting of data.complete and data.error, it's just as easy to
> > use static helpers instead of trying to share a common code path.
> >
> > Fixes: f95910f ("nbd: implement TLS support in the protocol negotiation")
> > Signed-off-by: Zhu Yangyang <zhuyangyang14@huawei.com>
> > [eblake: move callbacks to their use point]
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > ---
> >
> > After looking at this more, I'm less convinced that there is enough
> > common code here to even be worth trying to share in common.c. This
> > takes the essence of the v2 patch, but refactors it a bit.
>
> Maybe, do the complete split, and make separate structure definitions in
> client.c and server.c, and don't make shared NBDTLSHandshakeData with union?
> Finally, it's just a simple opaque-structure for static callback function,
> seems good to keep it in .c as well.
Sure, v4 coming up along those lines.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org