Re: [Qemu-block] [PATCH 3/9] nbd: BLOCK_STATUS for standard get_block_status function: server part

[Vladimir Sementsov-Ogievskiy]

16.02.2018 16:21, Eric Blake wrote:
> On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> > Minimal realization: only one extent in server answer is supported.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > ---

[...]

> > +        meta = &local_meta;
> > +    }
> > +
> > +    memset(meta, 0, sizeof(*meta));
> > +
> > +    ret = nbd_read_size_string(client, meta->export_name,
> > +                               NBD_MAX_NAME_SIZE, errp);
>
> Revisiting a question I raised in my first half review - you saved the 
> name as part of the struct because we have to later compare that the 
> final OPT_SET export name matches the request during OPT_GO (if they 
> don't match, then we have no contexts to serve after all).  So a 
> 'const char *' won't work, but maybe the struct could use a 'char *' 
> pointing to malloc'd storage rather than char[MAX_NAME] that reserves 
> array space that is mostly unused for the typical name that is much 
> shorter than the maximum name length.

Do these 256 bytes worth creating nbd_clear_meta function, to call it 
from client_put and on NBD_OPT_SET_META_CONTEXT, to clear previous meta? 
If yes, I'd prefer to postpone it to continuation about BLOCKSTATUS for 
dirty bitmaps, when I'll have to upgrade meta structure, to maintain 
dirty bitmaps.

> > +    if (ret <= 0) {
> > +        return ret;
> > +    }
> > +
> > +    exp = nbd_export_find(meta->export_name);
> > +    if (exp == NULL) {
> > +        return nbd_opt_invalid(client, errp,
> > +                               "export '%s' not present", 
> > meta->export_name);


--
Best regards,
Vladimir