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 <be...@igalia.com> Berto