Re: [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

[Alberto Garcia <berto@igalia.com>]

On Tue 14 Nov 2017 04:09:16 PM CET, Max Reitz wrote:
>>> +static inline const char *qcow2_cache_get_name(BDRVQcow2State *s, Qcow2Cache *c)
>>> +{
>>> +    if (c == s->refcount_block_cache) {
>>> +        return "refcount block";
>>> +    } else if (c == s->l2_table_cache) {
>>> +        return "L2 table";
>>> +    } else {
>>> +        /* Do not abort, because this is not critical */
>>> +        return "unknown";
>>> +    }
>>> +}
>> 
>> Why is an unknown cache not critical?
>
> Because this is debugging information.

Ah, right, this is only used for qcow2_signal_corruption().

> I know others disagree with my opinion that I'd rather not abort qemu
> just because someone forgot to add a 'return "foo";' here when adding
> a new cache, but that's my opinion so I wanted to at least be told by
> someone that we should abort here before doing it.

I just checked the code and at least qcow2_cache_entry_flush() assumes
that there can be other caches, so this problem is not new.

Perhaps we should rethink this in the future and add a stronger
assertion somewhere else instead of having dead code, but for now this
is OK I guess.

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto