On Mon 18 Jan 2021 11:15:17 AM CET, Vladimir Sementsov-Ogievskiy wrote:
>> +static int bdrv_reopen_parse_file(BDRVReopenState *reopen_state,
>> + GSList **tran,
>> + Error **errp)
>> +{
>> + BlockDriverState *bs = reopen_state->bs;
>> + BlockDriverState *new_file_bs;
>> + QObject *value;
>> + const char *str;
>> +
>> + value = qdict_get(reopen_state->options, "file");
>> + if (value == NULL) {
>> + return 0;
>> + }
>> +
>> + /* The 'file' option only allows strings */
>> + assert(qobject_type(value) == QTYPE_QSTRING);
>> +
>> + str = qobject_get_try_str(value);
>> + new_file_bs = bdrv_lookup_bs(NULL, str, errp);
>> + if (new_file_bs == NULL) {
>> + return -EINVAL;
>> + } else if (bdrv_recurse_has_child(new_file_bs, bs)) {
>> + error_setg(errp, "Making '%s' a file of '%s' "
>> + "would create a cycle", str, bs->node_name);
>> + return -EINVAL;
>> + }
>> +
>> + assert(bs->file && bs->file->bs);
>
> why are we sure at this point? Probably, we should just return an
> error..
Unlike 'backing', 'file' is a BlockdevRef and it is not optional, so
block devices that accept that parameter must have it set.
>> + /* At the moment only backing links are frozen */
>> + assert(!bs->file->frozen);
>
> I think it can: file-child based filters can be a part of frozen
> backing chain currently.
You're right, since 7b99a26600e bdrv_freeze_backing_chain() uses
bdrv_filter_or_cow_child().
Berto
