On 05/30/2017 09:30 AM, Vladimir Sementsov-Ogievskiy wrote: > The code in many cases return -EINVAL or -EIO instead of original error > code from, for example, write_sync(). Following patch will need EPIPE > handling, so, let's refactor this where possible (the only exclusion > is nbd_co_receive_request, with own return-code convention)
Do we still want/need EPIPE handling, given the discussion on the
previous two patches?
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> ---
> nbd/server.c | 124
> +++++++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 77 insertions(+), 47 deletions(-)
>
Feels weird to have a net gain in code, but maybe worthwhile.
> diff --git a/nbd/server.c b/nbd/server.c
> index a47f13e4fb..30dfb81a5c 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -136,30 +136,38 @@ static void nbd_client_receive_next_request(NBDClient
> *client);
> static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
> uint32_t opt, uint32_t len)
> {
> + int ret;
> uint64_t magic;
>
> TRACE("Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32,
> type, opt, len);
>
> magic = cpu_to_be64(NBD_REP_MAGIC);
> - if (write_sync(ioc, &magic, sizeof(magic), NULL) < 0) {
> + ret = write_sync(ioc, &magic, sizeof(magic), NULL);
> + if (ret < 0) {
> LOG("write failed (rep magic)");
> - return -EINVAL;
> + return ret;
> }
Constructs like this should get shorter once we plumb errp all the way
through. Okay, I can live with the temporary verbosity.
You may still have to make changes due to rebasing (in which case I'll
definitely want to review again); but if this patch doesn't need further
rework, you can add:
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
