From ec64b1ca08138b5f57d270a5756a9138f897ee5a Mon Sep 17 00:00:00 2001 From: Hanna Reitz Date: Wed, 3 Aug 2022 16:44:44 +0200 Subject: [PATCH] block/qcow2: Keep auto_backing_file if possible qcow2_do_open() is used by qcow2_co_invalidate_cache(), i.e. may be run on an image that has been opened before. When reading the backing file string from the image header, compare it against the existing bs->backing_file, and update bs->auto_backing_file only if they differ. auto_backing_file should ideally contain the filename the backing BDS will actually have after opening, i.e. a post-bdrv_refresh_filename() version of what is in the image header. So for example, if the image header reports the following backing file string: json:{"driver": "qcow2", "file": { "driver": "file", "filename": "/tmp/backing.qcow2" }} Then auto_backing_file should contain simply "/tmp/backing.qcow2". Because bdrv_refresh_filename() only works on existing BDSs, though, the way how we get this auto_backing_file value is to have the format driver set it to whatever is in the image header, and when the backing BDS is opened based on that, we update it with the filename the backing BDS actually got. However, qcow2's qcow2_co_invalidate_cache() implementation breaks this because it just resets auto_backing_file to whatever is in the image file without opening a BDS based on it, so we never get auto_backing_file back to the "refreshed" version, and in the example above, it would stay "json:{...}". Then, bs->backing->bs->filename will differ from bs->auto_backing_file, making bdrv_backing_overridden(bs) return true, which will lead bdrv_refresh_filename(bs) to generate a json:{} filename for bs, even though that may not have been necessary. This is reported in the issue linked below. Therefore, skip updating auto_backing_file if nothing has changed in the image header since we last read it. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1117 Signed-off-by: Hanna Reitz Message-Id: <20220803144446.20723-2-hreitz@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/qcow2.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index c8fc3a6160..6c8c8b2b5a 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1697,16 +1697,27 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, ret = -EINVAL; goto fail; } + + s->image_backing_file = g_malloc(len + 1); ret = bdrv_pread(bs->file, header.backing_file_offset, len, - bs->auto_backing_file, 0); + s->image_backing_file, 0); if (ret < 0) { error_setg_errno(errp, -ret, "Could not read backing file name"); goto fail; } - bs->auto_backing_file[len] = '\0'; - pstrcpy(bs->backing_file, sizeof(bs->backing_file), - bs->auto_backing_file); - s->image_backing_file = g_strdup(bs->auto_backing_file); + s->image_backing_file[len] = '\0'; + + /* + * Update only when something has changed. This function is called by + * qcow2_co_invalidate_cache(), and we do not want to reset + * auto_backing_file unless necessary. + */ + if (!g_str_equal(s->image_backing_file, bs->backing_file)) { + pstrcpy(bs->backing_file, sizeof(bs->backing_file), + s->image_backing_file); + pstrcpy(bs->auto_backing_file, sizeof(bs->auto_backing_file), + s->image_backing_file); + } } /*