From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, pkrempa@redhat.com, qemu-block@nongnu.org,
kchamart@redhat.com, libvir-list@redhat.com, mreitz@redhat.com
Subject: [PATCH v5 4/7] qcow2: Deprecate use of qemu-img amend to change backing file
Date: Fri, 3 Apr 2020 12:58:56 -0500 [thread overview]
Message-ID: <20200403175859.863248-5-eblake@redhat.com> (raw)
In-Reply-To: <20200403175859.863248-1-eblake@redhat.com>
The use of 'qemu-img amend' to change qcow2 backing files is not
tested very well. In particular, our implementation has a bug where
if a new backing file is provided without a format, then the prior
format is blindly reused, even if this results in data corruption, but
this is not caught by iotests.
There are also situations where amending other options needs access to
the original backing file (for example, on a downgrade to a v2 image,
knowing whether a v3 zero cluster must be allocated or may be left
unallocated depends on knowing whether the backing file already reads
as zero), but the command line does not have a nice way to tell us
both the backing file to use for opening the image as well as the
backing file to install after the operation is complete.
Even if we do allow changing the backing file, it is redundant with
the existing ability to change backing files via 'qemu-img rebase -u'.
It is time to deprecate this support (leaving the existing behavior
intact, even if it is buggy), and at a point in the future, require
the use of only 'qemu-img rebase' for adjusting backing chain
relations, saving 'qemu-img amend' for changes unrelated to the
backing chain.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
docs/system/deprecated.rst | 12 ++++++++++++
docs/tools/qemu-img.rst | 4 ++++
block/qcow2.c | 5 +++++
tests/qemu-iotests/061.out | 1 +
tests/qemu-iotests/082.out | 2 ++
5 files changed, 24 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index c633fe2beffd..aac6be58917a 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -417,6 +417,18 @@ The above, converted to the current supported format::
Related binaries
----------------
+qemu-img amend to adjust backing file (since 5.0.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The use of ``qemu-img amend`` to modify the name or format of a qcow2
+backing image is deprecated; this functionality was never fully
+documented or tested, and interferes with other amend operations that
+need access to the original backing image (such as deciding whether a
+v3 zero cluster may be left unallocated when converting to a v2
+image). Rather, any changes to the backing chain should be performed
+with ``qemu-img rebase -u`` either before or after the remaining
+changes being performed by amend, as appropriate.
+
``qemu-img convert -n -o`` (since 4.2.0)
''''''''''''''''''''''''''''''''''''''''
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 0080f83a76c9..83d57586be96 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -254,6 +254,10 @@ Command description:
Amends the image format specific *OPTIONS* for the image file
*FILENAME*. Not all file formats support this operation.
+ The set of options that can be amended are dependent on the image
+ format, but note that amending the backing chain relationship should
+ instead be performed with ``qemu-img rebase``.
+
.. option:: bench [-c COUNT] [-d DEPTH] [-f FMT] [--flush-interval=FLUSH_INTERVAL] [-i AIO] [-n] [--no-drain] [-o OFFSET] [--pattern=PATTERN] [-q] [-s BUFFER_SIZE] [-S STEP_SIZE] [-t CACHE] [-w] [-U] FILENAME
Run a simple sequential I/O benchmark on the specified image. If ``-w`` is
diff --git a/block/qcow2.c b/block/qcow2.c
index 2bb536b01494..e4b772726010 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5303,6 +5303,11 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
}
if (backing_file || backing_format) {
+ if (g_strcmp0(backing_file, s->image_backing_file) ||
+ g_strcmp0(backing_format, s->image_backing_format)) {
+ warn_report("Deprecated use of amend to alter the backing file; "
+ "use qemu-img rebase instead");
+ }
ret = qcow2_change_backing_file(bs,
backing_file ?: s->image_backing_file,
backing_format ?: s->image_backing_format);
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 413cc4e0f4ab..048248c121cb 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -342,6 +342,7 @@ wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
No errors were found on the image.
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 9d4ed4dc9d61..94ea990a2754 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -806,10 +806,12 @@ Creation options for 'qcow2':
Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2
+qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead
Testing: rebase -u -b -f qcow2 TEST_DIR/t.qcow2
Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,? TEST_DIR/t.qcow2
+qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead
Testing: rebase -u -b -f qcow2 TEST_DIR/t.qcow2
--
2.26.0.rc2
next prev parent reply other threads:[~2020-04-03 18:04 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-03 17:58 [PATCH v5 for-5.0? 0/7] Tighten qemu-img rules on missing backing format Eric Blake
2020-04-03 17:58 ` [PATCH v5 1/7] sheepdog: Add trivial backing_fmt support Eric Blake
2020-04-03 17:58 ` [PATCH v5 2/7] vmdk: " Eric Blake
2020-04-03 17:58 ` [PATCH v5 3/7] qcow: Tolerate backing_fmt=, but warn on backing_fmt=raw Eric Blake
2020-05-05 7:35 ` Kevin Wolf
2020-05-05 15:30 ` Eric Blake
2020-06-22 21:58 ` Eric Blake
2020-06-23 10:40 ` Kevin Wolf
2020-04-03 17:58 ` Eric Blake [this message]
2020-05-05 7:50 ` [PATCH v5 4/7] qcow2: Deprecate use of qemu-img amend to change backing file Kevin Wolf
2020-04-03 17:58 ` [PATCH v5 5/7] iotests: Specify explicit backing format where sensible Eric Blake
2020-04-03 17:58 ` [PATCH v5 6/7] block: Add support to warn on backing file change without format Eric Blake
2020-04-03 17:58 ` [PATCH v5 7/7] qemu-img: Deprecate use of -b without -F Eric Blake
2020-05-05 8:11 ` Kevin Wolf
2020-05-05 8:43 ` Peter Krempa
2020-05-04 20:02 ` [PATCH v5 for-5.0? 0/7] Tighten qemu-img rules on missing backing format Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200403175859.863248-5-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=kchamart@redhat.com \
--cc=kwolf@redhat.com \
--cc=libvir-list@redhat.com \
--cc=mreitz@redhat.com \
--cc=pkrempa@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).