Re: [PATCH v3] nbd/server: do not poll within a coroutine context

[Eric Blake]

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