qcow2: Fix header update with overridden backing file
PATCH v2 1/1 qcow2: Fix header update with overridden backing file
In recent qemu versions, it is possible to override the backing file name and format that is stored in the image file with values given at runtime. In such cases, the temporary override could end up in the image header if the qcow2 header was updated, while obviously correct behaviour would be to leave the on-disk backing file path/format unchanged.
Fix this and add a test case for it.
---
v2:
- Now freeing instead of leaking the new strings in qcow2_close()
block/qcow2.c | 29 ++++++++++---- block/qcow2.h | 6 +++ tests/qemu-iotests/130 | 95 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/130.out | 43 +++++++++++++++++++++ tests/qemu-iotests/group | 1 + 5 files changed, 167 insertions(+), 7 deletions(-) create mode 100755 tests/qemu-iotests/130 create mode 100644 tests/qemu-iotests/130.out
block/qcow2.c +22 -7
@@ -140,6 +140,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,return 3 ; } bs->backing_format[ext.len] ='\0' ; + s->image_backing_format =g_strdup (bs->backing_format);#ifdef DEBUG_EXT printf ("Qcow2: Got format extension %s\n" , bs->backing_format);#endif
@@ -884,6 +885,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } bs->backing_file[len] ='\0' ; + s->image_backing_file =g_strdup (bs->backing_file); }/* Internal snapshots */
@@ -1457,6 +1459,9 @@ static void qcow2_close(BlockDriverState *bs)g_free (s->unknown_header_fields);cleanup_unknown_header_ext (bs); +g_free (s->image_backing_file); +g_free (s->image_backing_format); +g_free (s->cluster_cache);qemu_vfree (s->cluster_data);qcow2_refcount_close (bs);
@@ -1622,9 +1627,10 @@ int qcow2_update_header(BlockDriverState *bs) }/* Backing file format header extension */ -if (*bs->backing_format) { +if (s->image_backing_format) { ret =header_ext_add (buf,QCOW2_EXT_MAGIC_BACKING_FORMAT , - bs->backing_format,strlen (bs->backing_format), + s->image_backing_format, +strlen (s->image_backing_format), buflen);if (ret <0 ) { goto fail;
@@ -1682,8 +1688,8 @@ int qcow2_update_header(BlockDriverState *bs) buflen -= ret;/* Backing file name */ -if (*bs->backing_file) { -size_t backing_file_len =strlen (bs->backing_file); +if (s->image_backing_file) { +size_t backing_file_len =strlen (s->image_backing_file);if (buflen < backing_file_len) { ret = -ENOSPC ;
@@ -1691,7 +1697,7 @@ int qcow2_update_header(BlockDriverState *bs) }/* Using strncpy is ok here, since buf is not NUL-terminated. */ -strncpy (buf, bs->backing_file, buflen); +strncpy (buf, s->image_backing_file, buflen); header->backing_file_offset =cpu_to_be64 (buf - ((char *) header)); header->backing_file_size =cpu_to_be32 (backing_file_len);
@@ -1712,9 +1718,17 @@ fail:static int qcow2_change_backing_file (BlockDriverState *bs,const char *backing_file,const char *backing_fmt) { +BDRVQcowState *s = bs->opaque; +pstrcpy (bs->backing_file,sizeof (bs->backing_file), backing_file ?:"" );pstrcpy (bs->backing_format,sizeof (bs->backing_format), backing_fmt ?:"" ); +g_free (s->image_backing_file); +g_free (s->image_backing_format); + + s->image_backing_file = backing_file ?g_strdup (bs->backing_file) :NULL ; + s->image_backing_format = backing_fmt ?g_strdup (bs->backing_format) :NULL ; +return qcow2_update_header (bs); }
@@ -2751,8 +2765,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, }if (backing_file || backing_format) { - ret =qcow2_change_backing_file (bs, backing_file ?: bs->backing_file, - backing_format ?: bs->backing_format); + ret =qcow2_change_backing_file (bs, + backing_file ?: s->image_backing_file, + backing_format ?: s->image_backing_format);if (ret <0 ) {return ret; }
block/qcow2.h +6 -0
@@ -283,6 +283,12 @@ typedef struct BDRVQcowState { QLIST_HEAD(, Qcow2UnknownHeaderExtension) unknown_header_ext; QTAILQ_HEAD (, Qcow2DiscardRegion) discards;bool cache_discards; + +/* Backing file path and format as stored in the image (this is not the +* effective path/format, which may be the result of a runtime option +* override) */ +char *image_backing_file; +char *image_backing_format; } BDRVQcowState;struct QCowAIOCB ;
tests/qemu-iotests/130 new file +95 -0
95 lines not on this page · raw message
tests/qemu-iotests/130.out new file +43 -0
43 lines not on this page · raw message
tests/qemu-iotests/group +1 -0
@@ -124,3 +124,4 @@ 121 rw auto 123 rw auto quick 128 rw auto quick +130 rw auto quick
Signature · 1 line
1.8.3.1
Quoted 15 lines · Kevin Wolf
In recent qemu versions, it is possible to override the backing file name and format that is stored in the image file with values given at runtime. In such cases, the temporary override could end up in the image header if the qcow2 header was updated, while obviously correct behaviour would be to leave the on-disk backing file path/format unchanged.
Fix this and add a test case for it.
Reported-by Michael Tokarev <[email protected]>Signed-off-by Kevin Wolf <[email protected]>---
v2:
- Now freeing instead of leaking the new strings inqcow2_close()
Quoted 8 lines
block/qcow2.c | 29 ++++++++++---- block/qcow2.h | 6 +++ tests/qemu-iotests/130 | 95 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/130.out | 43 +++++++++++++++++++++ tests/qemu-iotests/group | 1 + 5 files changed, 167 insertions(+), 7 deletions(-) create mode 100755 tests/qemu-iotests/130 create mode 100644 tests/qemu-iotests/130.out
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Stefan Hajnoczi
AppliedQuoted 15 lines · Kevin Wolf
In recent qemu versions, it is possible to override the backing file name and format that is stored in the image file with values given at runtime. In such cases, the temporary override could end up in the image header if the qcow2 header was updated, while obviously correct behaviour would be to leave the on-disk backing file path/format unchanged.
Fix this and add a test case for it.
Reported-by Michael Tokarev <[email protected]>Signed-off-by Kevin Wolf <[email protected]>---
v2:
- Now freeing instead of leaking the new strings inqcow2_close()
Quoted 8 lines
block/qcow2.c | 29 ++++++++++---- block/qcow2.h | 6 +++ tests/qemu-iotests/130 | 95 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/130.out | 43 +++++++++++++++++++++ tests/qemu-iotests/group | 1 + 5 files changed, 167 insertions(+), 7 deletions(-) create mode 100755 tests/qemu-iotests/130 create mode 100644 tests/qemu-iotests/130.out
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan