[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls |
Date: |
Tue, 7 Feb 2017 10:32:00 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/03/2017 09:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Split out nbd_receive_simple_option to be reused for structured reply
> option.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> nbd/client.c | 54
> +++++++++++++++++++++++++++++++++++-------------------
> nbd/nbd-internal.h | 14 ++++++++++++++
> 2 files changed, 49 insertions(+), 19 deletions(-)
>
> +++ b/nbd/nbd-internal.h
> @@ -96,6 +96,20 @@
> #define NBD_ENOSPC 28
> #define NBD_ESHUTDOWN 108
>
> +static inline const char *nbd_opt_name(int opt)
> +{
> + switch (opt) {
> + case NBD_OPT_EXPORT_NAME: return "export_name";
Does this really get past checkpatch?
> + case NBD_OPT_ABORT: return "abort";
> + case NBD_OPT_LIST: return "list";
> + case NBD_OPT_PEEK_EXPORT: return "peek_export";
> + case NBD_OPT_STARTTLS: return "tls";
Why just 'tls' instead of 'starttls'?
> + case NBD_OPT_STRUCTURED_REPLY: return "structured_reply";
> + }
> +
> + return "<unknown option>";
Can you please consider making this include the %d representation of the
unknown option; perhaps by snprintf'ing into static storage? While it
is unlikely that a well-behaved server will respond to a client with an
option the client doesn't recognize, it is much more likely that this
reverse lookup function will be used in a server to respond to an
unknown option from a client.
In fact, I might have split this into two patches: one providing
nbd_opt_name() and using it throughout the code base where appropriate,
and the other refactoring starttls in the client.
I'm not sure if the reverse lookup function needs to be inline in the
header; it could reasonably live in nbd/common.c, particularly if you
are going to take my advice to have it format a message for unknown values.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH 06/18] nbd/client: refactor drop_sync, (continued)
- [Qemu-block] [PATCH 18/18] nbd: BLOCK_STATUS for standard get_block_status function: client part, Vladimir Sementsov-Ogievskiy, 2017/02/03
- [Qemu-block] [PATCH 01/18] nbd: rename NBD_REPLY_MAGIC to NBD_SIMPLE_REPLY_MAGIC, Vladimir Sementsov-Ogievskiy, 2017/02/03
- [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Vladimir Sementsov-Ogievskiy, 2017/02/03
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls,
Eric Blake <=
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Vladimir Sementsov-Ogievskiy, 2017/02/09
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Eric Blake, 2017/02/09
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Vladimir Sementsov-Ogievskiy, 2017/02/10
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Eric Blake, 2017/02/13
- Re: [Qemu-block] [Qemu-devel] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Eric Blake, 2017/02/20
[Qemu-block] [PATCH 11/18] nbd: BLOCK_STATUS for bitmap export: server part, Vladimir Sementsov-Ogievskiy, 2017/02/03
[Qemu-block] [PATCH 07/18] nbd: Minimal structured read for client, Vladimir Sementsov-Ogievskiy, 2017/02/03