On 05/30/2017 09:30 AM, Vladimir Sementsov-Ogievskiy wrote:
Functions nbd_negotiate_{read,write,drop_sync} were introduced in
1a6245a5b, when nbd_wr_syncv was working through qemu_co_sendv_recvv,
There is no qemu_co_sendv_recvv. Did you mean qemu_co_recv/qemu_co_send?
which just yields, without setting any handlers. But now, nbd_wr_syncv
works through qio_channel_yield() which sets handlers, so watchers
are redundant in nbd_negotiate_{read,write,drop_sync}, then, let's just
use {read,write,drop}_sync functions.
Makes sense to me. Note that I have a pending patch that was also
related to 1a6245a5, where we introduced an assertion failure (which
later morphed into a segfault) if a client hangs up really early during
negotiation:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg06240.html
Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
+++ b/nbd/common.c
@@ -69,6 +69,32 @@ ssize_t nbd_wr_syncv(QIOChannel *ioc,
return done;
}
+/* Discard length bytes from channel. Return -errno on failure and 0 on
+ * success*/
Space before */, if you don't mind (yes, I know this was just code
motion, and that the old spot was wrong).
+int drop_sync(QIOChannel *ioc, size_t size, Error **errp)
As part of moving it into nbd/common.c, please rename this function to
an nbd_ prefix, since non-static functions are more likely to collide
with the rest of the code base if not properly named. Better yet: do it
in multiple patches:
- rename the static functions and fallout to callers (all of
nbd_drop_sync, nbd_read_sync, and nbd_write_sync)
- code motion (promote nbd_drop_sync from static in client.c to exported
in common.c)
- drop nbd_negotiate_ functions in favor of common nbd_*_sync functions