[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 11/19] nbd/client: Split out nbd_receive_one_
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 11/19] nbd/client: Split out nbd_receive_one_meta_context() |
Date: |
Tue, 15 Jan 2019 09:50:03 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
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 <address@hidden>
>> Message-Id: <address@hidden>
>>
>> ---
>> 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.
>
>> 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 <address@hidden>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature