| Message ID | 20200403175859.863248-4-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Tighten qemu-img rules on missing backing format | expand |
Am 03.04.2020 um 19:58 hat Eric Blake geschrieben: > qcow has no space in the metadata to store a backing format, and there > are existing qcow images backed both by raw or by other formats > (usually qcow) images, reliant on probing to tell the difference. > While we don't recommend the creation of new qcow images (as qcow2 is > hands-down better), we can at least insist that if the user does > request a specific format without using -u, then it must be non-raw > (as a raw backing file that gets inadvertently edited into some other > format can form a security hole); if the user does not request a > specific format or lies when using -u, then the status quo of probing > for the backing format remains intact (although an upcoming patch will > warn when omitting a format request). Thus, when this series is > complete, the only way to use a backing file for qcow without > triggering a warning is when using -F if the backing file is non-raw > to begin with. Note that this is only for QemuOpts usage; there is no > change to the QAPI to allow a format through -blockdev. > > Add a new iotest 290 just for qcow, to demonstrate the new warning. > > Signed-off-by: Eric Blake <eblake@redhat.com> Somehow this feels backwards. Not specifying the backing file format at all isn't any safer than explicitly specifying raw. If there is a difference at all, I would say that explicitly specifying raw means that the user is aware what they are doing. So we would have more reason to warn against raw images if the backing format isn't specified at all because then the user might not be aware that they are using a backing file that probes as raw. > block/qcow.c | 16 ++++++++- > tests/qemu-iotests/290 | 72 ++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/290.out | 42 ++++++++++++++++++++++ > tests/qemu-iotests/group | 1 + > 4 files changed, 130 insertions(+), 1 deletion(-) > create mode 100755 tests/qemu-iotests/290 > create mode 100644 tests/qemu-iotests/290.out > > diff --git a/block/qcow.c b/block/qcow.c > index 8973e4e565a1..bbe48b7db4d7 100644 > --- a/block/qcow.c > +++ b/block/qcow.c > @@ -940,11 +940,12 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv, > { > BlockdevCreateOptions *create_options = NULL; > BlockDriverState *bs = NULL; > - QDict *qdict; > + QDict *qdict = NULL; > Visitor *v; > const char *val; > Error *local_err = NULL; > int ret; > + char *backing_fmt; > > static const QDictRenames opt_renames[] = { > { BLOCK_OPT_BACKING_FILE, "backing-file" }, > @@ -953,6 +954,13 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv, > }; > > /* Parse options and convert legacy syntax */ > + backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT); > + if (backing_fmt && !strcmp(backing_fmt, "raw")) { > + error_setg(errp, "qcow cannot store backing format; an explicit " > + "backing format of raw is unsafe"); Does this message tell that an implicit backing format of raw is safe? > + ret = -EINVAL; > + goto fail; > + } The commit message promises a warning. This is not a warning, but a hard error. > qdict = qemu_opts_to_qdict_filtered(opts, NULL, &qcow_create_opts, true); > > val = qdict_get_try_str(qdict, BLOCK_OPT_ENCRYPT); > @@ -1018,6 +1026,7 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv, > > ret = 0; > fail: > + g_free(backing_fmt); > qobject_unref(qdict); > bdrv_unref(bs); > qapi_free_BlockdevCreateOptions(create_options); > @@ -1152,6 +1161,11 @@ static QemuOptsList qcow_create_opts = { > .type = QEMU_OPT_STRING, > .help = "File name of a base image" > }, > + { > + .name = BLOCK_OPT_BACKING_FMT, > + .type = QEMU_OPT_STRING, > + .help = "Format of the backing image (caution: raw backing is unsafe)", > + }, > { > .name = BLOCK_OPT_ENCRYPT, > .type = QEMU_OPT_BOOL, > diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290 > new file mode 100755 > index 000000000000..8482de58cb4b > --- /dev/null > +++ b/tests/qemu-iotests/290 > @@ -0,0 +1,72 @@ > +#!/usr/bin/env bash > +# > +# Test qcow backing file warnings > +# > +# Copyright (C) 2020 Red Hat, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > +# > + > +seq=`basename $0` > +echo "QA output created by $seq" > + > +status=1 # failure is the default! > + > +_cleanup() > +{ > + _cleanup_test_img > +} > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +# get standard environment, filters and checks > +. ./common.rc > +. ./common.filter > + > +_supported_fmt qcow > +_supported_proto file > +_supported_os Linux > + > +size=128M > + > +echo > +echo "== qcow backed by qcow ==" > + > +TEST_IMG="$TEST_IMG.base" _make_test_img $size > +_make_test_img -b "$TEST_IMG.base" > +_img_info > +_make_test_img -b "$TEST_IMG.base" -F $IMGFMT > +_img_info > + > +echo > +echo "== mismatched command line detection ==" > + > +_make_test_img -b "$TEST_IMG.base" -F vmdk > +# Use of -u bypasses the backing format sanity check > +_make_test_img -u -b "$TEST_IMG.base" -F vmdk $size > +_img_info > + > +echo > +echo "== qcow backed by raw ==" > + > +rm "$TEST_IMG.base" > +truncate --size=$size "$TEST_IMG.base" > +_make_test_img -b "$TEST_IMG.base" > +_img_info > +_make_test_img -b "$TEST_IMG.base" -F raw > +_img_info This test doesn't tell the difference between a warning and an error. In both cases, the image would look the same: Either because it was successfully created or because the old version is still there. Kevin
On 5/5/20 2:35 AM, Kevin Wolf wrote: > Am 03.04.2020 um 19:58 hat Eric Blake geschrieben: >> qcow has no space in the metadata to store a backing format, and there >> are existing qcow images backed both by raw or by other formats >> (usually qcow) images, reliant on probing to tell the difference. >> While we don't recommend the creation of new qcow images (as qcow2 is >> hands-down better), we can at least insist that if the user does >> request a specific format without using -u, then it must be non-raw >> (as a raw backing file that gets inadvertently edited into some other >> format can form a security hole); if the user does not request a >> specific format or lies when using -u, then the status quo of probing >> for the backing format remains intact (although an upcoming patch will >> warn when omitting a format request). Thus, when this series is >> complete, the only way to use a backing file for qcow without >> triggering a warning is when using -F if the backing file is non-raw >> to begin with. Note that this is only for QemuOpts usage; there is no >> change to the QAPI to allow a format through -blockdev. >> >> Add a new iotest 290 just for qcow, to demonstrate the new warning. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> > > Somehow this feels backwards. Not specifying the backing file format at > all isn't any safer than explicitly specifying raw. > > If there is a difference at all, I would say that explicitly specifying > raw means that the user is aware what they are doing. So we would have > more reason to warn against raw images if the backing format isn't > specified at all because then the user might not be aware that they are > using a backing file that probes as raw. Prior to this patch, -F does not work with qcow. And even with this patch, we still cannot store the explicit value of -F in the qcow file. Anything that does not use -F must continue to work for now (although it may now warn, and in fact must warn if we deprecate it), while anything explicit is free to fail (since it failed already), but could also be made to work (if letting it work is nicer than making it fail, and where "work" may still include a warning, although it's pointless to have something brand new that works but is deprecated out of the box). So the following is my summary of the two options we can choose between: Option 1, qcow backed by raw is more common than qcow backed by other, so we want: raw <- qcow, no -F: work without warning (but if backing file is edited, a future probe seeing non-raw would break image) raw <- qcow, with -F: work without warning (but if backing file is edited, a future probe seeing non-raw would break image) other <- qcow, no -F: works but issues a warning (but backing file will always probe correctly) other <- qcow, with -F: fails (we cannot honor the user's explicit request, because we would still have to probe) Option 2, qcow backed by other is more common than qcow backed by raw, so we want: raw <- qcow, no -F: works but issues a warning (using a raw backing file without explicit buy-in is risky) raw <- qcow, with -F: works but issues a warning (explicit buy-in will still require subsequent probing, and a backing file could change which would break image) other <- qcow, no -F: works without warning other <- qcow, with -F: works without warning (later probing will still see non-raw) It looks like you are leaning more towards option 1, while my patch leaned more towards option 2. Anyone else want to chime in with an opinion on which is safer vs. easier? >> @@ -953,6 +954,13 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv, >> }; >> >> /* Parse options and convert legacy syntax */ >> + backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT); >> + if (backing_fmt && !strcmp(backing_fmt, "raw")) { >> + error_setg(errp, "qcow cannot store backing format; an explicit " >> + "backing format of raw is unsafe"); > > Does this message tell that an implicit backing format of raw is safe? If we go with option 2, are we trying to deprecate ALL use of raw as a backing file to qcow, regardless of whether the user was explicit about it? If we go with option 1, then we are instead deprecating any use of non-raw as a backing file to qcow. At the end of the day, we are trying to discourage users from creating new qcow files in the first place; qcow2 is much better. We still have to read existing qcow images with backing files, but maybe we want: Option 3: completely deprecate qcow images with backing files, as there is no safe way to do things favoring either raw (option 1) or non-raw (option 2), and therefore accept -F solely for convenience with the rest of the series, but always issue a warning regardless of whether -F was present. > >> + ret = -EINVAL; >> + goto fail; >> + } > > The commit message promises a warning. This is not a warning, but a hard > error. Once we decide which behavior we want, I'll make sure the commit message matches the behavior. Remember, for implicit images, we are starting a deprecation clock; for explicit images, it previously failed, so continuing to fail is still viable. >> +echo >> +echo "== qcow backed by raw ==" >> + >> +rm "$TEST_IMG.base" >> +truncate --size=$size "$TEST_IMG.base" >> +_make_test_img -b "$TEST_IMG.base" >> +_img_info >> +_make_test_img -b "$TEST_IMG.base" -F raw >> +_img_info > > This test doesn't tell the difference between a warning and an error. In > both cases, the image would look the same: Either because it was > successfully created or because the old version is still there. Indeed, I can make the test more robust. But more importantly, we need consensus on _what_ we want behavior to be, and write the test along those lines.
On 5/5/20 10:30 AM, Eric Blake wrote: > On 5/5/20 2:35 AM, Kevin Wolf wrote: >> Am 03.04.2020 um 19:58 hat Eric Blake geschrieben: >>> qcow has no space in the metadata to store a backing format, and there >>> are existing qcow images backed both by raw or by other formats >>> (usually qcow) images, reliant on probing to tell the difference. >>> While we don't recommend the creation of new qcow images (as qcow2 is >>> hands-down better), we can at least insist that if the user does >>> request a specific format without using -u, then it must be non-raw >>> (as a raw backing file that gets inadvertently edited into some other >>> format can form a security hole); if the user does not request a >>> specific format or lies when using -u, then the status quo of probing >>> for the backing format remains intact (although an upcoming patch will >>> warn when omitting a format request). Thus, when this series is >>> complete, the only way to use a backing file for qcow without >>> triggering a warning is when using -F if the backing file is non-raw >>> to begin with. Note that this is only for QemuOpts usage; there is no >>> change to the QAPI to allow a format through -blockdev. >>> >>> Add a new iotest 290 just for qcow, to demonstrate the new warning. >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> Somehow this feels backwards. Not specifying the backing file format at >> all isn't any safer than explicitly specifying raw. >> >> If there is a difference at all, I would say that explicitly specifying >> raw means that the user is aware what they are doing. So we would have >> more reason to warn against raw images if the backing format isn't >> specified at all because then the user might not be aware that they are >> using a backing file that probes as raw. > > Prior to this patch, -F does not work with qcow. And even with this > patch, we still cannot store the explicit value of -F in the qcow file. > Anything that does not use -F must continue to work for now (although it > may now warn, and in fact must warn if we deprecate it), while anything > explicit is free to fail (since it failed already), but could also be > made to work (if letting it work is nicer than making it fail, and where > "work" may still include a warning, although it's pointless to have > something brand new that works but is deprecated out of the box). So > the following is my summary of the two options we can choose between: > > Option 1, qcow backed by raw is more common than qcow backed by other, > so we want: > raw <- qcow, no -F: work without warning (but if backing file is edited, > a future probe seeing non-raw would break image) > raw <- qcow, with -F: work without warning (but if backing file is > edited, a future probe seeing non-raw would break image) > other <- qcow, no -F: works but issues a warning (but backing file will > always probe correctly) > other <- qcow, with -F: fails (we cannot honor the user's explicit > request, because we would still have to probe) > > Option 2, qcow backed by other is more common than qcow backed by raw, > so we want: > raw <- qcow, no -F: works but issues a warning (using a raw backing file > without explicit buy-in is risky) > raw <- qcow, with -F: works but issues a warning (explicit buy-in will > still require subsequent probing, and a backing file could change which > would break image) > other <- qcow, no -F: works without warning > other <- qcow, with -F: works without warning (later probing will still > see non-raw) > > It looks like you are leaning more towards option 1, while my patch > leaned more towards option 2. Anyone else want to chime in with an > opinion on which is safer vs. easier? > Option 3: > completely deprecate qcow images with backing files, as there is no safe > way to do things favoring either raw (option 1) or non-raw (option 2), > and therefore accept -F solely for convenience with the rest of the > series, but always issue a warning regardless of whether -F was present. Hearing no other opinion in the meantime, I've come up with option 4: raw <- qcow, no -F: works but issues a warning to use -F (the user should be explicit that they know they are using raw) raw <- qcow, with -F raw: a probe is attempted, if it returns anything other than raw, then fail (since we can't store the backing type, and the user's explicit type didn't match reality); otherwise works without warning (users tend to treat backing files as read-only, so even though editing a backing file could make it appear non-raw, that's less likely to happen) other <- qcow, no -F: works without warning (we'll probe in future opens, but the probe will see the same file type and not corrupt user data) other <- qcow, with -F: a probe is attempted and must match, but otherwise works without warning (we'll still have to probe in future opens, but it's no worse than before)
Am 22.06.2020 um 23:58 hat Eric Blake geschrieben: > On 5/5/20 10:30 AM, Eric Blake wrote: > > On 5/5/20 2:35 AM, Kevin Wolf wrote: > > > Am 03.04.2020 um 19:58 hat Eric Blake geschrieben: > > > > qcow has no space in the metadata to store a backing format, and there > > > > are existing qcow images backed both by raw or by other formats > > > > (usually qcow) images, reliant on probing to tell the difference. > > > > While we don't recommend the creation of new qcow images (as qcow2 is > > > > hands-down better), we can at least insist that if the user does > > > > request a specific format without using -u, then it must be non-raw > > > > (as a raw backing file that gets inadvertently edited into some other > > > > format can form a security hole); if the user does not request a > > > > specific format or lies when using -u, then the status quo of probing > > > > for the backing format remains intact (although an upcoming patch will > > > > warn when omitting a format request). Thus, when this series is > > > > complete, the only way to use a backing file for qcow without > > > > triggering a warning is when using -F if the backing file is non-raw > > > > to begin with. Note that this is only for QemuOpts usage; there is no > > > > change to the QAPI to allow a format through -blockdev. > > > > > > > > Add a new iotest 290 just for qcow, to demonstrate the new warning. > > > > > > > > Signed-off-by: Eric Blake <eblake@redhat.com> > > > > > > Somehow this feels backwards. Not specifying the backing file format at > > > all isn't any safer than explicitly specifying raw. > > > > > > If there is a difference at all, I would say that explicitly specifying > > > raw means that the user is aware what they are doing. So we would have > > > more reason to warn against raw images if the backing format isn't > > > specified at all because then the user might not be aware that they are > > > using a backing file that probes as raw. > > > > Prior to this patch, -F does not work with qcow. And even with this > > patch, we still cannot store the explicit value of -F in the qcow file. > > Anything that does not use -F must continue to work for now (although it > > may now warn, and in fact must warn if we deprecate it), while anything > > explicit is free to fail (since it failed already), but could also be > > made to work (if letting it work is nicer than making it fail, and where > > "work" may still include a warning, although it's pointless to have > > something brand new that works but is deprecated out of the box). So > > the following is my summary of the two options we can choose between: > > > > Option 1, qcow backed by raw is more common than qcow backed by other, > > so we want: > > raw <- qcow, no -F: work without warning (but if backing file is edited, > > a future probe seeing non-raw would break image) > > raw <- qcow, with -F: work without warning (but if backing file is > > edited, a future probe seeing non-raw would break image) > > other <- qcow, no -F: works but issues a warning (but backing file will > > always probe correctly) > > other <- qcow, with -F: fails (we cannot honor the user's explicit > > request, because we would still have to probe) > > > > Option 2, qcow backed by other is more common than qcow backed by raw, > > so we want: > > raw <- qcow, no -F: works but issues a warning (using a raw backing file > > without explicit buy-in is risky) > > raw <- qcow, with -F: works but issues a warning (explicit buy-in will > > still require subsequent probing, and a backing file could change which > > would break image) > > other <- qcow, no -F: works without warning > > other <- qcow, with -F: works without warning (later probing will still > > see non-raw) > > > > It looks like you are leaning more towards option 1, while my patch > > leaned more towards option 2. Anyone else want to chime in with an > > opinion on which is safer vs. easier? > > > Option 3: > > completely deprecate qcow images with backing files, as there is no safe > > way to do things favoring either raw (option 1) or non-raw (option 2), > > and therefore accept -F solely for convenience with the rest of the > > series, but always issue a warning regardless of whether -F was present. > > > Hearing no other opinion in the meantime, I've come up with option 4: > > raw <- qcow, no -F: works but issues a warning to use -F (the user should be > explicit that they know they are using raw) > raw <- qcow, with -F raw: a probe is attempted, if it returns anything other > than raw, then fail (since we can't store the backing type, and the user's > explicit type didn't match reality); otherwise works without warning (users > tend to treat backing files as read-only, so even though editing a backing > file could make it appear non-raw, that's less likely to happen) Actually, even for a backing file, I think bs->probed should be set, so the raw driver would return an I/O error if you write the magic of an image format to the first sector. We should just add a test case to verify this behaviour for backing files (e.g. in the context of a commit job). Of course, if you edit the backing file outside of QEMU, that's your problem. > other <- qcow, no -F: works without warning (we'll probe in future opens, > but the probe will see the same file type and not corrupt user data) > other <- qcow, with -F: a probe is attempted and must match, but otherwise > works without warning (we'll still have to probe in future opens, but it's > no worse than before) This plan makes sense to me. Kevin
diff --git a/block/qcow.c b/block/qcow.c index 8973e4e565a1..bbe48b7db4d7 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -940,11 +940,12 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv, { BlockdevCreateOptions *create_options = NULL; BlockDriverState *bs = NULL; - QDict *qdict; + QDict *qdict = NULL; Visitor *v; const char *val; Error *local_err = NULL; int ret; + char *backing_fmt; static const QDictRenames opt_renames[] = { { BLOCK_OPT_BACKING_FILE, "backing-file" }, @@ -953,6 +954,13 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv, }; /* Parse options and convert legacy syntax */ + backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT); + if (backing_fmt && !strcmp(backing_fmt, "raw")) { + error_setg(errp, "qcow cannot store backing format; an explicit " + "backing format of raw is unsafe"); + ret = -EINVAL; + goto fail; + } qdict = qemu_opts_to_qdict_filtered(opts, NULL, &qcow_create_opts, true); val = qdict_get_try_str(qdict, BLOCK_OPT_ENCRYPT); @@ -1018,6 +1026,7 @@ static int coroutine_fn qcow_co_create_opts(BlockDriver *drv, ret = 0; fail: + g_free(backing_fmt); qobject_unref(qdict); bdrv_unref(bs); qapi_free_BlockdevCreateOptions(create_options); @@ -1152,6 +1161,11 @@ static QemuOptsList qcow_create_opts = { .type = QEMU_OPT_STRING, .help = "File name of a base image" }, + { + .name = BLOCK_OPT_BACKING_FMT, + .type = QEMU_OPT_STRING, + .help = "Format of the backing image (caution: raw backing is unsafe)", + }, { .name = BLOCK_OPT_ENCRYPT, .type = QEMU_OPT_BOOL, diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290 new file mode 100755 index 000000000000..8482de58cb4b --- /dev/null +++ b/tests/qemu-iotests/290 @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# +# Test qcow backing file warnings +# +# Copyright (C) 2020 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +seq=`basename $0` +echo "QA output created by $seq" + +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt qcow +_supported_proto file +_supported_os Linux + +size=128M + +echo +echo "== qcow backed by qcow ==" + +TEST_IMG="$TEST_IMG.base" _make_test_img $size +_make_test_img -b "$TEST_IMG.base" +_img_info +_make_test_img -b "$TEST_IMG.base" -F $IMGFMT +_img_info + +echo +echo "== mismatched command line detection ==" + +_make_test_img -b "$TEST_IMG.base" -F vmdk +# Use of -u bypasses the backing format sanity check +_make_test_img -u -b "$TEST_IMG.base" -F vmdk $size +_img_info + +echo +echo "== qcow backed by raw ==" + +rm "$TEST_IMG.base" +truncate --size=$size "$TEST_IMG.base" +_make_test_img -b "$TEST_IMG.base" +_img_info +_make_test_img -b "$TEST_IMG.base" -F raw +_img_info + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/290.out b/tests/qemu-iotests/290.out new file mode 100644 index 000000000000..5b4ae3196d04 --- /dev/null +++ b/tests/qemu-iotests/290.out @@ -0,0 +1,42 @@ +QA output created by 290 + +== qcow backed by qcow == +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 128 MiB (134217728 bytes) +cluster_size: 512 +backing file: TEST_DIR/t.IMGFMT.base +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 128 MiB (134217728 bytes) +cluster_size: 512 +backing file: TEST_DIR/t.IMGFMT.base + +== mismatched command line detection == +qemu-img: TEST_DIR/t.IMGFMT: invalid VMDK image descriptor +Could not open backing image to determine size. +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=vmdk +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 128 MiB (134217728 bytes) +cluster_size: 512 +backing file: TEST_DIR/t.IMGFMT.base + +== qcow backed by raw == +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 128 MiB (134217728 bytes) +cluster_size: 512 +backing file: TEST_DIR/t.IMGFMT.base +qemu-img: TEST_DIR/t.IMGFMT: IMGFMT cannot store backing format; an explicit backing format of raw is unsafe +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 128 MiB (134217728 bytes) +cluster_size: 512 +backing file: TEST_DIR/t.IMGFMT.base +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 79c6dfc85d1b..c4dc86a9cf73 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -296,3 +296,4 @@ 286 rw quick 288 quick 289 rw quick +290 backing quick
qcow has no space in the metadata to store a backing format, and there are existing qcow images backed both by raw or by other formats (usually qcow) images, reliant on probing to tell the difference. While we don't recommend the creation of new qcow images (as qcow2 is hands-down better), we can at least insist that if the user does request a specific format without using -u, then it must be non-raw (as a raw backing file that gets inadvertently edited into some other format can form a security hole); if the user does not request a specific format or lies when using -u, then the status quo of probing for the backing format remains intact (although an upcoming patch will warn when omitting a format request). Thus, when this series is complete, the only way to use a backing file for qcow without triggering a warning is when using -F if the backing file is non-raw to begin with. Note that this is only for QemuOpts usage; there is no change to the QAPI to allow a format through -blockdev. Add a new iotest 290 just for qcow, to demonstrate the new warning. Signed-off-by: Eric Blake <eblake@redhat.com> --- block/qcow.c | 16 ++++++++- tests/qemu-iotests/290 | 72 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/290.out | 42 ++++++++++++++++++++++ tests/qemu-iotests/group | 1 + 4 files changed, 130 insertions(+), 1 deletion(-) create mode 100755 tests/qemu-iotests/290 create mode 100644 tests/qemu-iotests/290.out