9ae3a8
From 8538f7d5ace7b8ddb751d61a17c4a822b2333b39 Mon Sep 17 00:00:00 2001
9ae3a8
From: Max Reitz <mreitz@redhat.com>
9ae3a8
Date: Sat, 13 Jun 2015 16:22:35 +0200
9ae3a8
Subject: [PATCH 41/42] qcow2: Fix header update with overridden backing file
9ae3a8
9ae3a8
Message-id: <1434212556-3927-42-git-send-email-mreitz@redhat.com>
9ae3a8
Patchwork-id: 66060
9ae3a8
O-Subject: [RHEL-7.2 qemu-kvm PATCH 41/42] qcow2: Fix header update with overridden backing file
9ae3a8
Bugzilla: 1129893
9ae3a8
RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
9ae3a8
RH-Acked-by: Fam Zheng <famz@redhat.com>
9ae3a8
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
9ae3a8
From: Kevin Wolf <kwolf@redhat.com>
9ae3a8
9ae3a8
BZ: 1129893
9ae3a8
9ae3a8
In recent qemu versions, it is possible to override the backing file
9ae3a8
name and format that is stored in the image file with values given at
9ae3a8
runtime. In such cases, the temporary override could end up in the
9ae3a8
image header if the qcow2 header was updated, while obviously correct
9ae3a8
behaviour would be to leave the on-disk backing file path/format
9ae3a8
unchanged.
9ae3a8
9ae3a8
Fix this and add a test case for it.
9ae3a8
9ae3a8
Reported-by: Michael Tokarev <mjt@tls.msk.ru>
9ae3a8
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
9ae3a8
Reviewed-by: Eric Blake <eblake@redhat.com>
9ae3a8
Message-id: 1428411796-2852-1-git-send-email-kwolf@redhat.com
9ae3a8
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
(cherry picked from commit e4603fe139e2161464d7e75faa3a650e31f057fc)
9ae3a8
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9ae3a8
9ae3a8
Conflicts:
9ae3a8
	tests/qemu-iotests/group
9ae3a8
	tests/qemu-iotests/130
9ae3a8
	tests/qemu-iotests/130.out
9ae3a8
9ae3a8
common.qemu does not exist downstream, so the HMP commit case cannot be
9ae3a8
tested. _filter_img_info does not exist either, so just omit it and add
9ae3a8
the additional cluster_size line to the test output (this test is
9ae3a8
qcow2-specific anyway).
9ae3a8
9ae3a8
Signed-off-by: Max Reitz <mreitz@redhat.com>
9ae3a8
---
9ae3a8
 block/qcow2.c              | 29 ++++++++++++++-----
9ae3a8
 block/qcow2.h              |  6 ++++
9ae3a8
 tests/qemu-iotests/130     | 71 ++++++++++++++++++++++++++++++++++++++++++++++
9ae3a8
 tests/qemu-iotests/130.out | 28 ++++++++++++++++++
9ae3a8
 tests/qemu-iotests/group   |  1 +
9ae3a8
 5 files changed, 128 insertions(+), 7 deletions(-)
9ae3a8
 create mode 100755 tests/qemu-iotests/130
9ae3a8
 create mode 100644 tests/qemu-iotests/130.out
9ae3a8
9ae3a8
diff --git a/block/qcow2.c b/block/qcow2.c
9ae3a8
index 991c41f..61f7e57 100644
9ae3a8
--- a/block/qcow2.c
9ae3a8
+++ b/block/qcow2.c
9ae3a8
@@ -139,6 +139,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
9ae3a8
                 return 3;
9ae3a8
             }
9ae3a8
             bs->backing_format[ext.len] = '\0';
9ae3a8
+            s->image_backing_format = g_strdup(bs->backing_format);
9ae3a8
 #ifdef DEBUG_EXT
9ae3a8
             printf("Qcow2: Got format extension %s\n", bs->backing_format);
9ae3a8
 #endif
