Re: [PATCH v2 3/5] nbd: Simplify meta-context parsing

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2209 bytes --]

On 10/7/20 6:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> 30.09.2020 15:11, Eric Blake wrote:
>> We had a premature optimization of trying to read as little from the
>> wire as possible while handling NBD_OPT_SET_META_CONTEXT in phases.
>> But in reality, we HAVE to read the entire string from the client
>> before we can get to the next command, and it is easier to just read
>> it all at once than it is to read it in pieces.  And once we do that,
>> several functions end up no longer performing I/O, so they can drop
>> length and errp parameters, and just return a bool instead of
>> modifying through a pointer.
>>
>> Our iotests still pass; I also checked that libnbd's testsuite (which
>> covers more corner cases of odd meta context requests) still passes.
>>
> 
> Also, do not advertise bitmaps meta context when bitmap export is not set.

That was already there, although seeing the logic change is tricky and
the trace messages change:

> +static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta,
> +                                const char *query)
>  {
> -    bool dirty_bitmap = false;
> -    size_t dirty_bitmap_len = strlen("dirty-bitmap:");
> -    int ret;
> -
> -    if (!meta->exp->export_bitmap) {
> -        trace_nbd_negotiate_meta_query_skip("no dirty-bitmap exported");
> -        return nbd_opt_skip(client, len, errp);

Old code returned early if there was no bitmap export set

> +    if (!nbd_strshift(&query, "qemu:")) {
> +        return false;
>      }
> +    trace_nbd_negotiate_meta_query_parse("qemu:");
> 
> -    if (len == 0) {
> +    if (!*query) {
>          if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
> +            meta->bitmap = !!meta->exp->export_bitmap;

while the new code has to handle it specifically.  I'll tweak the commit
message to mention the change in trace messages, even when the end
behavior is the same.


> 
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]