[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 06/11] nbd: Minimal structured read for serve
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v5 06/11] nbd: Minimal structured read for server |
Date: |
Sat, 21 Oct 2017 11:02:15 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 10/20/2017 02:30 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>> + } else if (client->structured_reply) {
>>>> + ret = nbd_negotiate_send_rep_err(
>>>> + client->ioc, NBD_REP_ERR_INVALID, option,
>>>> errp,
>>>> + "structured reply already negotiated");
>>>> + } else {
>>>> + ret = nbd_negotiate_send_rep(client->ioc,
>>>> NBD_REP_ACK,
>>>> + option, errp);
>>>> + }
>>> you've dropped "if (ret < 0) { return ret }", but the following two
>>> lines should not be
>>> executed if ret < 0.. May be it doesn't matter (we will abort connection
>>> anyway after switch {}) but
>>> it looks strange.
>>>
>>>> + client->structured_reply = true;
>>>> + myflags |= NBD_FLAG_SEND_DF;
>>>> + break;
>> Indeed, these two lines are harmless due to the catch-all 'ret < 0'
>> after the switch at the tail end of the loop (which is why I dropped the
>> 'if' here).
>>
>>
>
> yes. but it looks strange) Anyway,
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>
On second thought, modifying client->structured_reply when we
successfully replied with an error is wrong. So v6 will hoist the
modifications into the last else branch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v5 09/11] nbd/client: prepare nbd_receive_reply for structured reply, (continued)
- [Qemu-devel] [PATCH v5 09/11] nbd/client: prepare nbd_receive_reply for structured reply, Eric Blake, 2017/10/19
- [Qemu-devel] [PATCH v5 03/11] nbd: Expose constants and structs for structured read, Eric Blake, 2017/10/19
- [Qemu-devel] [PATCH v5 10/11] nbd: Move nbd_read() to common header, Eric Blake, 2017/10/19
- [Qemu-devel] [PATCH v5 07/11] nbd/server: Include human-readable message in structured errors, Eric Blake, 2017/10/19
- [Qemu-devel] [PATCH v5 06/11] nbd: Minimal structured read for server, Eric Blake, 2017/10/19
[Qemu-devel] [PATCH v5 11/11] nbd: Minimal structured read for client, Eric Blake, 2017/10/19
Re: [Qemu-devel] [PATCH v5 00/11] nbd minimal structured read, no-reply, 2017/10/19