9ae3a8
@@ -734,6 +735,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
9ae3a8
             goto fail;
9ae3a8
         }
9ae3a8
         bs->backing_file[len] = '\0';
9ae3a8
+        s->image_backing_file = g_strdup(bs->backing_file);
9ae3a8
     }
9ae3a8
 
9ae3a8
     /* Internal snapshots */
9ae3a8
@@ -1249,6 +1251,9 @@ static void qcow2_close(BlockDriverState *bs)
9ae3a8
     g_free(s->unknown_header_fields);
9ae3a8
     cleanup_unknown_header_ext(bs);
9ae3a8
 
9ae3a8
+    g_free(s->image_backing_file);
9ae3a8
+    g_free(s->image_backing_format);
9ae3a8
+
9ae3a8
     g_free(s->cluster_cache);
9ae3a8
     qemu_vfree(s->cluster_data);
9ae3a8
     qcow2_refcount_close(bs);
9ae3a8
@@ -1399,9 +1404,10 @@ int qcow2_update_header(BlockDriverState *bs)
9ae3a8
     }
9ae3a8
 
9ae3a8
     /* Backing file format header extension */
9ae3a8
-    if (*bs->backing_format) {
9ae3a8
+    if (s->image_backing_format) {
9ae3a8
         ret = header_ext_add(buf, QCOW2_EXT_MAGIC_BACKING_FORMAT,
9ae3a8
-                             bs->backing_format, strlen(bs->backing_format),
9ae3a8
+                             s->image_backing_format,
9ae3a8
+                             strlen(s->image_backing_format),
9ae3a8
                              buflen);
9ae3a8
         if (ret < 0) {
9ae3a8
             goto fail;
9ae3a8
@@ -1459,8 +1465,8 @@ int qcow2_update_header(BlockDriverState *bs)
9ae3a8
     buflen -= ret;
9ae3a8
 
9ae3a8
     /* Backing file name */
9ae3a8
-    if (*bs->backing_file) {
9ae3a8
-        size_t backing_file_len = strlen(bs->backing_file);
9ae3a8
+    if (s->image_backing_file) {
9ae3a8
+        size_t backing_file_len = strlen(s->image_backing_file);
9ae3a8
 
9ae3a8
         if (buflen < backing_file_len) {
9ae3a8
             ret = -ENOSPC;
9ae3a8
@@ -1468,7 +1474,7 @@ int qcow2_update_header(BlockDriverState *bs)
9ae3a8
         }
9ae3a8
 
9ae3a8
         /* Using strncpy is ok here, since buf is not NUL-terminated. */
9ae3a8
-        strncpy(buf, bs->backing_file, buflen);
9ae3a8
+        strncpy(buf, s->image_backing_file, buflen);
9ae3a8
 
9ae3a8
         header->backing_file_offset = cpu_to_be64(buf - ((char*) header));
9ae3a8
         header->backing_file_size   = cpu_to_be32(backing_file_len);
9ae3a8
@@ -1489,9 +1495,17 @@ fail:
9ae3a8
 static int qcow2_change_backing_file(BlockDriverState *bs,
9ae3a8
     const char *backing_file, const char *backing_fmt)
9ae3a8
 {
9ae3a8
+    BDRVQcowState *s = bs->opaque;
9ae3a8
+
9ae3a8
     pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_file ?: "");
9ae3a8
     pstrcpy(bs->backing_format, sizeof(bs->backing_format), backing_fmt ?: "");
9ae3a8
 
9ae3a8
+    g_free(s->image_backing_file);
9ae3a8
+    g_free(s->image_backing_format);
9ae3a8
+
9ae3a8
+    s->image_backing_file = backing_file ? g_strdup(bs->backing_file) : NULL;
9ae3a8
+    s->image_backing_format = backing_fmt ? g_strdup(bs->backing_format) : NULL;
9ae3a8
+
9ae3a8
     return qcow2_update_header(bs);
9ae3a8
 }
9ae3a8
 
9ae3a8
@@ -2286,8 +2300,9 @@ static int qcow2_amend_options(BlockDriverState *bs,
9ae3a8
     }
9ae3a8
 
9ae3a8
     if (backing_file || backing_format) {
9ae3a8
-        ret = qcow2_change_backing_file(bs, backing_file ?: bs->backing_file,
9ae3a8
-                                        backing_format ?: bs->backing_format);
9ae3a8
+        ret = qcow2_change_backing_file(bs,
9ae3a8
+                    backing_file ?: s->image_backing_file,
9ae3a8
+                    backing_format ?: s->image_backing_format);
9ae3a8
         if (ret < 0) {
9ae3a8
             return ret;
9ae3a8
         }
9ae3a8
diff --git a/block/qcow2.h b/block/qcow2.h
9ae3a8
index b210a7f..dd3e768 100644
9ae3a8
--- a/block/qcow2.h
9ae3a8
+++ b/block/qcow2.h
9ae3a8
@@ -263,6 +263,12 @@ typedef struct BDRVQcowState {
9ae3a8
     QLIST_HEAD(, Qcow2UnknownHeaderExtension) unknown_header_ext;
9ae3a8
     QTAILQ_HEAD (, Qcow2DiscardRegion) discards;
9ae3a8
     bool cache_discards;
9ae3a8
+
9ae3a8
+    /* Backing file path and format as stored in the image (this is not the
9ae3a8
+     * effective path/format, which may be the result of a runtime option
9ae3a8
+     * override) */
9ae3a8
+    char *image_backing_file;
9ae3a8
+    char *image_backing_format;
9ae3a8
 } BDRVQcowState;
9ae3a8
 
9ae3a8
 /* XXX: use std qcow open function ? */
9ae3a8
diff --git a/tests/qemu-iotests/130 b/tests/qemu-iotests/130
9ae3a8
new file mode 100755
9ae3a8
index 0000000..68dbb48
9ae3a8
--- /dev/null
9ae3a8
+++ b/tests/qemu-iotests/130
9ae3a8
@@ -0,0 +1,71 @@
9ae3a8
+#!/bin/bash
9ae3a8
+#
9ae3a8
+# Test that temporary backing file overrides (on the command line or in
9ae3a8
+# blockdev-add) don't replace the original path stored in the image during
9ae3a8
+# header updates.
9ae3a8
+#
9ae3a8
+# Copyright (C) 2015 Red Hat, Inc.
9ae3a8
+#
9ae3a8
+# This program is free software; you can redistribute it and/or modify
9ae3a8
+# it under the terms of the GNU General Public License as published by
9ae3a8
+# the Free Software Foundation; either version 2 of the License, or
9ae3a8
+# (at your option) any later version.
9ae3a8
+#
9ae3a8
+# This program is distributed in the hope that it will be useful,
9ae3a8
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
9ae3a8
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
9ae3a8
+# GNU General Public License for more details.
9ae3a8
+#
9ae3a8
+# You should have received a copy of the GNU General Public License
9ae3a8
+# along with this program.  If not, see <http: www.gnu.org="" licenses=""/>.
9ae3a8
+#
9ae3a8
+
9ae3a8
+# creator
9ae3a8
+owner=kwolf@redhat.com
9ae3a8
+
9ae3a8
+seq="$(basename $0)"
9ae3a8
+echo "QA output created by $seq"
9ae3a8
+
9ae3a8
+here="$PWD"
9ae3a8
+tmp=/tmp/$$
9ae3a8
+status=1	# failure is the default!
9ae3a8
+
9ae3a8
+_cleanup()
9ae3a8
+{
9ae3a8
+    _cleanup_test_img
9ae3a8
+}
9ae3a8
+trap "_cleanup; exit \$status" 0 1 2 3 15
9ae3a8
+
9ae3a8
+# get standard environment, filters and checks
9ae3a8
+. ./common.rc
9ae3a8
+. ./common.filter
9ae3a8
+
9ae3a8
+_supported_fmt qcow2
9ae3a8
+_supported_proto generic
9ae3a8
+_supported_os Linux
9ae3a8
+
9ae3a8
+
9ae3a8
+TEST_IMG="$TEST_IMG.orig" _make_test_img 64M
9ae3a8
+TEST_IMG="$TEST_IMG.base" _make_test_img 64M
9ae3a8
+_make_test_img 64M
9ae3a8
+_img_info
9ae3a8
+
9ae3a8
+echo
9ae3a8
+echo "=== Marking image dirty (lazy refcounts) ==="
9ae3a8
+echo
9ae3a8
+
9ae3a8
+# Test that a backing file isn't written
9ae3a8
+_make_test_img 64M
9ae3a8
+$QEMU_IO -c "open -o backing.file.filename=$TEST_IMG.base,lazy-refcounts=on $TEST_IMG" -c "write 0 4k" | _filter_qemu_io
9ae3a8
+_img_info
9ae3a8
+
9ae3a8
+# Make sure that if there was a backing file that was just overridden on the
9ae3a8
+# command line, that backing file is retained, with the right format
9ae3a8
+_make_test_img -F raw -b "$TEST_IMG.orig" 64M
9ae3a8
+$QEMU_IO -c "open -o backing.file.filename=$TEST_IMG.base,backing.driver=$IMGFMT,lazy-refcounts=on $TEST_IMG" -c "write 0 4k" | _filter_qemu_io
9ae3a8
+_img_info
9ae3a8
+
9ae3a8
+# success, all done
9ae3a8
+echo '*** done'
9ae3a8
+rm -f $seq.full
9ae3a8
+status=0
9ae3a8
diff --git a/tests/qemu-iotests/130.out b/tests/qemu-iotests/130.out
9ae3a8
new file mode 100644
9ae3a8
index 0000000..bd489dd
9ae3a8
--- /dev/null
9ae3a8
+++ b/tests/qemu-iotests/130.out
9ae3a8
@@ -0,0 +1,28 @@
9ae3a8
+QA output created by 130
9ae3a8
+Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=67108864
9ae3a8
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
9ae3a8
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
9ae3a8
+image: TEST_DIR/t.IMGFMT
9ae3a8
+file format: IMGFMT
9ae3a8
+virtual size: 64M (67108864 bytes)
9ae3a8
+cluster_size: 65536
9ae3a8
+
9ae3a8
+=== Marking image dirty (lazy refcounts) ===
9ae3a8
+
9ae3a8
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
9ae3a8
+wrote 4096/4096 bytes at offset 0
9ae3a8
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
9ae3a8
+image: TEST_DIR/t.IMGFMT
9ae3a8
+file format: IMGFMT
9ae3a8
+virtual size: 64M (67108864 bytes)
9ae3a8
+cluster_size: 65536
9ae3a8
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.orig' backing_fmt='raw'
9ae3a8
+wrote 4096/4096 bytes at offset 0
9ae3a8
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
9ae3a8
+image: TEST_DIR/t.IMGFMT
9ae3a8
+file format: IMGFMT
9ae3a8
+virtual size: 64M (67108864 bytes)
9ae3a8
+cluster_size: 65536
9ae3a8
+backing file: TEST_DIR/t.IMGFMT.orig
9ae3a8
+backing file format: raw
9ae3a8
+*** done
9ae3a8
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
9ae3a8
index dbb2a36..739c266 100644
9ae3a8
--- a/tests/qemu-iotests/group
9ae3a8
+++ b/tests/qemu-iotests/group
9ae3a8
@@ -90,3 +90,4 @@
9ae3a8
 108 rw auto quick
9ae3a8
 114 rw auto quick
9ae3a8
 121 rw auto
9ae3a8
+130 rw auto quick
9ae3a8
-- 
9ae3a8
1.8.3.1
9ae3a8