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