qcow2: Fix header update with overridden backing file
PATCH 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.
--- block/qcow2.c | 26 +++++++++---- block/qcow2.h | 6 +++ tests/qemu-iotests/130 | 95 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/130.out | 43 +++++++++++++++++++++ tests/qemu-iotests/group | 1 + 5 files changed, 164 insertions(+), 7 deletions(-) create mode 100755 tests/qemu-iotests/130 create mode 100644 tests/qemu-iotests/130.out
block/qcow2.c +19 -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 */
@@ -1622,9 +1624,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 +1685,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 +1694,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 +1715,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 +2762,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
Am 07.04.2015 um 14:51 hat Kevin Wolf geschrieben:
Quoted 15 lines
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]>
block/qcow2.h typedef struct BDRVQcowState { · line 292
Quoted 10 lines
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;
*sigh* I guess freeing these in qcow2_close() wouldn't hurt either. Sending v2.
Kevin