[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 10/19] nbd/client: Split out nbd_send_one_met
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH v3 10/19] nbd/client: Split out nbd_send_one_meta_context() |
Date: |
Wed, 16 Jan 2019 10:40:17 +0000 |
15.01.2019 18:44, Eric Blake wrote:
> On 1/15/19 7:18 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 12.01.2019 20:58, Eric Blake wrote:
>>> Refactor nbd_negotiate_simple_meta_context() to pull out the
>>> code that can be reused to send a LIST request for 0 or 1 query.
>>> No semantic change. The old comment about 'sizeof(uint32_t)'
>>> being equivalent to '/* number of queries */' is no longer
>>> needed, now that we are computing 'sizeof(queries)' instead.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>> Message-Id: <address@hidden>
>>> Reviewed-by: Richard W.M. Jones <address@hidden>
>>>
>>> ---
>>> v3: Improve commit message [Rich], formatting tweak [checkpatch],
>>> rebase to dropped patch
>>> ---
>
>>> +++ b/nbd/client.c
>>> @@ -629,6 +629,49 @@ static QIOChannel *nbd_receive_starttls(QIOChannel
>>> *ioc,
>>> return QIO_CHANNEL(tioc);
>>> }
>>>
>>> +/*
>>> + * nbd_send_one_meta_context:
>>> + * Send 0 or 1 set/list meta context queries.
>>> + * Return 0 on success, -1 with errp set for any error
>>> + */
>>> +static int nbd_send_one_meta_context(QIOChannel *ioc,
>>> + uint32_t opt,
>>> + const char *export,
>>> + const char *query,
>>> + Error **errp)
>>> +{
>>> + int ret;
>>> + uint32_t export_len = strlen(export);
>>> + uint32_t queries = !!query;
>>
>> n_ or nb_ prefix may make it more clear
>>
>>> + uint32_t context_len = 0;
>>> + uint32_t data_len;
>>> + char *data;
>>> + char *p;
>>> +
>>> + data_len = sizeof(export_len) + export_len + sizeof(queries);
>
> [1]
>
>>> + if (query) {
>>> + context_len = strlen(query);
>>
>> looks like it then should be query_len
>
> Sure, an alternative name may make things easier to read (I think this
> is somewhat fallout from my rebase churn, where earlier versions of the
> patch shared as much code with NBD_OPT_SET_META_CONTEXT, and that code
> used the name 'context' rather than 'query'; but now that I've split
> things to add a new function, it doesn't have to maintain the old naming).
and if you are going to fix it, note, that function name may be fixed too, as we
don't send context, we send query. Especially in the case when query=NULL,
we send empty query, but what is empty context?
--
Best regards,
Vladimir
[Qemu-block] [PATCH v3 17/19] qemu-nbd: Add --list option, Eric Blake, 2019/01/12
[Qemu-block] [PATCH v3 16/19] nbd/client: Add meta contexts to nbd_receive_export_list(), Eric Blake, 2019/01/12
[Qemu-block] [PATCH v3 13/19] nbd/client: Split handshake into two functions, Eric Blake, 2019/01/12
[Qemu-block] [PATCH v3 09/19] nbd/client: Change signature of nbd_negotiate_simple_meta_context(), Eric Blake, 2019/01/12
[Qemu-block] [PATCH v3 15/19] nbd/client: Add nbd_receive_export_list(), Eric Blake, 2019/01/12