Re: [Qemu-block] [PATCH v7 for-2.12 21/25] block: Purify .bdrv_refresh_filename()
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
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH v7 for-2.12 21/25] block: Purify .bdrv_refresh_filename()
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.
Max
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH v7 for-2.12 21/25] block: Purify .bdrv_refresh_filename()
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" ?
Berto
