15.01.2019 18:50, Eric Blake wrote:
> On 1/15/19 9:05 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 12.01.2019 20:58, Eric Blake wrote:
>>> Extract portions of nbd_negotiate_simple_meta_context() to
>>> a new function nbd_receive_one_meta_context() that copies the
>>> pattern of nbd_receive_list() for performing the argument
>>> validation of one reply. The error message when the server
>>> replies with more than one context changes slightly, but
>>> that shouldn't happen in the common case.
>>>
>>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>>> Message-Id: <20181215135324.152629-15-ebl...@redhat.com>
>>>
>>> ---
>>> v3: rebase, without changing into a loop
>>> ---
>
>>>
>>> +static int nbd_receive_one_meta_context(QIOChannel *ioc,
>>> + uint32_t opt,
>>> + char **name,
>>> + uint32_t *id,
>>> + Error **errp)
>>> +{
>
>>> + reply.length -= sizeof(local_id);
>>> + local_name = g_malloc(reply.length + 1);
>>> + if (nbd_read(ioc, local_name, reply.length, errp) < 0) {
>>> + g_free(local_name);
>>> + return -1;
>>> + }
>>> + local_name[reply.length] = '\0';
>>> + trace_nbd_opt_meta_reply(nbd_opt_lookup(opt), local_name, local_id);
>>> +
>
> [1]
>
>>> @@ -746,36 +796,20 @@ static int
>>> nbd_negotiate_simple_meta_context(QIOChannel *ioc,
>>> return -1;
>>> }
>>> g_free(name);
>>> -
>>> - trace_nbd_opt_meta_reply(context, info->context_id);
>>> received = true;
>>>
>>> - /* receive NBD_REP_ACK */
>>> - if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply,
>>> - errp) < 0)
>>> - {
>>> + ret = nbd_receive_one_meta_context(ioc, NBD_OPT_SET_META_CONTEXT,
>>> + &name, &info->context_id, errp);
>>
>> indent, and, no reasons to use variables instead of NULL, NULL, as success
>> here is an error
>> path anyway
>
> Not sure how I missed the indent, and good point about not needing &name
> or &info->context_id on this second call.
>
>>> +++ b/nbd/trace-events
>>> @@ -13,7 +13,7 @@ nbd_receive_query_exports_success(const char *wantname)
>>> "Found desired export na
>>> nbd_receive_starttls_new_client(void) "Setting up TLS"
>>> nbd_receive_starttls_tls_handshake(void) "Starting TLS handshake"
>>> nbd_opt_meta_request(const char *optname, const char *context, const
>>> char *export) "Requesting %s %s for export %s"
>>> -nbd_opt_meta_reply(const char *context, uint32_t id) "Received mapping of
>>> context %s to id %" PRIu32
>>> +nbd_opt_meta_reply(const char *optname, const char *context, uint32_t id)
>>> "Received %s mapping of %s to id %" PRIu32
>>
>> nbd_opt_lookup
>
> Added at [1], so I'm not sure what you are still wanting here.
Yes, my fault, it's OK.
>
>>
>>> nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving
>>> negotiation tlscreds=%p hostname=%s"
>>> nbd_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64
>>> nbd_receive_negotiate_server_flags(uint32_t globalflags) "Global flags
>>> are 0x%" PRIx32
>>>
>>
>> With at least nbd_opt_lookup:
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>>
>
--
Best regards,
Vladimir
