* [PATCH 0/2] block: Fix multiple blockdev-snapshot calls
@ 2019-11-08 8:53 Kevin Wolf
2019-11-08 8:53 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_, }options Kevin Wolf
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-11-08 8:53 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, pkrempa, qemu-devel, mreitz
Kevin Wolf (2):
block: Remove 'backing': null from bs->{explicit_,}options
iotests: Test multiple blockdev-snapshot calls
block.c | 2 +
tests/qemu-iotests/273 | 76 +++++++++
tests/qemu-iotests/273.out | 337 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
4 files changed, 416 insertions(+)
create mode 100755 tests/qemu-iotests/273
create mode 100644 tests/qemu-iotests/273.out
--
2.20.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_, }options 2019-11-08 8:53 [PATCH 0/2] block: Fix multiple blockdev-snapshot calls Kevin Wolf @ 2019-11-08 8:53 ` Kevin Wolf 2019-11-08 14:30 ` Alberto Garcia 2019-11-12 16:01 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_,}options Peter Krempa 2019-11-08 8:53 ` [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls Kevin Wolf 2019-11-18 16:26 ` [PATCH 0/2] block: Fix " Peter Krempa 2 siblings, 2 replies; 12+ messages in thread From: Kevin Wolf @ 2019-11-08 8:53 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, pkrempa, qemu-devel, mreitz bs->options and bs->explicit_options shouldn't contain any options for child nodes. bdrv_open_inherited() takes care to remove any options that match a child name after opening the image and the same is done when reopening. However, we miss the case of 'backing': null, which is a child option, but results in no child being created. This means that a 'backing': null remains in bs->options and bs->explicit_options. A typical use for 'backing': null is in live snapshots: blockdev-add for the qcow2 overlay makes sure not to open the backing file (because it is already opened and blockdev-snapshot will attach it). After doing a blockdev-snapshot, bs->options and bs->explicit_options become inconsistent with the actual state (bs has a backing file now, but the options still say null). On the next occasion that the image is reopened, e.g. switching it from read-write to read-only when another snapshot is taken, the option will take effect again and the node incorrectly loses its backing file. Fix bdrv_open_inherited() to remove the 'backing' option from bs->options and bs->explicit_options even for the case where it specifies that no backing file is wanted. Reported-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block.c b/block.c index dad5a3d8e0..74ba9acb08 100644 --- a/block.c +++ b/block.c @@ -3019,6 +3019,8 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, "use \"backing\": null instead"); } flags |= BDRV_O_NO_BACKING; + qdict_del(bs->explicit_options, "backing"); + qdict_del(bs->options, "backing"); qdict_del(options, "backing"); } -- 2.20.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_, }options 2019-11-08 8:53 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_, }options Kevin Wolf @ 2019-11-08 14:30 ` Alberto Garcia 2019-11-12 16:01 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_,}options Peter Krempa 1 sibling, 0 replies; 12+ messages in thread From: Alberto Garcia @ 2019-11-08 14:30 UTC (permalink / raw) To: Kevin Wolf, qemu-block; +Cc: kwolf, qemu-devel, mreitz On Fri 08 Nov 2019 09:53:11 AM CET, Kevin Wolf wrote: > bs->options and bs->explicit_options shouldn't contain any options for > child nodes. bdrv_open_inherited() takes care to remove any options that > match a child name after opening the image and the same is done when > reopening. > > However, we miss the case of 'backing': null, which is a child option, > but results in no child being created. This means that a 'backing': null > remains in bs->options and bs->explicit_options. > > A typical use for 'backing': null is in live snapshots: blockdev-add for > the qcow2 overlay makes sure not to open the backing file (because it is > already opened and blockdev-snapshot will attach it). After doing a > blockdev-snapshot, bs->options and bs->explicit_options become > inconsistent with the actual state (bs has a backing file now, but the > options still say null). On the next occasion that the image is > reopened, e.g. switching it from read-write to read-only when another > snapshot is taken, the option will take effect again and the node > incorrectly loses its backing file. > > Fix bdrv_open_inherited() to remove the 'backing' option from > bs->options and bs->explicit_options even for the case where it > specifies that no backing file is wanted. > > Reported-by: Peter Krempa <pkrempa@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Berto ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_,}options 2019-11-08 8:53 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_, }options Kevin Wolf 2019-11-08 14:30 ` Alberto Garcia @ 2019-11-12 16:01 ` Peter Krempa 1 sibling, 0 replies; 12+ messages in thread From: Peter Krempa @ 2019-11-12 16:01 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, qemu-block, mreitz On Fri, Nov 08, 2019 at 09:53:11 +0100, Kevin Wolf wrote: > bs->options and bs->explicit_options shouldn't contain any options for > child nodes. bdrv_open_inherited() takes care to remove any options that > match a child name after opening the image and the same is done when > reopening. > > However, we miss the case of 'backing': null, which is a child option, > but results in no child being created. This means that a 'backing': null > remains in bs->options and bs->explicit_options. > > A typical use for 'backing': null is in live snapshots: blockdev-add for > the qcow2 overlay makes sure not to open the backing file (because it is Note that we also use '"backing": null' as a terminator for the last image in the chain if the user configures the chain manually. This is kind-of a protection from opening the backing file from the header if it was misconfigured somehow. I think this functionality should be kept despite probably not making practical sense. In my testing this scenario worked properly. > already opened and blockdev-snapshot will attach it). After doing a > blockdev-snapshot, bs->options and bs->explicit_options become > inconsistent with the actual state (bs has a backing file now, but the > options still say null). On the next occasion that the image is > reopened, e.g. switching it from read-write to read-only when another > snapshot is taken, the option will take effect again and the node > incorrectly loses its backing file. > > Fix bdrv_open_inherited() to remove the 'backing' option from > bs->options and bs->explicit_options even for the case where it > specifies that no backing file is wanted. > > Reported-by: Peter Krempa <pkrempa@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> The fix looks sane-enough to me and works as expected, but since I'm not familiar enough with this code I'm comfortable only with a: Tested-by: Peter Krempa <pkrempa@redhat.com> > --- > block.c | 2 ++ > 1 file changed, 2 insertions(+) ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls 2019-11-08 8:53 [PATCH 0/2] block: Fix multiple blockdev-snapshot calls Kevin Wolf 2019-11-08 8:53 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_, }options Kevin Wolf @ 2019-11-08 8:53 ` Kevin Wolf 2019-11-08 14:32 ` Alberto Garcia ` (2 more replies) 2019-11-18 16:26 ` [PATCH 0/2] block: Fix " Peter Krempa 2 siblings, 3 replies; 12+ messages in thread From: Kevin Wolf @ 2019-11-08 8:53 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, pkrempa, qemu-devel, mreitz Test that doing a second blockdev-snapshot doesn't make the first overlay's backing file go away. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- tests/qemu-iotests/273 | 76 +++++++++ tests/qemu-iotests/273.out | 337 +++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 414 insertions(+) create mode 100755 tests/qemu-iotests/273 create mode 100644 tests/qemu-iotests/273.out diff --git a/tests/qemu-iotests/273 b/tests/qemu-iotests/273 new file mode 100755 index 0000000000..60076de7ce --- /dev/null +++ b/tests/qemu-iotests/273 @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# +# Test large write to a qcow2 image +# +# Copyright (C) 2019 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 + +# This is a qcow2 regression test +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +do_run_qemu() +{ + echo Testing: "$@" + $QEMU -nographic -qmp-pretty stdio -nodefaults "$@" + echo +} + +run_qemu() +{ + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_qmp | + _filter_generated_node_ids | _filter_imgfmt | _filter_actual_image_size +} + +TEST_IMG="$TEST_IMG.base" _make_test_img 64M +TEST_IMG="$TEST_IMG.mid" _make_test_img -b "$TEST_IMG.base" +_make_test_img -b "$TEST_IMG.mid" + +run_qemu \ + -blockdev file,node-name=base,filename="$TEST_IMG.base" \ + -blockdev file,node-name=midf,filename="$TEST_IMG.mid" \ + -blockdev '{"driver":"qcow2","node-name":"mid","file":"midf","backing":null}' \ + -blockdev file,node-name=topf,filename="$TEST_IMG" \ + -blockdev '{"driver":"qcow2","file":"topf","node-name":"top","backing":null}' \ +<<EOF +{"execute":"qmp_capabilities"} +{"execute":"blockdev-snapshot","arguments":{"node":"base","overlay":"mid"}} +{"execute":"blockdev-snapshot","arguments":{"node":"mid","overlay":"top"}} +{"execute":"query-named-block-nodes"} +{"execute":"x-debug-query-block-graph"} +{"execute":"quit"} +EOF + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/273.out b/tests/qemu-iotests/273.out new file mode 100644 index 0000000000..c410fee5c4 --- /dev/null +++ b/tests/qemu-iotests/273.out @@ -0,0 +1,337 @@ +QA output created by 273 +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', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.mid +Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev file,node-name=midf,filename=TEST_DIR/t.IMGFMT.mid -blockdev {"driver":"IMGFMT","node-name":"mid","file":"midf","backing":null} -blockdev file,node-name=topf,filename=TEST_DIR/t.IMGFMT -blockdev {"driver":"IMGFMT","file":"topf","node-name":"top","backing":null} +{ + QMP_VERSION +} +{ + "return": { + } +} +{ + "return": { + } +} +{ + "return": { + } +} +{ + "return": [ + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "backing-image": { + "backing-image": { + "virtual-size": 197120, + "filename": "TEST_DIR/t.IMGFMT.base", + "format": "file", + "actual-size": SIZE, + "dirty-flag": false + }, + "backing-filename-format": "file", + "virtual-size": 67108864, + "filename": "TEST_DIR/t.IMGFMT.mid", + "cluster-size": 65536, + "format": "IMGFMT", + "actual-size": SIZE, + "format-specific": { + "type": "IMGFMT", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "TEST_DIR/t.IMGFMT.base", + "backing-filename": "TEST_DIR/t.IMGFMT.base", + "dirty-flag": false + }, + "backing-filename-format": "IMGFMT", + "virtual-size": 67108864, + "filename": "TEST_DIR/t.IMGFMT", + "cluster-size": 65536, + "format": "IMGFMT", + "actual-size": SIZE, + "format-specific": { + "type": "IMGFMT", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "TEST_DIR/t.IMGFMT.mid", + "backing-filename": "TEST_DIR/t.IMGFMT.mid", + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "top", + "backing_file_depth": 2, + "drv": "IMGFMT", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "backing_file": "TEST_DIR/t.IMGFMT.mid", + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "TEST_DIR/t.IMGFMT", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 197120, + "filename": "TEST_DIR/t.IMGFMT", + "format": "file", + "actual-size": SIZE, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "topf", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "TEST_DIR/t.IMGFMT", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "backing-image": { + "virtual-size": 197120, + "filename": "TEST_DIR/t.IMGFMT.base", + "format": "file", + "actual-size": SIZE, + "dirty-flag": false + }, + "backing-filename-format": "file", + "virtual-size": 67108864, + "filename": "TEST_DIR/t.IMGFMT.mid", + "cluster-size": 65536, + "format": "IMGFMT", + "actual-size": SIZE, + "format-specific": { + "type": "IMGFMT", + "data": { + "compat": "1.1", + "lazy-refcounts": false, + "refcount-bits": 16, + "corrupt": false + } + }, + "full-backing-filename": "TEST_DIR/t.IMGFMT.base", + "backing-filename": "TEST_DIR/t.IMGFMT.base", + "dirty-flag": false + }, + "iops_wr": 0, + "ro": true, + "node-name": "mid", + "backing_file_depth": 1, + "drv": "IMGFMT", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "backing_file": "TEST_DIR/t.IMGFMT.base", + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "TEST_DIR/t.IMGFMT.mid", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 197120, + "filename": "TEST_DIR/t.IMGFMT.mid", + "format": "file", + "actual-size": SIZE, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "node-name": "midf", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "TEST_DIR/t.IMGFMT.mid", + "encryption_key_missing": false + }, + { + "iops_rd": 0, + "detect_zeroes": "off", + "image": { + "virtual-size": 197120, + "filename": "TEST_DIR/t.IMGFMT.base", + "format": "file", + "actual-size": SIZE, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": true, + "node-name": "base", + "backing_file_depth": 0, + "drv": "file", + "iops": 0, + "bps_wr": 0, + "write_threshold": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "cache": { + "no-flush": false, + "direct": false, + "writeback": true + }, + "file": "TEST_DIR/t.IMGFMT.base", + "encryption_key_missing": false + } + ] +} +{ + "return": { + "edges": [ + { + "name": "file", + "parent": 5, + "shared-perm": [ + "graph-mod", + "write-unchanged", + "consistent-read" + ], + "perm": [ + "resize", + "write", + "consistent-read" + ], + "child": 4 + }, + { + "name": "backing", + "parent": 5, + "shared-perm": [ + "graph-mod", + "resize", + "write-unchanged", + "write", + "consistent-read" + ], + "perm": [ + ], + "child": 3 + }, + { + "name": "file", + "parent": 3, + "shared-perm": [ + "graph-mod", + "write-unchanged", + "consistent-read" + ], + "perm": [ + "consistent-read" + ], + "child": 2 + }, + { + "name": "backing", + "parent": 3, + "shared-perm": [ + "graph-mod", + "resize", + "write-unchanged", + "write", + "consistent-read" + ], + "perm": [ + ], + "child": 1 + } + ], + "nodes": [ + { + "name": "top", + "type": "block-driver", + "id": 5 + }, + { + "name": "topf", + "type": "block-driver", + "id": 4 + }, + { + "name": "mid", + "type": "block-driver", + "id": 3 + }, + { + "name": "midf", + "type": "block-driver", + "id": 2 + }, + { + "name": "base", + "type": "block-driver", + "id": 1 + } + ] + } +} +{ + "return": { + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "SHUTDOWN", + "data": { + "guest": false, + "reason": "host-qmp-quit" + } +} + +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index af322af756..5aabdfefde 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -282,3 +282,4 @@ 267 rw auto quick snapshot 268 rw auto quick 270 rw backing quick +273 backing quick -- 2.20.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls 2019-11-08 8:53 ` [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls Kevin Wolf @ 2019-11-08 14:32 ` Alberto Garcia 2019-11-13 9:28 ` Kevin Wolf 2019-11-12 16:07 ` Peter Krempa 2019-11-18 18:03 ` Max Reitz 2 siblings, 1 reply; 12+ messages in thread From: Alberto Garcia @ 2019-11-08 14:32 UTC (permalink / raw) To: Kevin Wolf, qemu-block; +Cc: kwolf, qemu-devel, mreitz On Fri 08 Nov 2019 09:53:12 AM CET, Kevin Wolf wrote: > +# Test large write to a qcow2 image This doesn't belong here I guess :) I wonder if this test could go in 245 instead. Berto ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls 2019-11-08 14:32 ` Alberto Garcia @ 2019-11-13 9:28 ` Kevin Wolf 0 siblings, 0 replies; 12+ messages in thread From: Kevin Wolf @ 2019-11-13 9:28 UTC (permalink / raw) To: Alberto Garcia; +Cc: qemu-devel, qemu-block, mreitz Am 08.11.2019 um 15:32 hat Alberto Garcia geschrieben: > On Fri 08 Nov 2019 09:53:12 AM CET, Kevin Wolf wrote: > > +# Test large write to a qcow2 image > > This doesn't belong here I guess :) Yes, fixed. > I wonder if this test could go in 245 instead. The headline for 245 is "Test cases for the QMP 'x-blockdev-reopen' command", so I don't think so? We end up testing the reopen code, but only as a side effect of snapshotting, not through x-blockdev-reopen. Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls 2019-11-08 8:53 ` [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls Kevin Wolf 2019-11-08 14:32 ` Alberto Garcia @ 2019-11-12 16:07 ` Peter Krempa 2019-11-13 9:25 ` Kevin Wolf 2019-11-18 18:03 ` Max Reitz 2 siblings, 1 reply; 12+ messages in thread From: Peter Krempa @ 2019-11-12 16:07 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, qemu-block, mreitz On Fri, Nov 08, 2019 at 09:53:12 +0100, Kevin Wolf wrote: > Test that doing a second blockdev-snapshot doesn't make the first > overlay's backing file go away. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/273 | 76 +++++++++ > tests/qemu-iotests/273.out | 337 +++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/group | 1 + > 3 files changed, 414 insertions(+) > create mode 100755 tests/qemu-iotests/273 > create mode 100644 tests/qemu-iotests/273.out Didn't apply cleanly for me. > > diff --git a/tests/qemu-iotests/273 b/tests/qemu-iotests/273 > new file mode 100755 > index 0000000000..60076de7ce > --- /dev/null > +++ b/tests/qemu-iotests/273 > @@ -0,0 +1,76 @@ > +#!/usr/bin/env bash > +# > +# Test large write to a qcow2 image Cut&paste? Rest looks good Reviewed-by: Peter Krempa <pkrempa@redhat.com> > +# > +# Copyright (C) 2019 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 > + > +# This is a qcow2 regression test > +_supported_fmt qcow2 > +_supported_proto file > +_supported_os Linux > + > +do_run_qemu() > +{ > + echo Testing: "$@" > + $QEMU -nographic -qmp-pretty stdio -nodefaults "$@" -qmp-pretty, that's useful > + echo > +} > + > +run_qemu() > +{ > + do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_qmp | > + _filter_generated_node_ids | _filter_imgfmt | _filter_actual_image_size > +} > + > +TEST_IMG="$TEST_IMG.base" _make_test_img 64M > +TEST_IMG="$TEST_IMG.mid" _make_test_img -b "$TEST_IMG.base" > +_make_test_img -b "$TEST_IMG.mid" > + > +run_qemu \ > + -blockdev file,node-name=base,filename="$TEST_IMG.base" \ > + -blockdev file,node-name=midf,filename="$TEST_IMG.mid" \ > + -blockdev '{"driver":"qcow2","node-name":"mid","file":"midf","backing":null}' \ > + -blockdev file,node-name=topf,filename="$TEST_IMG" \ > + -blockdev '{"driver":"qcow2","file":"topf","node-name":"top","backing":null}' \ > +<<EOF > +{"execute":"qmp_capabilities"} > +{"execute":"blockdev-snapshot","arguments":{"node":"base","overlay":"mid"}} > +{"execute":"blockdev-snapshot","arguments":{"node":"mid","overlay":"top"}} > +{"execute":"query-named-block-nodes"} > +{"execute":"x-debug-query-block-graph"} Oh, this too! > +{"execute":"quit"} > +EOF > + > +# success, all done > +echo "*** done" > +rm -f $seq.full > +status=0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls 2019-11-12 16:07 ` Peter Krempa @ 2019-11-13 9:25 ` Kevin Wolf 0 siblings, 0 replies; 12+ messages in thread From: Kevin Wolf @ 2019-11-13 9:25 UTC (permalink / raw) To: Peter Krempa; +Cc: qemu-devel, qemu-block, mreitz Am 12.11.2019 um 17:07 hat Peter Krempa geschrieben: > On Fri, Nov 08, 2019 at 09:53:12 +0100, Kevin Wolf wrote: > > Test that doing a second blockdev-snapshot doesn't make the first > > overlay's backing file go away. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > tests/qemu-iotests/273 | 76 +++++++++ > > tests/qemu-iotests/273.out | 337 +++++++++++++++++++++++++++++++++++++ > > tests/qemu-iotests/group | 1 + > > 3 files changed, 414 insertions(+) > > create mode 100755 tests/qemu-iotests/273 > > create mode 100644 tests/qemu-iotests/273.out > > Didn't apply cleanly for me. > > > > > diff --git a/tests/qemu-iotests/273 b/tests/qemu-iotests/273 > > new file mode 100755 > > index 0000000000..60076de7ce > > --- /dev/null > > +++ b/tests/qemu-iotests/273 > > @@ -0,0 +1,76 @@ > > +#!/usr/bin/env bash > > +# > > +# Test large write to a qcow2 image > > Cut&paste? Indeed. I'll change it like this: # Test multiple blockdev-snapshot calls with 'backing': null > Rest looks good > > Reviewed-by: Peter Krempa <pkrempa@redhat.com> Thanks! Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls 2019-11-08 8:53 ` [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls Kevin Wolf 2019-11-08 14:32 ` Alberto Garcia 2019-11-12 16:07 ` Peter Krempa @ 2019-11-18 18:03 ` Max Reitz 2 siblings, 0 replies; 12+ messages in thread From: Max Reitz @ 2019-11-18 18:03 UTC (permalink / raw) To: Kevin Wolf, qemu-block; +Cc: pkrempa, qemu-devel [-- Attachment #1.1: Type: text/plain, Size: 920 bytes --] On 08.11.19 09:53, Kevin Wolf wrote: > Test that doing a second blockdev-snapshot doesn't make the first > overlay's backing file go away. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > tests/qemu-iotests/273 | 76 +++++++++ > tests/qemu-iotests/273.out | 337 +++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/group | 1 + > 3 files changed, 414 insertions(+) > create mode 100755 tests/qemu-iotests/273 > create mode 100644 tests/qemu-iotests/273.out [...] > + "format-specific": { > + "type": "IMGFMT", > + "data": { > + "compat": "1.1", > + "lazy-refcounts": false, > + "refcount-bits": 16, > + "corrupt": false > + } I get a mismatch here with -o compat=0.10 or -o refcount_bits=1. Max [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] block: Fix multiple blockdev-snapshot calls 2019-11-08 8:53 [PATCH 0/2] block: Fix multiple blockdev-snapshot calls Kevin Wolf 2019-11-08 8:53 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_, }options Kevin Wolf 2019-11-08 8:53 ` [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls Kevin Wolf @ 2019-11-18 16:26 ` Peter Krempa 2019-11-18 16:49 ` Kevin Wolf 2 siblings, 1 reply; 12+ messages in thread From: Peter Krempa @ 2019-11-18 16:26 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, qemu-block, mreitz [-- Attachment #1: Type: text/plain, Size: 646 bytes --] On Fri, Nov 08, 2019 at 09:53:10 +0100, Kevin Wolf wrote: > Kevin Wolf (2): > block: Remove 'backing': null from bs->{explicit_,}options > iotests: Test multiple blockdev-snapshot calls Hi, I'm not sure how the freeze rules for qemu are at this point thus: Will this patchset make it into 4.2. I argue it's a pretty important fix as it ends up in image corruption. If it won't make into we will need to add a QMP feature flag to detect presence of the fix so that I can gate libvirt's support of blockdev with it. (This can also be done if we make it into 4.2 but I'll be okay infering it from other fixes present in 4.2). Thanks Peter [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] block: Fix multiple blockdev-snapshot calls 2019-11-18 16:26 ` [PATCH 0/2] block: Fix " Peter Krempa @ 2019-11-18 16:49 ` Kevin Wolf 0 siblings, 0 replies; 12+ messages in thread From: Kevin Wolf @ 2019-11-18 16:49 UTC (permalink / raw) To: Peter Krempa; +Cc: qemu-devel, qemu-block, mreitz [-- Attachment #1: Type: text/plain, Size: 834 bytes --] Am 18.11.2019 um 17:26 hat Peter Krempa geschrieben: > On Fri, Nov 08, 2019 at 09:53:10 +0100, Kevin Wolf wrote: > > Kevin Wolf (2): > > block: Remove 'backing': null from bs->{explicit_,}options > > iotests: Test multiple blockdev-snapshot calls > > Hi, > > I'm not sure how the freeze rules for qemu are at this point thus: > > Will this patchset make it into 4.2. I argue it's a pretty important fix > as it ends up in image corruption. > > If it won't make into we will need to add a QMP feature flag to detect > presence of the fix so that I can gate libvirt's support of blockdev > with it. (This can also be done if we make it into 4.2 but I'll be okay > infering it from other fixes present in 4.2). Yes, this is planned for 4.2. I'm in the process of preparing a pull request right now. Kevin [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-11-18 18:04 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 8:53 [PATCH 0/2] block: Fix multiple blockdev-snapshot calls Kevin Wolf
2019-11-08 8:53 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_, }options Kevin Wolf
2019-11-08 14:30 ` Alberto Garcia
2019-11-12 16:01 ` [PATCH 1/2] block: Remove 'backing': null from bs->{explicit_,}options Peter Krempa
2019-11-08 8:53 ` [PATCH 2/2] iotests: Test multiple blockdev-snapshot calls Kevin Wolf
2019-11-08 14:32 ` Alberto Garcia
2019-11-13 9:28 ` Kevin Wolf
2019-11-12 16:07 ` Peter Krempa
2019-11-13 9:25 ` Kevin Wolf
2019-11-18 18:03 ` Max Reitz
2019-11-18 16:26 ` [PATCH 0/2] block: Fix " Peter Krempa
2019-11-18 16:49 ` Kevin Wolf
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).