On Fri, Mar 24, 2023 at 07:20:17PM +0100, Florian Westphal wrote:
> Kevin Wolf <kw...@redhat.com> wrote:
> > Am 24.03.2023 um 11:47 hat Florian Westphal geschrieben:
> > > + qio_channel_set_cork(client->ioc, true);
> > > +
> > > if (ret < 0) {
> > > /* It wasn't -EIO, so, according to nbd_co_receive_request()
> > > * semantics, we should return the error to the client. */
> > > @@ -2692,6 +2694,7 @@ static coroutine_fn void nbd_trip(void *opaque)
> > > goto disconnect;
> > > }
> > >
> > > + qio_channel_set_cork(client->ioc, false);
> > > done:
> > > nbd_request_put(req);
> > > nbd_client_put(client);
> >
> > In the error paths, we never call set_cork(false) again. I suppose the
> > reason that this is okay is because the next thing is actually that we
> > close the socket?
>
> Yes, no need to uncork before close.
Uncorking may let the close be cleaner (especially in TLS, it is nicer
to let the connection send the necessary handshaking that the
connection is intentionally going down, rather than being abruptly
ripped out by a man-in-the-middle attacker), but NBD already has to
gracefully handle abrupt connection teardown, so we aren't losing
anything if the cork delays the close or if corked data that would
have given a clean shutdown is lost.
If we teach qio_channel to honor MSG_MORE as a way to do auto-corking
without explicit cork/uncork calls, that may take care of itself. But
that's a bigger project, while this patch seems safe enough as-is.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
