When refering to a backing file of an image via node name
bdrv_open_backing_file would add the 'driver' option to the option list
filling it with the backing format driver. This breaks construction of
the backing chain via -blockdev, as bdrv_open_inherit reports an error
if both 'reference' and 'options' are provided.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
block.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 46eb1728da..684cb018da 100644
--- a/block.c
+++ b/block.c
@@ -2245,7 +2245,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
goto free_exit;
}
- if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) {
+ if (!reference &&
+ bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) {
qdict_put_str(options, "driver", bs->backing_format);
}
--
2.14.1
Am 12.10.2017 um 16:14 hat Peter Krempa geschrieben: > When refering to a backing file of an image via node name > bdrv_open_backing_file would add the 'driver' option to the option list > filling it with the backing format driver. This breaks construction of > the backing chain via -blockdev, as bdrv_open_inherit reports an error > if both 'reference' and 'options' are provided. > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> If you don't mind, I'd add a specific example to the commit message: $ qemu-img create -f raw /tmp/backing.raw 64M $ qemu-img create -f qcow2 -F raw -b /tmp/backing.raw /tmp/test.qcow2 $ qemu-system-x86_64 \ -blockdev driver=file,filename=/tmp/backing.raw,node-name=backing \ -blockdev driver=qcow2,file.driver=file,file.filename=/tmp/test.qcow2,node-name=root,backing=backing qemu-system-x86_64: -blockdev driver=qcow2,file.driver=file,file.filename=/tmp/test.qcow2,node-name=root,backing=backing: Could not open backing file: Cannot reference an existing block device with additional options or a new filename > diff --git a/block.c b/block.c > index 46eb1728da..684cb018da 100644 > --- a/block.c > +++ b/block.c > @@ -2245,7 +2245,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, > goto free_exit; > } > > - if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) { > + if (!reference && > + bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) { > qdict_put_str(options, "driver", bs->backing_format); > } Looks good to me. Kevin
On Tue, Oct 17, 2017 at 16:41:00 +0200, Kevin Wolf wrote: > Am 12.10.2017 um 16:14 hat Peter Krempa geschrieben: > > When refering to a backing file of an image via node name > > bdrv_open_backing_file would add the 'driver' option to the option list > > filling it with the backing format driver. This breaks construction of > > the backing chain via -blockdev, as bdrv_open_inherit reports an error > > if both 'reference' and 'options' are provided. > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > > If you don't mind, I'd add a specific example to the commit message: I certainly don't mind. I was frustrated after some attempts to find bugs in my code using it before looking into qemu so I forgot to add the info. (Also it would take me some time to figure out that I have the backing format specified in the overlay image). > > $ qemu-img create -f raw /tmp/backing.raw 64M > $ qemu-img create -f qcow2 -F raw -b /tmp/backing.raw /tmp/test.qcow2 > $ qemu-system-x86_64 \ > -blockdev driver=file,filename=/tmp/backing.raw,node-name=backing \ > -blockdev driver=qcow2,file.driver=file,file.filename=/tmp/test.qcow2,node-name=root,backing=backing > qemu-system-x86_64: -blockdev driver=qcow2,file.driver=file,file.filename=/tmp/test.qcow2,node-name=root,backing=backing: Could not open backing file: Cannot reference an existing block device with additional options or a new filename > > > diff --git a/block.c b/block.c > > index 46eb1728da..684cb018da 100644 > > --- a/block.c > > +++ b/block.c > > @@ -2245,7 +2245,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, > > goto free_exit; > > } > > > > - if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) { > > + if (!reference && > > + bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) { > > qdict_put_str(options, "driver", bs->backing_format); > > } > > Looks good to me. Thanks.
On 10/12/2017 09:14 AM, Peter Krempa wrote: > When refering to a backing file of an image via node name s/refering/referring/ (here and in the subject) > bdrv_open_backing_file would add the 'driver' option to the option list > filling it with the backing format driver. This breaks construction of > the backing chain via -blockdev, as bdrv_open_inherit reports an error > if both 'reference' and 'options' are provided. > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- > block.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
This changes test case 191 to include a backing image that has
backing_fmt set in the image file, but is referenced by node name in the
qemu command line.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/191 | 5 +++--
tests/qemu-iotests/191.out | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191
index ac2b88fd78..00202efd47 100755
--- a/tests/qemu-iotests/191
+++ b/tests/qemu-iotests/191
@@ -56,9 +56,10 @@ echo === Preparing and starting VM ===
echo
TEST_IMG="${TEST_IMG}.base" _make_test_img $size
-TEST_IMG="${TEST_IMG}.mid" _make_test_img -b "${TEST_IMG}.base"
+IMGOPTS=$(_optstr_add "$IMGOPTS" "backing_fmt=$IMGFMT") \
+ TEST_IMG="${TEST_IMG}.mid" _make_test_img -b "${TEST_IMG}.base" $size
_make_test_img -b "${TEST_IMG}.mid"
-TEST_IMG="${TEST_IMG}.ovl2" _make_test_img -b "${TEST_IMG}.mid"
+TEST_IMG="${TEST_IMG}.ovl2" _make_test_img -b "${TEST_IMG}.mid" $size
$QEMU_IO -c 'write -P 0x55 1M 64k' "${TEST_IMG}.mid" | _filter_qemu_io
diff --git a/tests/qemu-iotests/191.out b/tests/qemu-iotests/191.out
index 7bfcd2d5d8..6157fa520f 100644
--- a/tests/qemu-iotests/191.out
+++ b/tests/qemu-iotests/191.out
@@ -3,7 +3,7 @@ QA output created by 191
=== Preparing and starting VM ===
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
-Formatting 'TEST_DIR/t.IMGFMT.mid', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT.mid', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.mid
Formatting 'TEST_DIR/t.IMGFMT.ovl2', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.mid
wrote 65536/65536 bytes at offset 1048576
--
2.13.6
Am 17.10.2017 um 17:11 hat Kevin Wolf geschrieben: > This changes test case 191 to include a backing image that has > backing_fmt set in the image file, but is referenced by node name in the > qemu command line. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/191 | 5 +++-- > tests/qemu-iotests/191.out | 2 +- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191 > index ac2b88fd78..00202efd47 100755 > --- a/tests/qemu-iotests/191 > +++ b/tests/qemu-iotests/191 > @@ -56,9 +56,10 @@ echo === Preparing and starting VM === > echo > > TEST_IMG="${TEST_IMG}.base" _make_test_img $size > -TEST_IMG="${TEST_IMG}.mid" _make_test_img -b "${TEST_IMG}.base" > +IMGOPTS=$(_optstr_add "$IMGOPTS" "backing_fmt=$IMGFMT") \ > + TEST_IMG="${TEST_IMG}.mid" _make_test_img -b "${TEST_IMG}.base" $size > _make_test_img -b "${TEST_IMG}.mid" > -TEST_IMG="${TEST_IMG}.ovl2" _make_test_img -b "${TEST_IMG}.mid" > +TEST_IMG="${TEST_IMG}.ovl2" _make_test_img -b "${TEST_IMG}.mid" $size Please pretend you don't see the addition of " $size" here, it's unnecessary and I'll remove it again. Kevin
On 10/17/2017 10:16 AM, Kevin Wolf wrote: > Am 17.10.2017 um 17:11 hat Kevin Wolf geschrieben: >> This changes test case 191 to include a backing image that has >> backing_fmt set in the image file, but is referenced by node name in the >> qemu command line. >> >> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >> --- >> tests/qemu-iotests/191 | 5 +++-- >> tests/qemu-iotests/191.out | 2 +- >> 2 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191 >> index ac2b88fd78..00202efd47 100755 >> --- a/tests/qemu-iotests/191 >> +++ b/tests/qemu-iotests/191 >> @@ -56,9 +56,10 @@ echo === Preparing and starting VM === >> echo >> >> TEST_IMG="${TEST_IMG}.base" _make_test_img $size >> -TEST_IMG="${TEST_IMG}.mid" _make_test_img -b "${TEST_IMG}.base" >> +IMGOPTS=$(_optstr_add "$IMGOPTS" "backing_fmt=$IMGFMT") \ >> + TEST_IMG="${TEST_IMG}.mid" _make_test_img -b "${TEST_IMG}.base" $size >> _make_test_img -b "${TEST_IMG}.mid" >> -TEST_IMG="${TEST_IMG}.ovl2" _make_test_img -b "${TEST_IMG}.mid" >> +TEST_IMG="${TEST_IMG}.ovl2" _make_test_img -b "${TEST_IMG}.mid" $size > > Please pretend you don't see the addition of " $size" here, it's > unnecessary and I'll remove it again. As tweaked, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
© 2016 - 2024 Red Hat, Inc.