qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 03/12] nbd/server: get rid of nbd_negotiate_read


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH 03/12] nbd/server: get rid of nbd_negotiate_read and friends
Date: Wed, 31 May 2017 13:50:32 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0

On 05/31/2017 11:55 AM, Vladimir Sementsov-Ogievskiy wrote:
> Functions nbd_negotiate_{read,write,drop_sync} were introduced in
> 1a6245a5b, when nbd_rwv (was nbd_wr_sync) was working through
> qemu_co_sendv_recvv (the path is nbd_wr_sync -> qemu_co_{recv/send} ->
> qemu_co_send_recv -> qemu_co_sendv_recvv), which just yields, without
> setting any handlers. But starting from ff82911cd nbd_rwv (was
> 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 nbd_{read,write,drop} functions.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  nbd/server.c | 107 
> ++++++++++++-----------------------------------------------
>  1 file changed, 22 insertions(+), 85 deletions(-)


>     Server         Client
> @@ -205,22 +142,22 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, 
> uint32_t type,
>            type, opt, len);
>  
>      magic = cpu_to_be64(NBD_REP_MAGIC);
> -    if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) < 0) {
> +    if (nbd_write(ioc, &magic, sizeof(magic), NULL) < 0) {
>          LOG("write failed (rep magic)");
>          return -EINVAL;

This says that you are ignoring errors (via errp) rather than reporting
them (because we LOG() it instead).  You do clean it up later, but it
would be nice to mention that in the commit message.

But because we have the later error fixups, I'm okay with:

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]