qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v7 for-2.12 21/25] block: Purify .bdrv_refresh_f


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v7 for-2.12 21/25] block: Purify .bdrv_refresh_filename()
Date: Fri, 2 Feb 2018 18:41:46 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2

On 2017-12-04 19:25, Max Reitz wrote:
> On 2017-12-04 17:37, Alberto Garcia wrote:
>> On Mon 20 Nov 2017 09:10:00 PM CET, Max Reitz wrote:
>>> -static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
>>> +static void blkdebug_refresh_filename(BlockDriverState *bs)
>>>  {
>>>      BDRVBlkdebugState *s = bs->opaque;
>>> -    QDict *opts;
>>>      const QDictEntry *e;
>>> -    bool force_json = false;
>>> -
>>> -    for (e = qdict_first(options); e; e = qdict_next(options, e)) {
>>> -        if (strcmp(qdict_entry_key(e), "config") &&
>>> -            strcmp(qdict_entry_key(e), "x-image"))
>>> -        {
>>> -            force_json = true;
>>> -            break;
>>> -        }
>>> -    }
>>> +    int ret;
>>>  
>>> -    if (force_json && !bs->file->bs->full_open_options) {
>>> -        /* The config file cannot be recreated, so creating a plain 
>>> filename
>>> -         * is impossible */
>>> +    if (!bs->file->bs->exact_filename[0]) {
>>>          return;
>>>      }
>>>  
>>> -    if (!force_json && bs->file->bs->exact_filename[0]) {
>>> -        int ret = snprintf(bs->exact_filename, sizeof(bs->exact_filename),
>>> -                           "blkdebug:%s:%s", s->config_file ?: "",
>>> -                           bs->file->bs->exact_filename);
>>> -        if (ret >= sizeof(bs->exact_filename)) {
>>> -            /* An overflow makes the filename unusable, so do not report 
>>> any */
>>> -            bs->exact_filename[0] = 0;
>>> +    for (e = qdict_first(bs->full_open_options); e;
>>> +         e = qdict_next(bs->full_open_options, e))
>>> +    {
>>> +        if (strcmp(qdict_entry_key(e), "config") &&
>>> +            strcmp(qdict_entry_key(e), "image") &&
>>
>> Shouldn't this be "x-image" ?
> 
> Er, yes.  It should.

Actually, it should be both.  That's because the child is attached as
"image" and not "x-image", so when the child options are gathered, they
are put under "image".

And since the child is attached using bdrv_open_child(), you have to
specify all child options in an "image" sub qdict, too (as can be seen
in iotest 099), so this is indeed correct.  (Btw, note that the old code
already put these options under "image".)

(So with "x-image" instead of "image", iotest 162 fails.)

Of course, x-image can be specified, too (although I wouldn't really
mind breaking that for users...), so we have to ignore that, still.


Before this patch, we could ignore "image" because we iterated over the
options before they were newly generated.  Now they are generated
automatically before this function is called, so there may be an "image"
key now.

x-image

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]