* [Qemu-devel] [PATCH v2 1/2] qemu-img: Resolve relative backing paths in rebase
2018-05-09 18:20 [Qemu-devel] [PATCH v2 0/2] qemu-img: Resolve relative backing paths in rebase Max Reitz
@ 2018-05-09 18:20 ` Max Reitz
2018-05-09 18:51 ` Eric Blake
2018-05-09 18:20 ` [Qemu-devel] [PATCH v2 2/2] iotests: Add test for rebasing with relative paths Max Reitz
2018-06-01 11:13 ` [Qemu-devel] [PATCH v2 0/2] qemu-img: Resolve relative backing paths in rebase Max Reitz
2 siblings, 1 reply; 6+ messages in thread
From: Max Reitz @ 2018-05-09 18:20 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, qemu-stable, Eric Blake, Kevin Wolf
Currently, rebase interprets a relative path for the new backing image
as follows:
(1) Open the new backing image with the given relative path (thus relative to
qemu-img's working directory).
(2) Write it directly into the overlay's backing path field (thus
relative to the overlay).
If the overlay is not in qemu-img's working directory, both will be
different interpretations, which may either lead to an error somewhere
(either rebase fails because it cannot open the new backing image, or
your overlay becomes unusable because its backing path does not point
to a file), or, even worse, it may result in your rebase being
performed for a different backing file than what your overlay will point
to after the rebase.
Fix this by interpreting the target backing path as relative to the
overlay, like qemu-img does everywhere else.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1569835
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index ea62d2d61e..55b146aa49 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3191,6 +3191,9 @@ static int img_rebase(int argc, char **argv)
}
if (out_baseimg[0]) {
+ const char *overlay_filename;
+ char *out_real_path;
+
options = qdict_new();
if (out_basefmt) {
qdict_put_str(options, "driver", out_basefmt);
@@ -3199,8 +3202,26 @@ static int img_rebase(int argc, char **argv)
qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true);
}
- blk_new_backing = blk_new_open(out_baseimg, NULL,
+ overlay_filename = bs->exact_filename[0] ? bs->exact_filename
+ : bs->filename;
+ out_real_path = g_malloc(PATH_MAX);
+
+ bdrv_get_full_backing_filename_from_filename(overlay_filename,
+ out_baseimg,
+ out_real_path,
+ PATH_MAX,
+ &local_err);
+ if (local_err) {
+ error_reportf_err(local_err,
+ "Could not resolve backing filename: ");
+ ret = -1;
+ g_free(out_real_path);
+ goto out;
+ }
+
+ blk_new_backing = blk_new_open(out_real_path, NULL,
options, src_flags, &local_err);
+ g_free(out_real_path);
if (!blk_new_backing) {
error_reportf_err(local_err,
"Could not open new backing file '%s': ",
--
2.14.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/2] qemu-img: Resolve relative backing paths in rebase
2018-05-09 18:20 ` [Qemu-devel] [PATCH v2 1/2] " Max Reitz
@ 2018-05-09 18:51 ` Eric Blake
2018-05-09 19:20 ` Max Reitz
0 siblings, 1 reply; 6+ messages in thread
From: Eric Blake @ 2018-05-09 18:51 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, qemu-stable, Kevin Wolf
On 05/09/2018 01:20 PM, Max Reitz wrote:
> Currently, rebase interprets a relative path for the new backing image
> as follows:
> (1) Open the new backing image with the given relative path (thus relative to
> qemu-img's working directory).
> (2) Write it directly into the overlay's backing path field (thus
> relative to the overlay).
>
> If the overlay is not in qemu-img's working directory, both will be
> different interpretations, which may either lead to an error somewhere
> (either rebase fails because it cannot open the new backing image, or
> your overlay becomes unusable because its backing path does not point
> to a file), or, even worse, it may result in your rebase being
> performed for a different backing file than what your overlay will point
Interesting indentation in this paragraph. A maintainer could tweak it.
> to after the rebase.
>
> Fix this by interpreting the target backing path as relative to the
> overlay, like qemu-img does everywhere else.
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1569835
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> qemu-img.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] qemu-img: Resolve relative backing paths in rebase
2018-05-09 18:51 ` Eric Blake
@ 2018-05-09 19:20 ` Max Reitz
0 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2018-05-09 19:20 UTC (permalink / raw)
To: Eric Blake, qemu-block; +Cc: qemu-devel, qemu-stable, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]
On 2018-05-09 20:51, Eric Blake wrote:
> On 05/09/2018 01:20 PM, Max Reitz wrote:
>> Currently, rebase interprets a relative path for the new backing image
>> as follows:
>> (1) Open the new backing image with the given relative path (thus
>> relative to
>> qemu-img's working directory).
>> (2) Write it directly into the overlay's backing path field (thus
>> relative to the overlay).
>>
>> If the overlay is not in qemu-img's working directory, both will be
>> different interpretations, which may either lead to an error somewhere
>> (either rebase fails because it cannot open the new backing image, or
>> your overlay becomes unusable because its backing path does not point
>> to a file), or, even worse, it may result in your rebase being
>> performed for a different backing file than what your overlay will point
>
> Interesting indentation in this paragraph. A maintainer could tweak it.
Well, it was meant this way (because of the opening paranthesis), but I
could out-dent it.
Max
>> to after the rebase.
>>
>> Fix this by interpreting the target backing path as relative to the
>> overlay, like qemu-img does everywhere else.
>>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1569835
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qemu-img.c | 23 ++++++++++++++++++++++-
>> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] iotests: Add test for rebasing with relative paths
2018-05-09 18:20 [Qemu-devel] [PATCH v2 0/2] qemu-img: Resolve relative backing paths in rebase Max Reitz
2018-05-09 18:20 ` [Qemu-devel] [PATCH v2 1/2] " Max Reitz
@ 2018-05-09 18:20 ` Max Reitz
2018-06-01 11:13 ` [Qemu-devel] [PATCH v2 0/2] qemu-img: Resolve relative backing paths in rebase Max Reitz
2 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2018-05-09 18:20 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, qemu-stable, Eric Blake, Kevin Wolf
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/024 | 82 ++++++++++++++++++++++++++++++++++++++++++++--
tests/qemu-iotests/024.out | 30 +++++++++++++++++
2 files changed, 109 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/024 b/tests/qemu-iotests/024
index e0d77ce2f5..4071ed6093 100755
--- a/tests/qemu-iotests/024
+++ b/tests/qemu-iotests/024
@@ -29,9 +29,14 @@ status=1 # failure is the default!
_cleanup()
{
- _cleanup_test_img
- rm -f "$TEST_DIR/t.$IMGFMT.base_old"
- rm -f "$TEST_DIR/t.$IMGFMT.base_new"
+ _cleanup_test_img
+ rm -f "$TEST_DIR/t.$IMGFMT.base_old"
+ rm -f "$TEST_DIR/t.$IMGFMT.base_new"
+
+ rm -f "$TEST_DIR/subdir/t.$IMGFMT"
+ rm -f "$TEST_DIR/subdir/t.$IMGFMT.base_old"
+ rm -f "$TEST_DIR/subdir/t.$IMGFMT.base_new"
+ rmdir "$TEST_DIR/subdir" 2> /dev/null
}
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -123,6 +128,77 @@ io_pattern readv $((13 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x00
io_pattern readv $((14 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x11
io_pattern readv $((15 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x00
+echo
+echo "=== Test rebase in a subdirectory of the working directory ==="
+echo
+
+# Clean up the old images beforehand so they do not interfere with
+# this test
+_cleanup
+
+mkdir "$TEST_DIR/subdir"
+
+# Relative to the overlay
+BASE_OLD_OREL="t.$IMGFMT.base_old"
+BASE_NEW_OREL="t.$IMGFMT.base_new"
+
+# Relative to $TEST_DIR (which is going to be our working directory)
+OVERLAY_WREL="subdir/t.$IMGFMT"
+
+BASE_OLD="$TEST_DIR/subdir/$BASE_OLD_OREL"
+BASE_NEW="$TEST_DIR/subdir/$BASE_NEW_OREL"
+OVERLAY="$TEST_DIR/$OVERLAY_WREL"
+
+# Test done here:
+#
+# Backing (old): 11 11 -- 11
+# Backing (new): -- 22 22 11
+# Overlay: -- -- -- --
+#
+# Rebasing works, we have verified that above. Here, we just want to
+# see that rebasing is done for the correct target backing file.
+
+TEST_IMG=$BASE_OLD _make_test_img 1M
+TEST_IMG=$BASE_NEW _make_test_img 1M
+TEST_IMG=$OVERLAY _make_test_img -b "$BASE_OLD_OREL" 1M
+
+echo
+
+$QEMU_IO "$BASE_OLD" \
+ -c "write -P 0x11 $((0 * CLUSTER_SIZE)) $((2 * CLUSTER_SIZE))" \
+ -c "write -P 0x11 $((3 * CLUSTER_SIZE)) $((1 * CLUSTER_SIZE))" \
+ | _filter_qemu_io
+
+$QEMU_IO "$BASE_NEW" \
+ -c "write -P 0x22 $((1 * CLUSTER_SIZE)) $((2 * CLUSTER_SIZE))" \
+ -c "write -P 0x11 $((3 * CLUSTER_SIZE)) $((1 * CLUSTER_SIZE))" \
+ | _filter_qemu_io
+
+echo
+
+pushd "$TEST_DIR" >/dev/null
+$QEMU_IMG rebase -f "$IMGFMT" -b "$BASE_NEW_OREL" "$OVERLAY_WREL"
+popd >/dev/null
+
+# Verify the backing path is correct
+TEST_IMG=$OVERLAY _img_info | grep '^backing file'
+
+echo
+
+# Verify the data is correct
+$QEMU_IO "$OVERLAY" \
+ -c "read -P 0x11 $((0 * CLUSTER_SIZE)) $CLUSTER_SIZE" \
+ -c "read -P 0x11 $((1 * CLUSTER_SIZE)) $CLUSTER_SIZE" \
+ -c "read -P 0x00 $((2 * CLUSTER_SIZE)) $CLUSTER_SIZE" \
+ -c "read -P 0x11 $((3 * CLUSTER_SIZE)) $CLUSTER_SIZE" \
+ | _filter_qemu_io
+
+echo
+
+# Verify that cluster #3 is not allocated (because it is the same in
+# $BASE_OLD and $BASE_NEW)
+$QEMU_IMG map "$OVERLAY" | _filter_qemu_img_map
+
# success, all done
echo "*** done"
diff --git a/tests/qemu-iotests/024.out b/tests/qemu-iotests/024.out
index 33cfaf5cfc..024dc786b3 100644
--- a/tests/qemu-iotests/024.out
+++ b/tests/qemu-iotests/024.out
@@ -141,4 +141,34 @@ read 65536/65536 bytes at offset 917504
=== IO: pattern 0x00
read 65536/65536 bytes at offset 983040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Test rebase in a subdirectory of the working directory ===
+
+Formatting 'TEST_DIR/subdir/t.IMGFMT.base_old', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/subdir/t.IMGFMT.base_new', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/subdir/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=t.IMGFMT.base_old
+
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 196608
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 65536
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 196608
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+backing file: t.IMGFMT.base_new (actual path: TEST_DIR/subdir/t.IMGFMT.base_new)
+
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 131072
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 196608
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Offset Length File
+0 0x30000 TEST_DIR/subdir/t.IMGFMT
+0x30000 0x10000 TEST_DIR/subdir/t.IMGFMT.base_new
*** done
--
2.14.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/2] qemu-img: Resolve relative backing paths in rebase
2018-05-09 18:20 [Qemu-devel] [PATCH v2 0/2] qemu-img: Resolve relative backing paths in rebase Max Reitz
2018-05-09 18:20 ` [Qemu-devel] [PATCH v2 1/2] " Max Reitz
2018-05-09 18:20 ` [Qemu-devel] [PATCH v2 2/2] iotests: Add test for rebasing with relative paths Max Reitz
@ 2018-06-01 11:13 ` Max Reitz
2 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2018-06-01 11:13 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, qemu-stable, Eric Blake, Kevin Wolf
[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]
On 2018-05-09 20:20, Max Reitz wrote:
> v1 of the cover letter here (no change necessary there):
> http://lists.nongnu.org/archive/html/qemu-block/2018-05/msg00243.html
>
> v2: Fixed a leaked buffer in an error path in patch 1 [Eric]
>
>
> git-backport-diff against v1:
>
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/2:[0001] [FC] 'qemu-img: Resolve relative backing paths in rebase'
> 002/2:[----] [--] 'iotests: Add test for rebasing with relative paths'
>
>
> Max Reitz (2):
> qemu-img: Resolve relative backing paths in rebase
> iotests: Add test for rebasing with relative paths
>
> qemu-img.c | 23 ++++++++++++-
> tests/qemu-iotests/024 | 82 ++++++++++++++++++++++++++++++++++++++++++++--
> tests/qemu-iotests/024.out | 30 +++++++++++++++++
> 3 files changed, 131 insertions(+), 4 deletions(-)
Applied to my block branch.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread