* [PULL v2 1/4] block/snapshot: Restrict set of snapshot nodes
2019-10-02 16:18 [PULL v2 0/4] Block layer patches Kevin Wolf
@ 2019-10-02 16:18 ` Kevin Wolf
2019-10-02 16:18 ` [PULL v2 2/4] iotests: Test internal snapshots with -blockdev Kevin Wolf
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2019-10-02 16:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Nodes involved in internal snapshots were those that were returned by
bdrv_next(), inserted and not read-only. bdrv_next() in turn returns all
nodes that are either the root node of a BlockBackend or monitor-owned
nodes.
With the typical -drive use, this worked well enough. However, in the
typical -blockdev case, the user defines one node per option, making all
nodes monitor-owned nodes. This includes protocol nodes etc. which often
are not snapshottable, so "savevm" only returns an error.
Change the conditions so that internal snapshot still include all nodes
that have a BlockBackend attached (we definitely want to snapshot
anything attached to a guest device and probably also the built-in NBD
server; snapshotting block job BlockBackends is more of an accident, but
a preexisting one), but other monitor-owned nodes are only included if
they have no parents.
This makes internal snapshots usable again with typical -blockdev
configurations.
Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
---
block/snapshot.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index f2f48f926a..8081616ae9 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -31,6 +31,7 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qstring.h"
#include "qemu/option.h"
+#include "sysemu/block-backend.h"
QemuOptsList internal_snapshot_opts = {
.name = "snapshot",
@@ -384,6 +385,16 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
return ret;
}
+static bool bdrv_all_snapshots_includes_bs(BlockDriverState *bs)
+{
+ if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
+ return false;
+ }
+
+ /* Include all nodes that are either in use by a BlockBackend, or that
+ * aren't attached to any node, but owned by the monitor. */
+ return bdrv_has_blk(bs) || QLIST_EMPTY(&bs->parents);
+}
/* Group operations. All block drivers are involved.
* These functions will properly handle dataplane (take aio_context_acquire
@@ -399,7 +410,7 @@ bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs)
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
- if (bdrv_is_inserted(bs) && !bdrv_is_read_only(bs)) {
+ if (bdrv_all_snapshots_includes_bs(bs)) {
ok = bdrv_can_snapshot(bs);
}
aio_context_release(ctx);
@@ -426,8 +437,9 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
- if (bdrv_can_snapshot(bs) &&
- bdrv_snapshot_find(bs, snapshot, name) >= 0) {
+ if (bdrv_all_snapshots_includes_bs(bs) &&
+ bdrv_snapshot_find(bs, snapshot, name) >= 0)
+ {
ret = bdrv_snapshot_delete(bs, snapshot->id_str,
snapshot->name, err);
}
@@ -455,7 +467,7 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
- if (bdrv_can_snapshot(bs)) {
+ if (bdrv_all_snapshots_includes_bs(bs)) {
ret = bdrv_snapshot_goto(bs, name, errp);
}
aio_context_release(ctx);
@@ -481,7 +493,7 @@ int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
AioContext *ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
- if (bdrv_can_snapshot(bs)) {
+ if (bdrv_all_snapshots_includes_bs(bs)) {
err = bdrv_snapshot_find(bs, &sn, name);
}
aio_context_release(ctx);
@@ -512,7 +524,7 @@ int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
if (bs == vm_state_bs) {
sn->vm_state_size = vm_state_size;
err = bdrv_snapshot_create(bs, sn);
- } else if (bdrv_can_snapshot(bs)) {
+ } else if (bdrv_all_snapshots_includes_bs(bs)) {
sn->vm_state_size = 0;
err = bdrv_snapshot_create(bs, sn);
}
@@ -538,7 +550,7 @@ BlockDriverState *bdrv_all_find_vmstate_bs(void)
bool found;
aio_context_acquire(ctx);
- found = bdrv_can_snapshot(bs);
+ found = bdrv_all_snapshots_includes_bs(bs) && bdrv_can_snapshot(bs);
aio_context_release(ctx);
if (found) {
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL v2 2/4] iotests: Test internal snapshots with -blockdev
2019-10-02 16:18 [PULL v2 0/4] Block layer patches Kevin Wolf
2019-10-02 16:18 ` [PULL v2 1/4] block/snapshot: Restrict set of snapshot nodes Kevin Wolf
@ 2019-10-02 16:18 ` Kevin Wolf
2019-10-02 16:18 ` [PULL v2 3/4] iotests: Require Python 3.6 or later Kevin Wolf
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2019-10-02 16:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Peter Krempa <pkrempa@redhat.com>
---
tests/qemu-iotests/267 | 168 ++++++++++++++++++++++++++++
tests/qemu-iotests/267.out | 182 +++++++++++++++++++++++++++++++
tests/qemu-iotests/common.filter | 11 +-
tests/qemu-iotests/group | 1 +
4 files changed, 358 insertions(+), 4 deletions(-)
create mode 100755 tests/qemu-iotests/267
create mode 100644 tests/qemu-iotests/267.out
diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
new file mode 100755
index 0000000000..d37a67c012
--- /dev/null
+++ b/tests/qemu-iotests/267
@@ -0,0 +1,168 @@
+#!/usr/bin/env bash
+#
+# Test which nodes are involved in internal snapshots
+#
+# 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/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+ rm -f "$TEST_DIR/nbd"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# Internal snapshots are (currently) impossible with refcount_bits=1
+_unsupported_imgopts 'refcount_bits=1[^0-9]'
+
+do_run_qemu()
+{
+ echo Testing: "$@"
+ (
+ if ! test -t 0; then
+ while read cmd; do
+ echo $cmd
+ done
+ fi
+ echo quit
+ ) | $QEMU -nographic -monitor stdio -nodefaults "$@"
+ echo
+}
+
+run_qemu()
+{
+ do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp |
+ _filter_generated_node_ids | _filter_imgfmt | _filter_vmstate_size
+}
+
+size=128M
+
+run_test()
+{
+ _make_test_img $size
+ printf "savevm snap0\ninfo snapshots\nloadvm snap0\n" | run_qemu "$@" | _filter_date
+}
+
+
+echo
+echo "=== No block devices at all ==="
+echo
+
+run_test
+
+echo
+echo "=== -drive if=none ==="
+echo
+
+run_test -drive driver=file,file="$TEST_IMG",if=none
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=none -device virtio-blk,drive=none0
+
+echo
+echo "=== -drive if=virtio ==="
+echo
+
+run_test -drive driver=file,file="$TEST_IMG",if=virtio
+run_test -drive driver=$IMGFMT,file="$TEST_IMG",if=virtio
+
+echo
+echo "=== Simple -blockdev ==="
+echo
+
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+ -blockdev driver=$IMGFMT,file=file,node-name=fmt
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+ -blockdev driver=raw,file=file,node-name=raw \
+ -blockdev driver=$IMGFMT,file=raw,node-name=fmt
+
+echo
+echo "=== -blockdev with a filter on top ==="
+echo
+
+run_test -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+ -blockdev driver=$IMGFMT,file=file,node-name=fmt \
+ -blockdev driver=copy-on-read,file=fmt,node-name=filter
+
+echo
+echo "=== -blockdev with a backing file ==="
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img $size
+
+IMGOPTS="backing_file=$TEST_IMG.base" \
+run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+ -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+ -blockdev driver=$IMGFMT,file=file,backing=backing-file,node-name=fmt
+
+IMGOPTS="backing_file=$TEST_IMG.base" \
+run_test -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+ -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
+ -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+ -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+
+# A snapshot should be present on the overlay, but not the backing file
+echo Internal snapshots on overlay:
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+
+echo Internal snapshots on backing file:
+$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
+
+echo
+echo "=== -blockdev with NBD server on the backing file ==="
+echo
+
+IMGOPTS="backing_file=$TEST_IMG.base" _make_test_img $size
+cat <<EOF |
+nbd_server_start unix:$TEST_DIR/nbd
+nbd_server_add -w backing-fmt
+savevm snap0
+info snapshots
+loadvm snap0
+EOF
+run_qemu -blockdev driver=file,filename="$TEST_IMG.base",node-name=backing-file \
+ -blockdev driver=$IMGFMT,file=backing-file,node-name=backing-fmt \
+ -blockdev driver=file,filename="$TEST_IMG",node-name=file \
+ -blockdev driver=$IMGFMT,file=file,backing=backing-fmt,node-name=fmt |
+ _filter_date
+
+# This time, a snapshot should be created on both files
+echo Internal snapshots on overlay:
+$QEMU_IMG snapshot -l "$TEST_IMG" | _filter_date | _filter_vmstate_size
+
+echo Internal snapshots on backing file:
+$QEMU_IMG snapshot -l "$TEST_IMG.base" | _filter_date | _filter_vmstate_size
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/267.out b/tests/qemu-iotests/267.out
new file mode 100644
index 0000000000..9d812e3c72
--- /dev/null
+++ b/tests/qemu-iotests/267.out
@@ -0,0 +1,182 @@
+QA output created by 267
+
+=== No block devices at all ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing:
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: No block device can accept snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: No block device supports snapshots
+(qemu) quit
+
+
+=== -drive if=none ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device 'none0' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device 'none0' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=none -device virtio-blk,drive=none0
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -drive if=virtio ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=virtio
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device 'virtio0' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device 'virtio0' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -drive driver=IMGFMT,file=TEST_DIR/t.IMGFMT,if=virtio
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== Simple -blockdev ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+Error: Device '' is writable but does not support snapshots
+(qemu) info snapshots
+No available block device supports snapshots
+(qemu) loadvm snap0
+Error: Device '' is writable but does not support snapshots
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=raw,file=file,node-name=raw -blockdev driver=IMGFMT,file=raw,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -blockdev with a filter on top ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,node-name=fmt -blockdev driver=copy-on-read,file=fmt,node-name=filter
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+
+=== -blockdev with a backing file ===
+
+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
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-file,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Internal snapshots on overlay:
+Snapshot list:
+ID TAG VM SIZE DATE VM CLOCK
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+Internal snapshots on backing file:
+
+=== -blockdev with NBD server on the backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT.base,node-name=backing-file -blockdev driver=IMGFMT,file=backing-file,node-name=backing-fmt -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file -blockdev driver=IMGFMT,file=file,backing=backing-fmt,node-name=fmt
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) nbd_server_start unix:TEST_DIR/nbd
+(qemu) nbd_server_add -w backing-fmt
+(qemu) savevm snap0
+(qemu) info snapshots
+List of snapshots present on all disks:
+ID TAG VM SIZE DATE VM CLOCK
+-- snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+(qemu) loadvm snap0
+(qemu) quit
+
+Internal snapshots on overlay:
+Snapshot list:
+ID TAG VM SIZE DATE VM CLOCK
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+Internal snapshots on backing file:
+Snapshot list:
+ID TAG VM SIZE DATE VM CLOCK
+1 snap0 SIZE yyyy-mm-dd hh:mm:ss 00:00:00.000
+*** done
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 445a1c23e0..5c0790a693 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -19,12 +19,15 @@
# standard filters
#
-# ctime(3) dates
-#
_filter_date()
{
- $SED \
- -e 's/[A-Z][a-z][a-z] [A-z][a-z][a-z] *[0-9][0-9]* [0-9][0-9]:[0-9][0-9]:[0-9][0-9] [0-9][0-9][0-9][0-9]$/DATE/'
+ $SED -re 's/[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2}/yyyy-mm-dd hh:mm:ss/'
+}
+
+_filter_vmstate_size()
+{
+ $SED -re 's/[0-9. ]{5} [KMGT]iB/ SIZE/' \
+ -re 's/[0-9. ]{5} B/ SIZE/'
}
_filter_generated_node_ids()
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 5d3da937e4..5805a79d9e 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -277,3 +277,4 @@
263 rw quick
265 rw auto quick
266 rw quick
+267 rw auto quick snapshot
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL v2 3/4] iotests: Require Python 3.6 or later
2019-10-02 16:18 [PULL v2 0/4] Block layer patches Kevin Wolf
2019-10-02 16:18 ` [PULL v2 1/4] block/snapshot: Restrict set of snapshot nodes Kevin Wolf
2019-10-02 16:18 ` [PULL v2 2/4] iotests: Test internal snapshots with -blockdev Kevin Wolf
@ 2019-10-02 16:18 ` Kevin Wolf
2019-10-02 16:18 ` [PULL v2 4/4] iotests: Remove Python 2 compatibility code Kevin Wolf
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2019-10-02 16:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Running iotests is not required to build QEMU, so we can have stricter
version requirements for Python here and can make use of new features
and drop compatibility code earlier.
This makes qemu-iotests skip all Python tests if a Python version before
3.6 is used for the build.
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/check | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 875399d79f..588c453a94 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -633,6 +633,12 @@ then
export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
fi
+python_usable=false
+if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
+then
+ python_usable=true
+fi
+
default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
default_alias_machine=$($QEMU_PROG -machine help | \
sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
@@ -809,7 +815,12 @@ do
start=$(_wallclock)
if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" ]; then
- run_command="$PYTHON $seq"
+ if $python_usable; then
+ run_command="$PYTHON $seq"
+ else
+ run_command="false"
+ echo "Unsupported Python version" > $seq.notrun
+ fi
else
run_command="./$seq"
fi
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PULL v2 4/4] iotests: Remove Python 2 compatibility code
2019-10-02 16:18 [PULL v2 0/4] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2019-10-02 16:18 ` [PULL v2 3/4] iotests: Require Python 3.6 or later Kevin Wolf
@ 2019-10-02 16:18 ` Kevin Wolf
2019-10-02 17:18 ` [PULL v2 0/4] Block layer patches no-reply
2019-10-03 16:18 ` Peter Maydell
5 siblings, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2019-10-02 16:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
Some scripts check the Python version number and have two code paths to
accomodate both Python 2 and 3. Remove the code specific to Python 2 and
assert the minimum version of 3.6 instead (check skips Python tests in
this case, so the assertion would only ever trigger if a Python script
is executed manually).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/044 | 3 ---
tests/qemu-iotests/163 | 3 ---
tests/qemu-iotests/iotests.py | 13 +++----------
3 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
index 05ea1f49c5..8b2afa2a11 100755
--- a/tests/qemu-iotests/044
+++ b/tests/qemu-iotests/044
@@ -28,9 +28,6 @@ import struct
import subprocess
import sys
-if sys.version_info.major == 2:
- range = xrange
-
test_img = os.path.join(iotests.test_dir, 'test.img')
class TestRefcountTableGrowth(iotests.QMPTestCase):
diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
index 081ccc8ac1..d94728e080 100755
--- a/tests/qemu-iotests/163
+++ b/tests/qemu-iotests/163
@@ -21,9 +21,6 @@
import os, random, iotests, struct, qcow2, sys
from iotests import qemu_img, qemu_io, image_size
-if sys.version_info.major == 2:
- range = xrange
-
test_img = os.path.join(iotests.test_dir, 'test.img')
check_img = os.path.join(iotests.test_dir, 'check.img')
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b26271187c..9fb5181c3d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -35,6 +35,7 @@ from collections import OrderedDict
sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
from qemu import qtest
+assert sys.version_info >= (3,6)
# This will not work if arguments contain spaces but is necessary if we
# want to support the override options that ./check supports.
@@ -250,10 +251,7 @@ def image_size(img):
return json.loads(r)['virtual-size']
def is_str(val):
- if sys.version_info.major >= 3:
- return isinstance(val, str)
- else:
- return isinstance(val, str) or isinstance(val, unicode)
+ return isinstance(val, str)
test_dir_re = re.compile(r"%s" % test_dir)
def filter_test_dir(msg):
@@ -935,12 +933,7 @@ def execute_test(test_function=None,
else:
# We need to filter out the time taken from the output so that
# qemu-iotest can reliably diff the results against master output.
- if sys.version_info.major >= 3:
- output = io.StringIO()
- else:
- # io.StringIO is for unicode strings, which is not what
- # 2.x's test runner emits.
- output = io.BytesIO()
+ output = io.StringIO()
logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PULL v2 0/4] Block layer patches
2019-10-02 16:18 [PULL v2 0/4] Block layer patches Kevin Wolf
` (3 preceding siblings ...)
2019-10-02 16:18 ` [PULL v2 4/4] iotests: Remove Python 2 compatibility code Kevin Wolf
@ 2019-10-02 17:18 ` no-reply
2019-10-03 16:18 ` Peter Maydell
5 siblings, 0 replies; 7+ messages in thread
From: no-reply @ 2019-10-02 17:18 UTC (permalink / raw)
To: kwolf; +Cc: kwolf, peter.maydell, qemu-devel, qemu-block
Patchew URL: https://patchew.org/QEMU/20191002161851.1016-1-kwolf@redhat.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===
-Testing:
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) savevm snap0
-Error: No block device can accept snapshots
-(qemu) info snapshots
-No available block device supports snapshots
-(qemu) loadvm snap0
-Error: No block device supports snapshots
-(qemu) quit
+Usage: sed [OPTION]... {script-only-if-no-other-script} [input-file]...
---
-Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) savevm snap0
-Error: Device 'none0' is writable but does not support snapshots
-(qemu) info snapshots
-No available block device supports snapshots
-(qemu) loadvm snap0
-Error: Device 'none0' is writable but does not support snapshots
-(qemu) quit
-
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
---
-Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=virtio
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) savevm snap0
-Error: Device 'virtio0' is writable but does not support snapshots
-(qemu) info snapshots
-No available block device supports snapshots
-(qemu) loadvm snap0
-Error: Device 'virtio0' is writable but does not support snapshots
-(qemu) quit
-
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
---
-Testing: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=file
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) savevm snap0
-Error: Device '' is writable but does not support snapshots
-(qemu) info snapshots
-No available block device supports snapshots
-(qemu) loadvm snap0
-Error: Device '' is writable but does not support snapshots
-(qemu) quit
-
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
---
*** done
Failures: 267
Failed 1 of 108 iotests
make: *** [check-tests/check-block.sh] Error 1
Traceback (most recent call last):
File "./tests/docker/docker.py", line 662, in <module>
sys.exit(main())
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=92726187a0bd495c9bd9618f815ba7bb', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-irvd3y2z/src/docker-src.2019-10-02-13.08.52.15131:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=92726187a0bd495c9bd9618f815ba7bb
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-irvd3y2z/src'
make: *** [docker-run-test-quick@centos7] Error 2
real 9m50.205s
user 0m8.303s
The full log is available at
http://patchew.org/logs/20191002161851.1016-1-kwolf@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PULL v2 0/4] Block layer patches
2019-10-02 16:18 [PULL v2 0/4] Block layer patches Kevin Wolf
` (4 preceding siblings ...)
2019-10-02 17:18 ` [PULL v2 0/4] Block layer patches no-reply
@ 2019-10-03 16:18 ` Peter Maydell
5 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2019-10-03 16:18 UTC (permalink / raw)
To: Kevin Wolf; +Cc: QEMU Developers, Qemu-block
On Wed, 2 Oct 2019 at 17:18, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit 7f21573c822805a8e6be379d9bcf3ad9effef3dc:
>
> Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-10-01' into staging (2019-10-01 13:13:38 +0100)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 352492e5541e293ea8117412ffd171ce1c7ac15d:
>
> iotests: Remove Python 2 compatibility code (2019-10-02 16:47:23 +0200)
>
> ----------------------------------------------------------------
> Block layer patches:
>
> - Fix internal snapshots with typical -blockdev setups
> - iotests: Require Python 3.6 or later
>
> ----------------------------------------------------------------
> Kevin Wolf (4):
> block/snapshot: Restrict set of snapshot nodes
> iotests: Test internal snapshots with -blockdev
> iotests: Require Python 3.6 or later
> iotests: Remove Python 2 compatibility code
Hi. This failed on iotest 267 on the ppc build machine I test on,
in the same way that the patchew report for centos7 failed:
--- /home/pm215/qemu/tests/qemu-iotests/267.out 2019-10-03
15:35:50.099974113 +0000
+++ /home/pm215/qemu/build/all/tests/qemu-iotests/267.out.bad
2019-10-03 15:41:46.190579461 +0000
@@ -3,180 +3,770 @@
=== No block devices at all ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
-Testing:
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) savevm snap0
-Error: No block device can accept snapshots
-(qemu) info snapshots
-No available block device supports snapshots
-(qemu) loadvm snap0
-Error: No block device supports snapshots
-(qemu) quit
+Usage: sed [OPTION]... {script-only-if-no-other-script} [input-file]...
+ -n, --quiet, --silent
+ suppress automatic printing of pattern space
+ -e script, --expression=script
+ add the script to the commands to be executed
[etc]
The machine is running 'sed (GNU sed) 4.2.2'.
It looks like this version of sed doesn't like it if you
specify '-r' twice, as you have done in the _filter_vmstate_size()
function in common.filter in the "iotests: Test internal snapshots
with -blockdev" patch. I think '-r' is supposed to apply to all
scripts in the sed invocation; if that's correct then a command
line like '-re thing1 -re thing2' is confusing and would be better
written '-r -e thing1 -e thing2' even if this older sed didn't
complain about it.
thanks
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread