From af957387547b05ed6dc4d84c10cca42700a7aeda Mon Sep 17 00:00:00 2001 From: Zhang Haoyu Date: Mon, 29 Sep 2014 16:38:02 +0800 Subject: [PATCH 01/23] snapshot: fix referencing wrong variable in while loop in do_delvm The while loop variabal is "bs1", but "bs" is always passed to bdrv_snapshot_delete_by_id_or_name. Broken in commit a89d89d, v1.7.0. Signed-off-by: Zhang Haoyu Reviewed-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- savevm.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/savevm.c b/savevm.c index e19ae0a9b2..2d8eb960bb 100644 --- a/savevm.c +++ b/savevm.c @@ -1245,19 +1245,18 @@ int load_vmstate(const char *name) void do_delvm(Monitor *mon, const QDict *qdict) { - BlockDriverState *bs, *bs1; + BlockDriverState *bs; Error *err = NULL; const char *name = qdict_get_str(qdict, "name"); - bs = find_vmstate_bs(); - if (!bs) { + if (!find_vmstate_bs()) { monitor_printf(mon, "No block device supports snapshots\n"); return; } - bs1 = NULL; - while ((bs1 = bdrv_next(bs1))) { - if (bdrv_can_snapshot(bs1)) { + bs = NULL; + while ((bs = bdrv_next(bs))) { + if (bdrv_can_snapshot(bs)) { bdrv_snapshot_delete_by_id_or_name(bs, name, &err); if (err) { monitor_printf(mon, From 18fe46d79a6de61cb2c379fb610d834ef658d84b Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Mon, 29 Sep 2014 09:06:22 +0100 Subject: [PATCH 02/23] ssh: Don't crash if either host or path is not specified. $ ./qemu-img create -f qcow2 overlay \ -b 'json: { "file.driver":"ssh", "file.host":"localhost", "file.host_key_check":"no" }' qemu-img: qobject/qdict.c:193: qdict_get_obj: Assertion `obj != ((void *)0)' failed. Aborted A similar crash also happens if the file.host field is omitted. https://bugzilla.redhat.com/show_bug.cgi?id=1147343 Bug found and reported by Jun Li. Signed-off-by: Richard W.M. Jones Reviewed-by: Gonglei Signed-off-by: Stefan Hajnoczi --- block/ssh.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/block/ssh.c b/block/ssh.c index cf43bc0f89..f466cbf396 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -517,6 +517,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, const char *host, *user, *path, *host_key_check; int port; + if (!qdict_haskey(options, "host")) { + ret = -EINVAL; + error_setg(errp, "No hostname was specified"); + goto err; + } host = qdict_get_str(options, "host"); if (qdict_haskey(options, "port")) { @@ -525,6 +530,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, port = 22; } + if (!qdict_haskey(options, "path")) { + ret = -EINVAL; + error_setg(errp, "No path was specified"); + goto err; + } path = qdict_get_str(options, "path"); if (qdict_haskey(options, "user")) { From fbf28a4328123b3259d100eedc0e6f5b7f8bf186 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 29 Sep 2014 16:07:55 +0200 Subject: [PATCH 03/23] block: Drop superfluous conditionals around qemu_opts_del() Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-id: 1411999675-14533-1-git-send-email-armbru@redhat.com Signed-off-by: Stefan Hajnoczi --- blockdev.c | 4 +--- qemu-img.c | 4 +--- qemu-nbd.c | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/blockdev.c b/blockdev.c index ad436488b7..2f441c5343 100644 --- a/blockdev.c +++ b/blockdev.c @@ -224,9 +224,7 @@ void drive_info_del(DriveInfo *dinfo) if (!dinfo) { return; } - if (dinfo->opts) { - qemu_opts_del(dinfo->opts); - } + qemu_opts_del(dinfo->opts); g_free(dinfo->id); QTAILQ_REMOVE(&drives, dinfo, next); g_free(dinfo->serial); diff --git a/qemu-img.c b/qemu-img.c index ea4bbae546..27b85dbda4 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1736,9 +1736,7 @@ out: qemu_opts_del(opts); qemu_opts_free(create_opts); qemu_vfree(buf); - if (sn_opts) { - qemu_opts_del(sn_opts); - } + qemu_opts_del(sn_opts); if (out_bs) { bdrv_unref(out_bs); } diff --git a/qemu-nbd.c b/qemu-nbd.c index fa603382d4..b524b3420f 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -778,9 +778,7 @@ int main(int argc, char **argv) unlink(sockpath); } - if (sn_opts) { - qemu_opts_del(sn_opts); - } + qemu_opts_del(sn_opts); if (device) { void *ret; From d1319b077a4bd980ca1b8a167b02b519330dd26b Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 23 Sep 2014 09:56:21 +0800 Subject: [PATCH 04/23] vmdk: Fix integer overflow in offset calculation This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster allocation). $ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k' write failed: Invalid argument Reported-by: Mark Cave-Ayland Reviewed-by: Max Reitz Signed-off-by: Fam Zheng Message-id: 1411437381-11234-1-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/vmdk.c | 2 +- tests/qemu-iotests/105 | 70 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/105.out | 21 ++++++++++++ tests/qemu-iotests/group | 1 + 4 files changed, 93 insertions(+), 1 deletion(-) create mode 100755 tests/qemu-iotests/105 create mode 100644 tests/qemu-iotests/105.out diff --git a/block/vmdk.c b/block/vmdk.c index afdea1a8b6..4ae6c75310 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs, uint32_t min_count, *l2_table; bool zeroed = false; int64_t ret; - int32_t cluster_sector; + int64_t cluster_sector; if (m_data) { m_data->valid = 0; diff --git a/tests/qemu-iotests/105 b/tests/qemu-iotests/105 new file mode 100755 index 0000000000..9bae49e327 --- /dev/null +++ b/tests/qemu-iotests/105 @@ -0,0 +1,70 @@ +#!/bin/bash +# +# Create, read, write big image +# +# Copyright (C) 2014 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 . +# + +# creator +owner=famz@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +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 qcow2 vmdk vhdx qed +_supported_proto generic +_supported_os Linux +_unsupported_imgopts "subformat=twoGbMaxExtentFlat" \ + "subformat=twoGbMaxExtentSparse" + +echo +echo "creating large image" +_make_test_img 16T + +echo +echo "small read" +$QEMU_IO -c "read 1024 4096" "$TEST_IMG" | _filter_qemu_io + +echo +echo "small write" +$QEMU_IO -c "write 8192 4096" "$TEST_IMG" | _filter_qemu_io + +echo +echo "small read at high offset" +$QEMU_IO -c "read 14T 4096" "$TEST_IMG" | _filter_qemu_io + +echo +echo "small write at high offset" +$QEMU_IO -c "write 14T 4096" "$TEST_IMG" | _filter_qemu_io + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/105.out b/tests/qemu-iotests/105.out new file mode 100644 index 0000000000..13ffcb5932 --- /dev/null +++ b/tests/qemu-iotests/105.out @@ -0,0 +1,21 @@ +QA output created by 105 + +creating large image +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=17592186044416 + +small read +read 4096/4096 bytes at offset 1024 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small write +wrote 4096/4096 bytes at offset 8192 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small read at high offset +read 4096/4096 bytes at offset 15393162788864 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small write at high offset +wrote 4096/4096 bytes at offset 15393162788864 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 622685e94c..b230996528 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -105,3 +105,4 @@ 101 rw auto quick 103 rw auto quick 104 rw auto +105 rw auto quick From 20d6cd47d0459bdc6a8b64e7abe8f713e0d4718e Mon Sep 17 00:00:00 2001 From: Jun Li Date: Wed, 24 Sep 2014 13:45:27 +0800 Subject: [PATCH 05/23] Modify qemu_opt_rename to realize renaming all items in opts Add realization of rename all items in opts for qemu_opt_rename. e.g: When add bps twice in command line, need to rename all bps to throttling.bps-total. This patch solved following bug: Bug 1145586 - qemu-kvm will give strange hint when add bps twice for a drive ref:https://bugzilla.redhat.com/show_bug.cgi?id=1145586 [Resolved conflict with commit 5abbf0ee4d87c695deb1c3fca9bb994b93a3e3be ("block: Catch simultaneous usage of options and their aliases"). Check for simultaneous use first, and then loop over all options. --Stefan] Signed-off-by: Jun Li Reviewed-by: Eric Blake Message-id: 1411537527-16715-1-git-send-email-junmuzi@gmail.com Signed-off-by: Stefan Hajnoczi --- blockdev.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/blockdev.c b/blockdev.c index 2f441c5343..dc94ad319a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -548,6 +548,10 @@ static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to, "same time", to, from); return; } + } + + /* rename all items in opts */ + while ((value = qemu_opt_get(opts, from))) { qemu_opt_set(opts, to, value); qemu_opt_unset(opts, from); } From d9323e9b20b1a74720f17e81387cffe013d9cf0b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 30 Sep 2014 13:27:09 +0200 Subject: [PATCH 06/23] make check-block: Use default cache modes When qemu-iotests only gave a choice between cache=none and cache=writethrough, we picked cache=none because it was the option that would complete the test in finite time. Some tests could only work for one of the two options and would be skipped with cache=none, but that was an acceptable trade-off at the time. Today, however, qemu-iotests is a bit more flexible than that and you can specify any of the cache modes supported by qemu. The default is writeback, like in qemu, which is fast and (unlike cache=none) compatible with any host filesystem. Test cases that have specific requirements for the cache mode can also specify a different default. In order to get a fast test run that works everywhere and doesn't skip tests that need a different cache mode, not specifying any cache mode and instead relying on the default is the best we can do today. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster Message-id: 1412076430-11623-2-git-send-email-kwolf@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests-quick.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests-quick.sh b/tests/qemu-iotests-quick.sh index 8a9a4c68e9..12af731c68 100755 --- a/tests/qemu-iotests-quick.sh +++ b/tests/qemu-iotests-quick.sh @@ -3,6 +3,6 @@ cd tests/qemu-iotests ret=0 -./check -T -nocache -qcow2 -g quick || ret=1 +./check -T -qcow2 -g quick || ret=1 exit $ret From cf77b2d25eeabe268412cf41d4ea38ec5de8c611 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 30 Sep 2014 13:27:10 +0200 Subject: [PATCH 07/23] qemu-iotests: Fix supported cache modes for 052 The requirement for this test case is really "no O_DIRECT", because the temporary snapshot for BDRV_O_SNAPSHOT is created in /tmp, which often is a tmpfs. Commit f210a83c ('qemu-iotests: Add _default_cache_mode and _supported_cache_modes') turned the restriction into writethrough-only, but that's not really necessary. Allow to run the test for any non-O_DIRECT cache modes, and use the global default of writeback if no cache mode is specified. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster Message-id: 1412076430-11623-3-git-send-email-kwolf@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests/052 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052 index 6bdae92780..61959e286e 100755 --- a/tests/qemu-iotests/052 +++ b/tests/qemu-iotests/052 @@ -41,8 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt generic _supported_proto file _supported_os Linux -_default_cache_mode "writethrough" -_supported_cache_modes "writethrough" + +# Don't do O_DIRECT on tmpfs +_supported_cache_modes "writeback" "writethrough" "unsafe" size=128M _make_test_img $size From a66c9dc734fb30de1e18e9dc217f2d37e16c492a Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:24 -0400 Subject: [PATCH 08/23] blockdev: Orphaned drive search When users use command line options like -hda, -cdrom, or even -drive if=ide, it is up to the board initialization routines to pick up these drives and create backing devices for them. Some boards, like Q35, have not been doing this. However, there is no warning explaining why certain drive specifications are just silently ignored, so this function adds a check to print some warnings to assist users in debugging these sorts of issues in the future. This patch will not warn about drives added with if_none, for which it is not possible to tell in advance if the omission of a backing device is an issue. A warning in these cases is considered appropriate. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Message-id: 1412187569-23452-2-git-send-email-jsnow@redhat.com Signed-off-by: Stefan Hajnoczi --- blockdev.c | 21 +++++++++++++++++++++ include/sysemu/blockdev.h | 2 ++ vl.c | 10 +++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/blockdev.c b/blockdev.c index dc94ad319a..48da1a7e0f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -166,6 +166,27 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit) return NULL; } +bool drive_check_orphaned(void) +{ + DriveInfo *dinfo; + bool rs = false; + + QTAILQ_FOREACH(dinfo, &drives, next) { + /* If dinfo->bdrv->dev is NULL, it has no device attached. */ + /* Unless this is a default drive, this may be an oversight. */ + if (!dinfo->bdrv->dev && !dinfo->is_default && + dinfo->type != IF_NONE) { + fprintf(stderr, "Warning: Orphaned drive without device: " + "id=%s,file=%s,if=%s,bus=%d,unit=%d\n", + dinfo->id, dinfo->bdrv->filename, if_name[dinfo->type], + dinfo->bus, dinfo->unit); + rs = true; + } + } + + return rs; +} + DriveInfo *drive_get_by_index(BlockInterfaceType type, int index) { return drive_get(type, diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index abec381049..30402867b9 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -38,6 +38,7 @@ struct DriveInfo { int unit; int auto_del; /* see blockdev_mark_auto_del() */ bool enable_auto_del; /* Only for legacy drive_new() */ + bool is_default; /* Added by default_drive() ? */ int media_cd; int cyls, heads, secs, trans; QemuOpts *opts; @@ -46,6 +47,7 @@ struct DriveInfo { }; DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit); +bool drive_check_orphaned(void); DriveInfo *drive_get_by_index(BlockInterfaceType type, int index); int drive_get_max_bus(BlockInterfaceType type); DriveInfo *drive_get_next(BlockInterfaceType type); diff --git a/vl.c b/vl.c index 9d2aaaf1dc..4bc8f97c25 100644 --- a/vl.c +++ b/vl.c @@ -1169,6 +1169,7 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type, int index, const char *optstr) { QemuOpts *opts; + DriveInfo *dinfo; if (!enable || drive_get_by_index(type, index)) { return; @@ -1178,9 +1179,13 @@ static void default_drive(int enable, int snapshot, BlockInterfaceType type, if (snapshot) { drive_enable_snapshot(opts, NULL); } - if (!drive_new(opts, type)) { + + dinfo = drive_new(opts, type); + if (!dinfo) { exit(1); } + dinfo->is_default = true; + } void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque) @@ -4457,6 +4462,9 @@ int main(int argc, char **argv, char **envp) if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1) != 0) exit(1); + /* Did we create any drives that we failed to create a device for? */ + drive_check_orphaned(); + net_check_clients(); ds = init_displaystate(); From 21dff8cf38d311a917ab33f19d5cea7696f0c354 Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:25 -0400 Subject: [PATCH 09/23] blockdev: Allow overriding if_max_dev property The if_max_devs table as in the past been an immutable default that controls the mapping of index => (bus,unit) for all boards and all HBAs for each interface type. Since adding this mapping information to the HBA device itself is currently unwieldly from the perspective of retrieving this information at option parsing time (e.g, within drive_new), we consider the alternative of marking the if_max_devs table mutable so that later configuration and initialization can adjust the mapping at will, but only up until a drive is added, at which point the mapping is finalized. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Message-id: 1412187569-23452-3-git-send-email-jsnow@redhat.com Signed-off-by: Stefan Hajnoczi --- blockdev.c | 26 +++++++++++++++++++++++++- include/sysemu/blockdev.h | 2 ++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/blockdev.c b/blockdev.c index 48da1a7e0f..1780d7723a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -60,7 +60,7 @@ static const char *const if_name[IF_COUNT] = { [IF_XEN] = "xen", }; -static const int if_max_devs[IF_COUNT] = { +static int if_max_devs[IF_COUNT] = { /* * Do not change these numbers! They govern how drive option * index maps to unit and bus. That mapping is ABI. @@ -79,6 +79,30 @@ static const int if_max_devs[IF_COUNT] = { [IF_SCSI] = 7, }; +/** + * Boards may call this to offer board-by-board overrides + * of the default, global values. + */ +void override_max_devs(BlockInterfaceType type, int max_devs) +{ + DriveInfo *dinfo; + + if (max_devs <= 0) { + return; + } + + QTAILQ_FOREACH(dinfo, &drives, next) { + if (dinfo->type == type) { + fprintf(stderr, "Cannot override units-per-bus property of" + " the %s interface, because a drive of that type has" + " already been added.\n", if_name[type]); + g_assert_not_reached(); + } + } + + if_max_devs[type] = max_devs; +} + /* * We automatically delete the drive when a device using it gets * unplugged. Questionable feature, but we can't just drop it. diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index 30402867b9..a4033d4c29 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -46,6 +46,8 @@ struct DriveInfo { QTAILQ_ENTRY(DriveInfo) next; }; +void override_max_devs(BlockInterfaceType type, int max_devs); + DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit); bool drive_check_orphaned(void); DriveInfo *drive_get_by_index(BlockInterfaceType type, int index); From 1602651833c081e32366c9e534ad72e4287840c5 Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:26 -0400 Subject: [PATCH 10/23] pc/vl: Add units-per-default-bus property This patch adds the 'units_per_default_bus' property which allows individual boards to declare their desired index => (bus,unit) mapping for their default HBA, so that boards such as Q35 can specify that its default if_ide HBA, AHCI, only accepts one unit per bus. This property only overrides the mapping for drives matching the block_default_type interface. This patch also adds this property to *all* past and present Q35 machine types. This retroactive addition is justified because the previous erroneous index=>(bus,unit) mappings caused by lack of such a property were not utilized due to lack of initialization code in the Q35 init routine. Further, semantically, the Q35 board type has always had the property that its default HBA, AHCI, only accepts one unit per bus. The new code added to add devices to drives relies upon the accuracy of this mapping. Thus, the property is applied retroactively to reduce complexity of allowing IDE HBAs with different units per bus. Examples: Prior to this patch, all IDE HBAs were assumed to use 2 units per bus (Master, Slave). When using Q35 and AHCI, however, we only allow one unit per bus. -hdb foo.qcow2 would become index=1, or bus=0,unit=1. -hdd foo.qcow2 would become index=3, or bus=1,unit=1. -drive file=foo.qcow2,index=5 becomes bus=2,unit=1. These are invalid for AHCI. They now become, under Q35 only: -hdb foo.qcow2 --> index=1, bus=1, unit=0. -hdd foo.qcow2 --> index=3, bus=3, unit=0. -drive file=foo.qcow2,index=5 --> bus=5,unit=0. The mapping is adjusted based on the fact that the default IF for the Q35 machine type is IF_IDE, and units-per-default-bus overrides the IDE mapping from its default of 2 units per bus to just 1 unit per bus. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Reviewed-by: Michael S. Tsirkin Message-id: 1412187569-23452-4-git-send-email-jsnow@redhat.com Signed-off-by: Stefan Hajnoczi --- hw/i386/pc.c | 1 + hw/i386/pc_q35.c | 3 ++- include/hw/boards.h | 2 ++ vl.c | 8 ++++++++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 82a7daa188..d045e8b454 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1524,6 +1524,7 @@ static void pc_generic_machine_class_init(ObjectClass *oc, void *data) mc->hot_add_cpu = qm->hot_add_cpu; mc->kvm_type = qm->kvm_type; mc->block_default_type = qm->block_default_type; + mc->units_per_default_bus = qm->units_per_default_bus; mc->max_cpus = qm->max_cpus; mc->no_serial = qm->no_serial; mc->no_parallel = qm->no_parallel; diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index d4a907c71b..b28ddbb742 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -344,7 +344,8 @@ static void pc_q35_init_1_4(MachineState *machine) #define PC_Q35_MACHINE_OPTIONS \ PC_DEFAULT_MACHINE_OPTIONS, \ .desc = "Standard PC (Q35 + ICH9, 2009)", \ - .hot_add_cpu = pc_hot_add_cpu + .hot_add_cpu = pc_hot_add_cpu, \ + .units_per_default_bus = 1 #define PC_Q35_2_2_MACHINE_OPTIONS \ PC_Q35_MACHINE_OPTIONS, \ diff --git a/include/hw/boards.h b/include/hw/boards.h index dfb6718dc1..663f16acdd 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -28,6 +28,7 @@ struct QEMUMachine { QEMUMachineHotAddCPUFunc *hot_add_cpu; QEMUMachineGetKvmtypeFunc *kvm_type; BlockInterfaceType block_default_type; + int units_per_default_bus; int max_cpus; unsigned int no_serial:1, no_parallel:1, @@ -86,6 +87,7 @@ struct MachineClass { int (*kvm_type)(const char *arg); BlockInterfaceType block_default_type; + int units_per_default_bus; int max_cpus; unsigned int no_serial:1, no_parallel:1, diff --git a/vl.c b/vl.c index 4bc8f97c25..debcbcceea 100644 --- a/vl.c +++ b/vl.c @@ -1585,6 +1585,7 @@ static void machine_class_init(ObjectClass *oc, void *data) mc->hot_add_cpu = qm->hot_add_cpu; mc->kvm_type = qm->kvm_type; mc->block_default_type = qm->block_default_type; + mc->units_per_default_bus = qm->units_per_default_bus; mc->max_cpus = qm->max_cpus; mc->no_serial = qm->no_serial; mc->no_parallel = qm->no_parallel; @@ -4375,6 +4376,13 @@ int main(int argc, char **argv, char **envp) blk_mig_init(); ram_mig_init(); + /* If the currently selected machine wishes to override the units-per-bus + * property of its default HBA interface type, do so now. */ + if (machine_class->units_per_default_bus) { + override_max_devs(machine_class->block_default_type, + machine_class->units_per_default_bus); + } + /* open the virtual block devices */ if (snapshot) qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0); From d8f94e1bb275ab6a14a15220fd6afd0d04324aeb Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:27 -0400 Subject: [PATCH 11/23] ide: Update ide_drive_get to be HBA agnostic Instead of duplicating the logic for the if_ide (bus,unit) mappings, rely on the blockdev layer for managing those mappings for us, and use the drive_get_by_index call instead. This allows ide_drive_get to work for AHCI HBAs as well, and can be used in the Q35 initialization. Lastly, change the nature of the argument to ide_drive_get so that represents the number of total drives we can support, and not the total number of buses. This will prevent array overflows if the units-per-default-bus property ever needs to be adjusted for compatibility reasons. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Reviewed-by: Michael S. Tsirkin Message-id: 1412187569-23452-5-git-send-email-jsnow@redhat.com Signed-off-by: Stefan Hajnoczi --- blockdev.c | 17 +++++++++++++++++ hw/alpha/dp264.c | 2 +- hw/i386/pc_piix.c | 2 +- hw/ide/core.c | 22 +++++++++++++++++----- hw/mips/mips_fulong2e.c | 2 +- hw/mips/mips_malta.c | 2 +- hw/mips/mips_r4k.c | 2 +- hw/ppc/mac_newworld.c | 2 +- hw/ppc/mac_oldworld.c | 2 +- hw/ppc/prep.c | 2 +- hw/sparc64/sun4u.c | 2 +- include/sysemu/blockdev.h | 1 + 12 files changed, 44 insertions(+), 14 deletions(-) diff --git a/blockdev.c b/blockdev.c index 1780d7723a..e595910476 100644 --- a/blockdev.c +++ b/blockdev.c @@ -135,6 +135,23 @@ void blockdev_auto_del(BlockDriverState *bs) } } +/** + * Returns the current mapping of how many units per bus + * a particular interface can support. + * + * A positive integer indicates n units per bus. + * 0 implies the mapping has not been established. + * -1 indicates an invalid BlockInterfaceType was given. + */ +int drive_get_max_devs(BlockInterfaceType type) +{ + if (type >= IF_IDE && type < IF_COUNT) { + return if_max_devs[type]; + } + + return -1; +} + static int drive_index_to_bus_id(BlockInterfaceType type, int index) { int max_devs = if_max_devs[type]; diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c index b178a03604..84a55e41a2 100644 --- a/hw/alpha/dp264.c +++ b/hw/alpha/dp264.c @@ -97,7 +97,7 @@ static void clipper_init(MachineState *machine) /* IDE disk setup. */ { DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); pci_cmd646_ide_init(pci_bus, hd, 0); } diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 103d756a72..43846339bc 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -239,7 +239,7 @@ static void pc_init1(MachineState *machine, pc_nic_init(isa_bus, pci_bus); - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); if (pci_enabled) { PCIDevice *dev; if (xen_enabled()) { diff --git a/hw/ide/core.c b/hw/ide/core.c index 190700ac74..ae85428864 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2558,16 +2558,28 @@ const VMStateDescription vmstate_ide_bus = { } }; -void ide_drive_get(DriveInfo **hd, int max_bus) +void ide_drive_get(DriveInfo **hd, int n) { int i; + int highest_bus = drive_get_max_bus(IF_IDE) + 1; + int max_devs = drive_get_max_devs(IF_IDE); + int n_buses = max_devs ? (n / max_devs) : n; - if (drive_get_max_bus(IF_IDE) >= max_bus) { - fprintf(stderr, "qemu: too many IDE bus: %d\n", max_bus); + /* + * Note: The number of actual buses available is not known. + * We compute this based on the size of the DriveInfo* array, n. + * If it is less than max_devs * , + * We will stop looking for drives prematurely instead of overfilling + * the array. + */ + + if (highest_bus > n_buses) { + error_report("Too many IDE buses defined (%d > %d)", + highest_bus, n_buses); exit(1); } - for(i = 0; i < max_bus * MAX_IDE_DEVS; i++) { - hd[i] = drive_get(IF_IDE, i / MAX_IDE_DEVS, i % MAX_IDE_DEVS); + for (i = 0; i < n; i++) { + hd[i] = drive_get_by_index(IF_IDE, i); } } diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c index be286da18b..29cd708e07 100644 --- a/hw/mips/mips_fulong2e.c +++ b/hw/mips/mips_fulong2e.c @@ -350,7 +350,7 @@ static void mips_fulong2e_init(MachineState *machine) pci_bus = bonito_init((qemu_irq *)&(env->irq[2])); /* South bridge */ - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); isa_bus = vt82c686b_init(pci_bus, PCI_DEVFN(FULONG2E_VIA_SLOT, 0)); if (!isa_bus) { diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index 2d87de9ea5..b20807c4e4 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -1147,7 +1147,7 @@ void mips_malta_init(MachineState *machine) pci_bus = gt64120_register(isa_irq); /* Southbridge */ - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); piix4_devfn = piix4_init(pci_bus, &isa_bus, 80); diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c index e219766f3e..93606a490c 100644 --- a/hw/mips/mips_r4k.c +++ b/hw/mips/mips_r4k.c @@ -294,7 +294,7 @@ void mips_r4k_init(MachineState *machine) if (nd_table[0].used) isa_ne2000_init(isa_bus, 0x300, 9, &nd_table[0]); - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); for(i = 0; i < MAX_IDE_BUS; i++) isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i], ide_irq[i], hd[MAX_IDE_DEVS * i], diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 1626db44ef..4094beb0ed 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -400,7 +400,7 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c index be9a194038..ebd87fb8f5 100644 --- a/hw/ppc/mac_oldworld.c +++ b/hw/ppc/mac_oldworld.c @@ -278,7 +278,7 @@ static void ppc_heathrow_init(MachineState *machine) pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); macio = pci_create(pci_bus, -1, TYPE_OLDWORLD_MACIO); dev = DEVICE(macio); diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c index f0ef1af118..9f484cdcb8 100644 --- a/hw/ppc/prep.c +++ b/hw/ppc/prep.c @@ -519,7 +519,7 @@ static void ppc_prep_init(MachineState *machine) } } - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); for(i = 0; i < MAX_IDE_BUS; i++) { isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i], ide_irq[i], hd[2 * i], diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c index 9c77e18244..871a0ea246 100644 --- a/hw/sparc64/sun4u.c +++ b/hw/sparc64/sun4u.c @@ -864,7 +864,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem, for(i = 0; i < nb_nics; i++) pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - ide_drive_get(hd, MAX_IDE_BUS); + ide_drive_get(hd, ARRAY_SIZE(hd)); pci_cmd646_ide_init(pci_bus, hd, 1); diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index a4033d4c29..09716de809 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -52,6 +52,7 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit); bool drive_check_orphaned(void); DriveInfo *drive_get_by_index(BlockInterfaceType type, int index); int drive_get_max_bus(BlockInterfaceType type); +int drive_get_max_devs(BlockInterfaceType type); DriveInfo *drive_get_next(BlockInterfaceType type); DriveInfo *drive_get_by_blockdev(BlockDriverState *bs); From 6b9e03a4e7598765a6cebb7618f2eeb22e928f6e Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:28 -0400 Subject: [PATCH 12/23] qtest/bios-tables: Correct Q35 command line If the Q35 board types are to begin recognizing and decoding syntactic sugar for drive/device declarations, then workarounds found within the qtests suite need to be adjusted to prevent any test failures after the fix. bios-tables-test improperly uses this cli: -drive file=etc,id=hd -device ide-hd,drive=hd Which will create a drive and device due to the lack of specifying if=none. Then, it will attempt to create a second device and fail. This patch corrects this test to always use the full, non-sugared -device/-drive syntax for both PC and Q35. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Reviewed-by: Michael S. Tsirkin Message-id: 1412187569-23452-6-git-send-email-jsnow@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/bios-tables-test.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 602932b888..9e4d20592b 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -714,14 +714,12 @@ static void test_acpi_one(const char *params, test_data *data) uint8_t signature_high; uint16_t signature; int i; - const char *device = ""; - if (!g_strcmp0(data->machine, MACHINE_Q35)) { - device = ",id=hd -device ide-hd,drive=hd"; - } + args = g_strdup_printf("-net none -display none %s " + "-drive id=hd0,if=none,file=%s " + "-device ide-hd,drive=hd0 ", + params ? params : "", disk); - args = g_strdup_printf("-net none -display none %s -drive file=%s%s,", - params ? params : "", disk, device); qtest_start(args); /* Wait at most 1 minute */ From d93162e13c1f4a5b2a4de6b1997f32e3fca19e67 Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:29 -0400 Subject: [PATCH 13/23] q35/ahci: Pick up -cdrom and -hda options This patch implements the backend for the Q35 board for us to be able to pick up and use drives defined by the -cdrom, -hda, or -drive if=ide shorthand options. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Reviewed-by: Michael S. Tsirkin Message-id: 1412187569-23452-7-git-send-email-jsnow@redhat.com Signed-off-by: Stefan Hajnoczi --- hw/i386/pc_q35.c | 4 ++++ hw/ide/ahci.c | 15 +++++++++++++++ hw/ide/ahci.h | 2 ++ 3 files changed, 21 insertions(+) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index b28ddbb742..bb0dc8e08a 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -86,6 +86,7 @@ static void pc_q35_init(MachineState *machine) DeviceState *icc_bridge; PcGuestInfo *guest_info; ram_addr_t lowmem; + DriveInfo *hd[MAX_SATA_PORTS]; /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping @@ -253,6 +254,9 @@ static void pc_q35_init(MachineState *machine) true, "ich9-ahci"); idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0"); idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1"); + g_assert_cmpint(MAX_SATA_PORTS, ==, ICH_AHCI(ahci)->ahci.ports); + ide_drive_get(hd, ICH_AHCI(ahci)->ahci.ports); + ahci_ide_create_devs(ahci, hd); if (usb_enabled(false)) { /* Should we create 6 UHCI according to ich9 spec? */ diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 8978643fff..063730e8df 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1419,3 +1419,18 @@ static void sysbus_ahci_register_types(void) } type_init(sysbus_ahci_register_types) + +void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd) +{ + AHCIPCIState *d = ICH_AHCI(dev); + AHCIState *ahci = &d->ahci; + int i; + + for (i = 0; i < ahci->ports; i++) { + if (hd[i] == NULL) { + continue; + } + ide_create_drive(&ahci->dev[i].port, 0, hd[i]); + } + +} diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index 1543df7b7d..e223258820 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -332,4 +332,6 @@ void ahci_uninit(AHCIState *s); void ahci_reset(AHCIState *s); +void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd); + #endif /* HW_IDE_AHCI_H */ From f5bebbbb28dc7a149a891f0f1e112fb50bb72664 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 30 Sep 2014 13:59:30 +0200 Subject: [PATCH 14/23] util: Emancipate id_wellformed() from QemuOpts IDs have long spread beyond QemuOpts: not everything with an ID necessarily goes through QemuOpts. Commit 9aebf3b is about such a case: block layer names are meant to be well-formed IDs, but some of them don't go through QemuOpts, and thus weren't checked. The commit fixed that the straightforward way: rename the internal QemuOpts helper id_wellformed() to qemu_opts_id_wellformed() and give it external linkage. Instead of using it directly in block.c, the commit adds wrapper bdrv_is_valid_name(), probably to hide the connection to QemuOpts. Go one logical step further: emancipate IDs from QemuOpts. Rename the function back to id_wellformed(), and put it in another file. While there, clean up its value to bool. Peel off the bdrv_is_valid_name() wrapper. [Replaced stray return 0 with return false to match bool returns used elsewhere in id_wellformed(). --Stefan] Signed-off-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- block.c | 9 ++------- include/qemu-common.h | 3 +++ include/qemu/option.h | 1 - util/Makefile.objs | 1 + util/id.c | 28 ++++++++++++++++++++++++++++ util/qemu-option.c | 17 +---------------- 6 files changed, 35 insertions(+), 24 deletions(-) create mode 100644 util/id.c diff --git a/block.c b/block.c index c5a251c57e..d3aebeb050 100644 --- a/block.c +++ b/block.c @@ -335,18 +335,13 @@ void bdrv_register(BlockDriver *bdrv) QLIST_INSERT_HEAD(&bdrv_drivers, bdrv, list); } -static bool bdrv_is_valid_name(const char *name) -{ - return qemu_opts_id_wellformed(name); -} - /* create a new block device (by default it is empty) */ BlockDriverState *bdrv_new(const char *device_name, Error **errp) { BlockDriverState *bs; int i; - if (*device_name && !bdrv_is_valid_name(device_name)) { + if (*device_name && !id_wellformed(device_name)) { error_setg(errp, "Invalid device name"); return NULL; } @@ -874,7 +869,7 @@ static void bdrv_assign_node_name(BlockDriverState *bs, } /* Check for empty string or invalid characters */ - if (!bdrv_is_valid_name(node_name)) { + if (!id_wellformed(node_name)) { error_setg(errp, "Invalid node name"); return; } diff --git a/include/qemu-common.h b/include/qemu-common.h index dcb57ab4b9..b87e9c2d8b 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -190,6 +190,9 @@ int64_t strtosz_suffix_unit(const char *nptr, char **end, /* used to print char* safely */ #define STR_OR_NULL(str) ((str) ? (str) : "null") +/* id.c */ +bool id_wellformed(const char *id); + /* path.c */ void init_paths(const char *prefix); const char *path(const char *pathname); diff --git a/include/qemu/option.h b/include/qemu/option.h index 945347cc8f..59bea759a2 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -103,7 +103,6 @@ typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaq int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, int abort_on_failure); -int qemu_opts_id_wellformed(const char *id); QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id); QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists, Error **errp); diff --git a/util/Makefile.objs b/util/Makefile.objs index cb8862ba92..93007e2f56 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -8,6 +8,7 @@ util-obj-y += fifo8.o util-obj-y += acl.o util-obj-y += error.o qemu-error.o util-obj-$(CONFIG_POSIX) += compatfd.o +util-obj-y += id.o util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o util-obj-y += qemu-option.o qemu-progress.o util-obj-y += hexdump.o diff --git a/util/id.c b/util/id.c new file mode 100644 index 0000000000..09b22fb8fa --- /dev/null +++ b/util/id.c @@ -0,0 +1,28 @@ +/* + * Dealing with identifiers + * + * Copyright (C) 2014 Red Hat, Inc. + * + * Authors: + * Markus Armbruster , + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 + * or later. See the COPYING.LIB file in the top-level directory. + */ + +#include "qemu-common.h" + +bool id_wellformed(const char *id) +{ + int i; + + if (!qemu_isalpha(id[0])) { + return false; + } + for (i = 1; id[i]; i++) { + if (!qemu_isalnum(id[i]) && !strchr("-._", id[i])) { + return false; + } + } + return true; +} diff --git a/util/qemu-option.c b/util/qemu-option.c index 0cf9960fc5..5d106959ca 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -641,28 +641,13 @@ QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id) return NULL; } -int qemu_opts_id_wellformed(const char *id) -{ - int i; - - if (!qemu_isalpha(id[0])) { - return 0; - } - for (i = 1; id[i]; i++) { - if (!qemu_isalnum(id[i]) && !strchr("-._", id[i])) { - return 0; - } - } - return 1; -} - QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists, Error **errp) { QemuOpts *opts = NULL; if (id) { - if (!qemu_opts_id_wellformed(id)) { + if (!id_wellformed(id)) { error_set(errp,QERR_INVALID_PARAMETER_VALUE, "id", "an identifier"); #if 0 /* conversion from qerror_report() to error_set() broke this: */ error_printf_unless_qmp("Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.\n"); From 1b53eab270ee08e61e21c3fcc77e34c4b5484c30 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 30 Sep 2014 21:31:27 +0200 Subject: [PATCH 15/23] iotests: Use _img_info qemu-img info should only be used directly if the format-specific information or the name of the format is relevant (some tests explicitly test format-specific information; test 082 uses qcow2-specific settings to test the qemu-img interface); otherwise, tests should always use _img_info instead. Test 082 was touched only partially. It does test the qemu-img interface; however, its invocations of qemu-img info are not real tests but rather verifications, so if format-specific information is not important for the test, there is no reason not to use _img_info. In contrast to directly invoking qemu-img info, "qcow2" is replaced by "IMGFMT"; but as "qcow2" is only mentioned once in test 082 (in _supported_fmt), I consider this an improvement. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Message-id: 1412105489-7681-2-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests/070 | 2 +- tests/qemu-iotests/070.out | 5 ++-- tests/qemu-iotests/082 | 12 ++++----- tests/qemu-iotests/082.out | 55 ++++++++------------------------------ tests/qemu-iotests/095 | 4 +-- tests/qemu-iotests/095.out | 16 +++-------- 6 files changed, 26 insertions(+), 68 deletions(-) diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070 index ea0dae7e9c..d649ddf9bd 100755 --- a/tests/qemu-iotests/070 +++ b/tests/qemu-iotests/070 @@ -77,7 +77,7 @@ _use_sample_img test-disk2vhd.vhdx.bz2 echo echo "=== Verify image created by Disk2VHD can be opened ===" -$QEMU_IMG info "$TEST_IMG" 2>&1 | _filter_testdir | _filter_qemu +_img_info # success, all done echo "*** done" diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out index 15f1fc1471..ca743831c9 100644 --- a/tests/qemu-iotests/070.out +++ b/tests/qemu-iotests/070.out @@ -20,9 +20,8 @@ read 18874368/18874368 bytes at offset 0 18 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) === Verify image created by Disk2VHD can be opened === -image: TEST_DIR/test-disk2vhd.vhdx -file format: vhdx +image: TEST_DIR/test-disk2vhd.IMGFMT +file format: IMGFMT virtual size: 256M (268435456 bytes) -disk size: 260M cluster_size: 2097152 *** done diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082 index f6eb75f624..910b13e8f0 100755 --- a/tests/qemu-iotests/082 +++ b/tests/qemu-iotests/082 @@ -56,7 +56,7 @@ echo === create: Options specified more than once === # Last -f should win run_qemu_img create -f foo -f $IMGFMT "$TEST_IMG" $size -run_qemu_img info "$TEST_IMG" +_img_info # Multiple -o should be merged run_qemu_img create -f $IMGFMT -o cluster_size=4k -o lazy_refcounts=on "$TEST_IMG" $size @@ -66,7 +66,7 @@ run_qemu_img info "$TEST_IMG" run_qemu_img create -f $IMGFMT -o cluster_size=4k -o lazy_refcounts=on -o cluster_size=8k "$TEST_IMG" $size run_qemu_img info "$TEST_IMG" run_qemu_img create -f $IMGFMT -o cluster_size=4k,cluster_size=8k "$TEST_IMG" $size -run_qemu_img info "$TEST_IMG" +_img_info echo echo === create: help for -o === @@ -106,11 +106,11 @@ run_qemu_img create -f $IMGFMT "$TEST_IMG" $size # Last -f should win run_qemu_img convert -f foo -f $IMGFMT "$TEST_IMG" "$TEST_IMG".base -run_qemu_img info "$TEST_IMG".base +TEST_IMG="${TEST_IMG}.base" _img_info # Last -O should win run_qemu_img convert -O foo -O $IMGFMT "$TEST_IMG" "$TEST_IMG".base -run_qemu_img info "$TEST_IMG".base +TEST_IMG="${TEST_IMG}.base" _img_info # Multiple -o should be merged run_qemu_img convert -O $IMGFMT -o cluster_size=4k -o lazy_refcounts=on "$TEST_IMG" "$TEST_IMG".base @@ -120,7 +120,7 @@ run_qemu_img info "$TEST_IMG".base run_qemu_img convert -O $IMGFMT -o cluster_size=4k -o lazy_refcounts=on -o cluster_size=8k "$TEST_IMG" "$TEST_IMG".base run_qemu_img info "$TEST_IMG".base run_qemu_img convert -O $IMGFMT -o cluster_size=4k,cluster_size=8k "$TEST_IMG" "$TEST_IMG".base -run_qemu_img info "$TEST_IMG".base +TEST_IMG="${TEST_IMG}.base" _img_info echo echo === convert: help for -o === @@ -167,7 +167,7 @@ run_qemu_img info "$TEST_IMG" run_qemu_img amend -f $IMGFMT -o size=8M -o lazy_refcounts=on -o size=132M "$TEST_IMG" run_qemu_img info "$TEST_IMG" run_qemu_img amend -f $IMGFMT -o size=4M,size=148M "$TEST_IMG" -run_qemu_img info "$TEST_IMG" +_img_info echo echo === amend: help for -o === diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out index 90c21c893b..249c5e4e93 100644 --- a/tests/qemu-iotests/082.out +++ b/tests/qemu-iotests/082.out @@ -4,16 +4,10 @@ QA output created by 082 Testing: create -f foo -f qcow2 TEST_DIR/t.qcow2 128M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=65536 lazy_refcounts=off - -Testing: info TEST_DIR/t.qcow2 -image: TEST_DIR/t.qcow2 -file format: qcow2 +image: TEST_DIR/t.IMGFMT +file format: IMGFMT virtual size: 128M (134217728 bytes) -disk size: 196K cluster_size: 65536 -Format specific information: - compat: 1.1 - lazy refcounts: false Testing: create -f qcow2 -o cluster_size=4k -o lazy_refcounts=on TEST_DIR/t.qcow2 128M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=4096 lazy_refcounts=on @@ -43,16 +37,10 @@ Format specific information: Testing: create -f qcow2 -o cluster_size=4k,cluster_size=8k TEST_DIR/t.qcow2 128M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=8192 lazy_refcounts=off - -Testing: info TEST_DIR/t.qcow2 -image: TEST_DIR/t.qcow2 -file format: qcow2 +image: TEST_DIR/t.IMGFMT +file format: IMGFMT virtual size: 128M (134217728 bytes) -disk size: 28K cluster_size: 8192 -Format specific information: - compat: 1.1 - lazy refcounts: false === create: help for -o === @@ -188,24 +176,15 @@ Testing: create -f qcow2 TEST_DIR/t.qcow2 128M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=65536 lazy_refcounts=off Testing: convert -f foo -f qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base - -Testing: info TEST_DIR/t.qcow2.base -image: TEST_DIR/t.qcow2.base +image: TEST_DIR/t.IMGFMT.base file format: raw virtual size: 128M (134217728 bytes) -disk size: 0 Testing: convert -O foo -O qcow2 TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base - -Testing: info TEST_DIR/t.qcow2.base -image: TEST_DIR/t.qcow2.base -file format: qcow2 +image: TEST_DIR/t.IMGFMT.base +file format: IMGFMT virtual size: 128M (134217728 bytes) -disk size: 196K cluster_size: 65536 -Format specific information: - compat: 1.1 - lazy refcounts: false Testing: convert -O qcow2 -o cluster_size=4k -o lazy_refcounts=on TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base @@ -232,16 +211,10 @@ Format specific information: lazy refcounts: true Testing: convert -O qcow2 -o cluster_size=4k,cluster_size=8k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base - -Testing: info TEST_DIR/t.qcow2.base -image: TEST_DIR/t.qcow2.base -file format: qcow2 +image: TEST_DIR/t.IMGFMT.base +file format: IMGFMT virtual size: 128M (134217728 bytes) -disk size: 28K cluster_size: 8192 -Format specific information: - compat: 1.1 - lazy refcounts: false === convert: help for -o === @@ -410,16 +383,10 @@ Format specific information: lazy refcounts: true Testing: amend -f qcow2 -o size=4M,size=148M TEST_DIR/t.qcow2 - -Testing: info TEST_DIR/t.qcow2 -image: TEST_DIR/t.qcow2 -file format: qcow2 +image: TEST_DIR/t.IMGFMT +file format: IMGFMT virtual size: 148M (155189248 bytes) -disk size: 196K cluster_size: 65536 -Format specific information: - compat: 1.1 - lazy refcounts: true === amend: help for -o === diff --git a/tests/qemu-iotests/095 b/tests/qemu-iotests/095 index acc7dbf182..6630181a78 100755 --- a/tests/qemu-iotests/095 +++ b/tests/qemu-iotests/095 @@ -60,7 +60,7 @@ _make_test_img -b "${TEST_IMG}.snp1" $size_larger echo echo "=== Base image info before commit and resize ===" -$QEMU_IMG info "${TEST_IMG}.base" | _filter_testdir +TEST_IMG="${TEST_IMG}.base" _img_info echo echo === Running QEMU Live Commit Test === @@ -78,7 +78,7 @@ _send_qemu_cmd $h "{ 'execute': 'block-commit', echo echo "=== Base image info after commit and resize ===" -$QEMU_IMG info "${TEST_IMG}.base" | _filter_testdir +TEST_IMG="${TEST_IMG}.base" _img_info # success, all done echo "*** done" diff --git a/tests/qemu-iotests/095.out b/tests/qemu-iotests/095.out index 5864ddac2b..cc86efacc6 100644 --- a/tests/qemu-iotests/095.out +++ b/tests/qemu-iotests/095.out @@ -4,14 +4,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=104857600 backing_file='TEST_DIR Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=104857600 backing_file='TEST_DIR/t.IMGFMT.snp1' === Base image info before commit and resize === -image: TEST_DIR/t.qcow2.base -file format: qcow2 +image: TEST_DIR/t.IMGFMT.base +file format: IMGFMT virtual size: 5.0M (5242880 bytes) -disk size: 196K cluster_size: 65536 -Format specific information: - compat: 1.1 - lazy refcounts: false === Running QEMU Live Commit Test === @@ -20,12 +16,8 @@ Format specific information: {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "test", "len": 104857600, "offset": 104857600, "speed": 0, "type": "commit"}} === Base image info after commit and resize === -image: TEST_DIR/t.qcow2.base -file format: qcow2 +image: TEST_DIR/t.IMGFMT.base +file format: IMGFMT virtual size: 100M (104857600 bytes) -disk size: 196K cluster_size: 65536 -Format specific information: - compat: 1.1 - lazy refcounts: false *** done From 9009b1963c5ed9bb826c8116e8b1d3aa94d47f85 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 30 Sep 2014 21:31:28 +0200 Subject: [PATCH 16/23] qapi: Add corrupt field to ImageInfoSpecificQCow2 Just like lazy-refcounts, this field will be present iff the qcow2 compat level is 1.1 (or probably any future revision). As expected, this breaks some tests due to the new field present in qemu-img info output; so fix their output accordingly. Suggested-by: Eric Blake Signed-off-by: Max Reitz Reviewed-by: Eric Blake Message-id: 1412105489-7681-3-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- block/qcow2.c | 3 +++ qapi/block-core.json | 6 +++++- tests/qemu-iotests/065 | 12 ++++++------ tests/qemu-iotests/067.out | 10 +++++----- tests/qemu-iotests/082.out | 7 +++++++ tests/qemu-iotests/089.out | 2 ++ 6 files changed, 28 insertions(+), 12 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 778fc1ec13..fb28493c02 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2282,6 +2282,9 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs) .lazy_refcounts = s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS, .has_lazy_refcounts = true, + .corrupt = s->incompatible_features & + QCOW2_INCOMPAT_CORRUPT, + .has_corrupt = true, }; } diff --git a/qapi/block-core.json b/qapi/block-core.json index fa2d1b7528..8f7089e054 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -38,12 +38,16 @@ # # @lazy-refcounts: #optional on or off; only valid for compat >= 1.1 # +# @corrupt: #optional true if the image has been marked corrupt; only valid for +# compat >= 1.1 (since 2.2) +# # Since: 1.7 ## { 'type': 'ImageInfoSpecificQCow2', 'data': { 'compat': 'str', - '*lazy-refcounts': 'bool' + '*lazy-refcounts': 'bool', + '*corrupt': 'bool' } } ## diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065 index e89b61d70b..8d3a9c9afd 100755 --- a/tests/qemu-iotests/065 +++ b/tests/qemu-iotests/065 @@ -94,28 +94,28 @@ class TestQCow2(TestQemuImgInfo): class TestQCow3NotLazy(TestQemuImgInfo): '''Testing a qcow2 version 3 image with lazy refcounts disabled''' img_options = 'compat=1.1,lazy_refcounts=off' - json_compare = { 'compat': '1.1', 'lazy-refcounts': False } - human_compare = [ 'compat: 1.1', 'lazy refcounts: false' ] + json_compare = { 'compat': '1.1', 'lazy-refcounts': False, 'corrupt': False } + human_compare = [ 'compat: 1.1', 'lazy refcounts: false', 'corrupt: false' ] class TestQCow3Lazy(TestQemuImgInfo): '''Testing a qcow2 version 3 image with lazy refcounts enabled''' img_options = 'compat=1.1,lazy_refcounts=on' - json_compare = { 'compat': '1.1', 'lazy-refcounts': True } - human_compare = [ 'compat: 1.1', 'lazy refcounts: true' ] + json_compare = { 'compat': '1.1', 'lazy-refcounts': True, 'corrupt': False } + human_compare = [ 'compat: 1.1', 'lazy refcounts: true', 'corrupt: false' ] class TestQCow3NotLazyQMP(TestQMP): '''Testing a qcow2 version 3 image with lazy refcounts disabled, opening with lazy refcounts enabled''' img_options = 'compat=1.1,lazy_refcounts=off' qemu_options = 'lazy-refcounts=on' - compare = { 'compat': '1.1', 'lazy-refcounts': False } + compare = { 'compat': '1.1', 'lazy-refcounts': False, 'corrupt': False } class TestQCow3LazyQMP(TestQMP): '''Testing a qcow2 version 3 image with lazy refcounts enabled, opening with lazy refcounts disabled''' img_options = 'compat=1.1,lazy_refcounts=on' qemu_options = 'lazy-refcounts=off' - compare = { 'compat': '1.1', 'lazy-refcounts': True } + compare = { 'compat': '1.1', 'lazy-refcounts': True, 'corrupt': False } TestImageInfoSpecific = None TestQemuImgInfo = None diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out index 7e090b95ab..0f72dcf63a 100644 --- a/tests/qemu-iotests/067.out +++ b/tests/qemu-iotests/067.out @@ -6,7 +6,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virtio-blk-pci,drive=disk,id=virtio0 QMP_VERSION {"return": {}} -{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} +{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} {"return": {}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/virtio0/virtio-backend"}} @@ -24,7 +24,7 @@ QMP_VERSION Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk QMP_VERSION {"return": {}} -{"return": [{"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} +{"return": [{"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} {"return": {}} {"return": {}} {"return": {}} @@ -44,7 +44,7 @@ Testing: QMP_VERSION {"return": {}} {"return": "OK\r\n"} -{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]} +{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]} {"return": {}} {"return": {}} {"return": {}} @@ -64,14 +64,14 @@ Testing: QMP_VERSION {"return": {}} {"return": {}} -{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]} +{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]} {"return": {}} {"return": {}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/virtio0/virtio-backend"}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"device": "virtio0", "path": "/machine/peripheral/virtio0"}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESET"} -{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]} +{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out index 249c5e4e93..0a3ab5ac90 100644 --- a/tests/qemu-iotests/082.out +++ b/tests/qemu-iotests/082.out @@ -21,6 +21,7 @@ cluster_size: 4096 Format specific information: compat: 1.1 lazy refcounts: true + corrupt: false Testing: create -f qcow2 -o cluster_size=4k -o lazy_refcounts=on -o cluster_size=8k TEST_DIR/t.qcow2 128M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=8192 lazy_refcounts=on @@ -34,6 +35,7 @@ cluster_size: 8192 Format specific information: compat: 1.1 lazy refcounts: true + corrupt: false Testing: create -f qcow2 -o cluster_size=4k,cluster_size=8k TEST_DIR/t.qcow2 128M Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=134217728 encryption=off cluster_size=8192 lazy_refcounts=off @@ -197,6 +199,7 @@ cluster_size: 4096 Format specific information: compat: 1.1 lazy refcounts: true + corrupt: false Testing: convert -O qcow2 -o cluster_size=4k -o lazy_refcounts=on -o cluster_size=8k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base @@ -209,6 +212,7 @@ cluster_size: 8192 Format specific information: compat: 1.1 lazy refcounts: true + corrupt: false Testing: convert -O qcow2 -o cluster_size=4k,cluster_size=8k TEST_DIR/t.qcow2 TEST_DIR/t.qcow2.base image: TEST_DIR/t.IMGFMT.base @@ -357,6 +361,7 @@ cluster_size: 65536 Format specific information: compat: 1.1 lazy refcounts: true + corrupt: false Testing: amend -f qcow2 -o size=130M -o lazy_refcounts=off TEST_DIR/t.qcow2 @@ -369,6 +374,7 @@ cluster_size: 65536 Format specific information: compat: 1.1 lazy refcounts: false + corrupt: false Testing: amend -f qcow2 -o size=8M -o lazy_refcounts=on -o size=132M TEST_DIR/t.qcow2 @@ -381,6 +387,7 @@ cluster_size: 65536 Format specific information: compat: 1.1 lazy refcounts: true + corrupt: false Testing: amend -f qcow2 -o size=4M,size=148M TEST_DIR/t.qcow2 image: TEST_DIR/t.IMGFMT diff --git a/tests/qemu-iotests/089.out b/tests/qemu-iotests/089.out index 4ca2f88e6a..b2b0390818 100644 --- a/tests/qemu-iotests/089.out +++ b/tests/qemu-iotests/089.out @@ -41,10 +41,12 @@ vm state offset: 512 MiB Format specific information: compat: 1.1 lazy refcounts: false + corrupt: false format name: IMGFMT cluster size: 64 KiB vm state offset: 512 MiB Format specific information: compat: 1.1 lazy refcounts: false + corrupt: false *** done From f383611a0a464e0bd06da9d98ab0d63f987cb885 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 30 Sep 2014 21:31:29 +0200 Subject: [PATCH 17/23] iotests: qemu-img info output for corrupt image The "corrupt" entry in the format-specific information section should be "true". Signed-off-by: Max Reitz Reviewed-by: Eric Blake Message-id: 1412105489-7681-4-git-send-email-mreitz@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests/060 | 3 +++ tests/qemu-iotests/060.out | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index 2355567951..9772d365ae 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -76,6 +76,9 @@ $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io # The corrupt bit must now be set $PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +# This information should be available through qemu-img info +$QEMU_IMG info "$TEST_IMG" | _filter_testdir + # Try to open the image R/W (which should fail) $QEMU_IO -c "$OPEN_RW" -c "read 0 512" 2>&1 | _filter_qemu_io \ | _filter_testdir \ diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index 4f0c6d0c8e..cd679f9454 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -11,6 +11,15 @@ incompatible_features 0x0 qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with active L1 table); further corruption events will be suppressed write failed: Input/output error incompatible_features 0x2 +image: TEST_DIR/t.qcow2 +file format: qcow2 +virtual size: 64M (67108864 bytes) +disk size: 196K +cluster_size: 65536 +Format specific information: + compat: 1.1 + lazy refcounts: false + corrupt: true qemu-io: can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot be opened read/write read 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) From e2f3f221885a90de766ce9a38b87badeb658635a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 2 Oct 2014 16:51:31 +0200 Subject: [PATCH 18/23] drive_del-test: Merge of qdev-monitor-test, blockdev-test Each of qdev-monitor-test and blockdev-test has just one test case, and both are about drive_del. [Extended copyright from 2013 to 2013-2014 as requested by Eric Blake. --Stefan] Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-id: 1412261496-24455-2-git-send-email-armbru@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/Makefile | 5 +- tests/blockdev-test.c | 59 ------------------- .../{qdev-monitor-test.c => drive_del-test.c} | 59 +++++++++++++++---- 3 files changed, 48 insertions(+), 75 deletions(-) delete mode 100644 tests/blockdev-test.c rename tests/{qdev-monitor-test.c => drive_del-test.c} (55%) diff --git a/tests/Makefile b/tests/Makefile index 834279cd0d..ffa8312eb5 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -140,8 +140,7 @@ check-qtest-i386-y += tests/bios-tables-test$(EXESUF) check-qtest-i386-y += tests/rtc-test$(EXESUF) check-qtest-i386-y += tests/i440fx-test$(EXESUF) check-qtest-i386-y += tests/fw_cfg-test$(EXESUF) -check-qtest-i386-y += tests/blockdev-test$(EXESUF) -check-qtest-i386-y += tests/qdev-monitor-test$(EXESUF) +check-qtest-i386-y += tests/drive_del-test$(EXESUF) check-qtest-i386-y += tests/wdt_ib700-test$(EXESUF) gcov-files-i386-y += hw/watchdog/watchdog.c hw/watchdog/wdt_ib700.c check-qtest-i386-y += $(check-qtest-pci-y) @@ -335,7 +334,7 @@ tests/tpci200-test$(EXESUF): tests/tpci200-test.o tests/display-vga-test$(EXESUF): tests/display-vga-test.o tests/ipoctal232-test$(EXESUF): tests/ipoctal232-test.o tests/qom-test$(EXESUF): tests/qom-test.o -tests/blockdev-test$(EXESUF): tests/blockdev-test.o $(libqos-pc-obj-y) +tests/drive_del-test$(EXESUF): tests/drive_del-test.o $(libqos-pc-obj-y) tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-pc-obj-y) tests/nvme-test$(EXESUF): tests/nvme-test.o tests/pvpanic-test$(EXESUF): tests/pvpanic-test.o diff --git a/tests/blockdev-test.c b/tests/blockdev-test.c deleted file mode 100644 index c940e00690..0000000000 --- a/tests/blockdev-test.c +++ /dev/null @@ -1,59 +0,0 @@ -/* - * blockdev.c test cases - * - * Copyright (C) 2013 Red Hat Inc. - * - * Authors: - * Stefan Hajnoczi - * - * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. - * See the COPYING.LIB file in the top-level directory. - */ - -#include -#include -#include "libqtest.h" - -static void test_drive_add_empty(void) -{ - QDict *response; - const char *response_return; - - /* Start with an empty drive */ - qtest_start("-drive if=none,id=drive0"); - - /* Delete the drive */ - response = qmp("{\"execute\": \"human-monitor-command\"," - " \"arguments\": {" - " \"command-line\": \"drive_del drive0\"" - "}}"); - g_assert(response); - response_return = qdict_get_try_str(response, "return"); - g_assert(response_return); - g_assert(strcmp(response_return, "") == 0); - QDECREF(response); - - /* Ensure re-adding the drive works - there should be no duplicate ID error - * because the old drive must be gone. - */ - response = qmp("{\"execute\": \"human-monitor-command\"," - " \"arguments\": {" - " \"command-line\": \"drive_add 0 if=none,id=drive0\"" - "}}"); - g_assert(response); - response_return = qdict_get_try_str(response, "return"); - g_assert(response_return); - g_assert(strcmp(response_return, "OK\r\n") == 0); - QDECREF(response); - - qtest_end(); -} - -int main(int argc, char **argv) -{ - g_test_init(&argc, &argv, NULL); - - qtest_add_func("/qmp/drive_add_empty", test_drive_add_empty); - - return g_test_run(); -} diff --git a/tests/qdev-monitor-test.c b/tests/drive_del-test.c similarity index 55% rename from tests/qdev-monitor-test.c rename to tests/drive_del-test.c index e20ffd67a7..39c56fc78c 100644 --- a/tests/qdev-monitor-test.c +++ b/tests/drive_del-test.c @@ -1,7 +1,7 @@ /* - * qdev-monitor.c test cases + * blockdev.c test cases * - * Copyright (C) 2013 Red Hat Inc. + * Copyright (C) 2013-2014 Red Hat Inc. * * Authors: * Stefan Hajnoczi @@ -10,12 +10,46 @@ * See the COPYING.LIB file in the top-level directory. */ -#include #include +#include #include "libqtest.h" -#include "qapi/qmp/qjson.h" -static void test_device_add(void) +static void test_drive_without_dev(void) +{ + QDict *response; + const char *response_return; + + /* Start with an empty drive */ + qtest_start("-drive if=none,id=drive0"); + + /* Delete the drive */ + response = qmp("{\"execute\": \"human-monitor-command\"," + " \"arguments\": {" + " \"command-line\": \"drive_del drive0\"" + "}}"); + g_assert(response); + response_return = qdict_get_try_str(response, "return"); + g_assert(response_return); + g_assert(strcmp(response_return, "") == 0); + QDECREF(response); + + /* Ensure re-adding the drive works - there should be no duplicate ID error + * because the old drive must be gone. + */ + response = qmp("{\"execute\": \"human-monitor-command\"," + " \"arguments\": {" + " \"command-line\": \"drive_add 0 if=none,id=drive0\"" + "}}"); + g_assert(response); + response_return = qdict_get_try_str(response, "return"); + g_assert(response_return); + g_assert(strcmp(response_return, "OK\r\n") == 0); + QDECREF(response); + + qtest_end(); +} + +static void test_after_failed_device_add(void) { QDict *response; QDict *error; @@ -62,16 +96,15 @@ int main(int argc, char **argv) { const char *arch = qtest_get_arch(); - /* Check architecture */ - if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) { - g_test_message("Skipping test for non-x86\n"); - return 0; - } - - /* Run the tests */ g_test_init(&argc, &argv, NULL); - qtest_add_func("/qmp/device_add", test_device_add); + qtest_add_func("/drive_del/without-dev", test_drive_without_dev); + + /* TODO I guess any arch with PCI would do */ + if (!strcmp(arch, "i386") || !strcmp(arch, "x86_64")) { + qtest_add_func("/drive_del/after_failed_device_add", + test_after_failed_device_add); + } return g_test_run(); } From d0e386683779f09afe5bf198b957a780c960204b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 2 Oct 2014 16:51:32 +0200 Subject: [PATCH 19/23] blockdev-test: Use single rather than double quotes in QMP QMP accepts both single and double quotes. This is the only test using double quotes. They need to be quoted in C strings. Replace them by single quotes. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-id: 1412261496-24455-3-git-send-email-armbru@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/drive_del-test.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c index 39c56fc78c..a5de2395d6 100644 --- a/tests/drive_del-test.c +++ b/tests/drive_del-test.c @@ -23,9 +23,9 @@ static void test_drive_without_dev(void) qtest_start("-drive if=none,id=drive0"); /* Delete the drive */ - response = qmp("{\"execute\": \"human-monitor-command\"," - " \"arguments\": {" - " \"command-line\": \"drive_del drive0\"" + response = qmp("{'execute': 'human-monitor-command'," + " 'arguments': {" + " 'command-line': 'drive_del drive0'" "}}"); g_assert(response); response_return = qdict_get_try_str(response, "return"); @@ -36,9 +36,9 @@ static void test_drive_without_dev(void) /* Ensure re-adding the drive works - there should be no duplicate ID error * because the old drive must be gone. */ - response = qmp("{\"execute\": \"human-monitor-command\"," - " \"arguments\": {" - " \"command-line\": \"drive_add 0 if=none,id=drive0\"" + response = qmp("{'execute': 'human-monitor-command'," + " 'arguments': {" + " 'command-line': 'drive_add 0 if=none,id=drive0'" "}}"); g_assert(response); response_return = qdict_get_try_str(response, "return"); @@ -59,10 +59,10 @@ static void test_after_failed_device_add(void) /* Make device_add fail. If this leaks the virtio-blk-pci device then a * reference to drive0 will also be held (via qdev properties). */ - response = qmp("{\"execute\": \"device_add\"," - " \"arguments\": {" - " \"driver\": \"virtio-blk-pci\"," - " \"drive\": \"drive0\"" + response = qmp("{'execute': 'device_add'," + " 'arguments': {" + " 'driver': 'virtio-blk-pci'," + " 'drive': 'drive0'" "}}"); g_assert(response); error = qdict_get_qdict(response, "error"); @@ -70,9 +70,9 @@ static void test_after_failed_device_add(void) QDECREF(response); /* Delete the drive */ - response = qmp("{\"execute\": \"human-monitor-command\"," - " \"arguments\": {" - " \"command-line\": \"drive_del drive0\"" + response = qmp("{'execute': 'human-monitor-command'," + " 'arguments': {" + " 'command-line': 'drive_del drive0'" "}}"); g_assert(response); g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, ""); @@ -81,9 +81,9 @@ static void test_after_failed_device_add(void) /* Try to re-add the drive. This fails with duplicate IDs if a leaked * virtio-blk-pci exists that holds a reference to the old drive0. */ - response = qmp("{\"execute\": \"human-monitor-command\"," - " \"arguments\": {" - " \"command-line\": \"drive_add pci-addr=auto if=none,id=drive0\"" + response = qmp("{'execute': 'human-monitor-command'," + " 'arguments': {" + " 'command-line': 'drive_add pci-addr=auto if=none,id=drive0'" "}}"); g_assert(response); g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "OK\r\n"); From e319df669da92f6ba56595e486637108256da08f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 2 Oct 2014 16:51:33 +0200 Subject: [PATCH 20/23] blockdev-test: Clean up bogus drive_add argument The first argument should be a PCI address, which pci-addr=auto isn't. Doesn't really matter, as drive_add ignores its first argument when its second argument has if=none. Clean it up anyway. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-id: 1412261496-24455-4-git-send-email-armbru@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/drive_del-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c index a5de2395d6..32516a251c 100644 --- a/tests/drive_del-test.c +++ b/tests/drive_del-test.c @@ -83,7 +83,7 @@ static void test_after_failed_device_add(void) */ response = qmp("{'execute': 'human-monitor-command'," " 'arguments': {" - " 'command-line': 'drive_add pci-addr=auto if=none,id=drive0'" + " 'command-line': 'drive_add 0 if=none,id=drive0'" "}}"); g_assert(response); g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "OK\r\n"); From 37e153fe45020c38229a113adc1eda05757e2c37 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 2 Oct 2014 16:51:34 +0200 Subject: [PATCH 21/23] blockdev-test: Simplify by using g_assert_cmpstr() Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-id: 1412261496-24455-5-git-send-email-armbru@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/drive_del-test.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c index 32516a251c..80917ad86b 100644 --- a/tests/drive_del-test.c +++ b/tests/drive_del-test.c @@ -17,7 +17,6 @@ static void test_drive_without_dev(void) { QDict *response; - const char *response_return; /* Start with an empty drive */ qtest_start("-drive if=none,id=drive0"); @@ -28,9 +27,7 @@ static void test_drive_without_dev(void) " 'command-line': 'drive_del drive0'" "}}"); g_assert(response); - response_return = qdict_get_try_str(response, "return"); - g_assert(response_return); - g_assert(strcmp(response_return, "") == 0); + g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, ""); QDECREF(response); /* Ensure re-adding the drive works - there should be no duplicate ID error @@ -41,9 +38,7 @@ static void test_drive_without_dev(void) " 'command-line': 'drive_add 0 if=none,id=drive0'" "}}"); g_assert(response); - response_return = qdict_get_try_str(response, "return"); - g_assert(response_return); - g_assert(strcmp(response_return, "OK\r\n") == 0); + g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "OK\r\n"); QDECREF(response); qtest_end(); From 2eea5cd452592ac9b31ed498555f36273b3e0a94 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 2 Oct 2014 16:51:35 +0200 Subject: [PATCH 22/23] blockdev-test: Factor out some common code into helpers Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-id: 1412261496-24455-6-git-send-email-armbru@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/drive_del-test.c | 60 +++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c index 80917ad86b..38f8229782 100644 --- a/tests/drive_del-test.c +++ b/tests/drive_del-test.c @@ -14,25 +14,10 @@ #include #include "libqtest.h" -static void test_drive_without_dev(void) +static void drive_add(void) { QDict *response; - /* Start with an empty drive */ - qtest_start("-drive if=none,id=drive0"); - - /* Delete the drive */ - response = qmp("{'execute': 'human-monitor-command'," - " 'arguments': {" - " 'command-line': 'drive_del drive0'" - "}}"); - g_assert(response); - g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, ""); - QDECREF(response); - - /* Ensure re-adding the drive works - there should be no duplicate ID error - * because the old drive must be gone. - */ response = qmp("{'execute': 'human-monitor-command'," " 'arguments': {" " 'command-line': 'drive_add 0 if=none,id=drive0'" @@ -40,6 +25,33 @@ static void test_drive_without_dev(void) g_assert(response); g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "OK\r\n"); QDECREF(response); +} + +static void drive_del(void) +{ + QDict *response; + + response = qmp("{'execute': 'human-monitor-command'," + " 'arguments': {" + " 'command-line': 'drive_del drive0'" + "}}"); + g_assert(response); + g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, ""); + QDECREF(response); +} + +static void test_drive_without_dev(void) +{ + /* Start with an empty drive */ + qtest_start("-drive if=none,id=drive0"); + + /* Delete the drive */ + drive_del(); + + /* Ensure re-adding the drive works - there should be no duplicate ID error + * because the old drive must be gone. + */ + drive_add(); qtest_end(); } @@ -65,24 +77,12 @@ static void test_after_failed_device_add(void) QDECREF(response); /* Delete the drive */ - response = qmp("{'execute': 'human-monitor-command'," - " 'arguments': {" - " 'command-line': 'drive_del drive0'" - "}}"); - g_assert(response); - g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, ""); - QDECREF(response); + drive_del(); /* Try to re-add the drive. This fails with duplicate IDs if a leaked * virtio-blk-pci exists that holds a reference to the old drive0. */ - response = qmp("{'execute': 'human-monitor-command'," - " 'arguments': {" - " 'command-line': 'drive_add 0 if=none,id=drive0'" - "}}"); - g_assert(response); - g_assert_cmpstr(qdict_get_try_str(response, "return"), ==, "OK\r\n"); - QDECREF(response); + drive_add(); qtest_end(); } From 767c86d3e752dfc68ff5d018c3b0b63b333371b2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 2 Oct 2014 16:51:36 +0200 Subject: [PATCH 23/23] blockdev-test: Test device_del after drive_del Executed in this order, drive_del and device_del's automatic drive deletion take notoriously tricky special paths. [Fixed "an device" -> "a device" typo as requested by Eric Blake. --Stefan] Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-id: 1412261496-24455-7-git-send-email-armbru@redhat.com Signed-off-by: Stefan Hajnoczi --- tests/drive_del-test.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c index 38f8229782..53fa969260 100644 --- a/tests/drive_del-test.c +++ b/tests/drive_del-test.c @@ -40,6 +40,19 @@ static void drive_del(void) QDECREF(response); } +static void device_del(void) +{ + QDict *response; + + /* Complication: ignore DEVICE_DELETED event */ + qmp_discard_response("{'execute': 'device_del'," + " 'arguments': { 'id': 'dev0' } }"); + response = qmp_receive(); + g_assert(response); + g_assert(qdict_haskey(response, "return")); + QDECREF(response); +} + static void test_drive_without_dev(void) { /* Start with an empty drive */ @@ -87,6 +100,23 @@ static void test_after_failed_device_add(void) qtest_end(); } +static void test_drive_del_device_del(void) +{ + /* Start with a drive used by a device that unplugs instantaneously */ + qtest_start("-drive if=none,id=drive0,file=/dev/null" + " -device virtio-scsi-pci" + " -device scsi-hd,drive=drive0,id=dev0"); + + /* + * Delete the drive, and then the device + * Doing it in this order takes notoriously tricky special paths + */ + drive_del(); + device_del(); + + qtest_end(); +} + int main(int argc, char **argv) { const char *arch = qtest_get_arch(); @@ -99,6 +129,8 @@ int main(int argc, char **argv) if (!strcmp(arch, "i386") || !strcmp(arch, "x86_64")) { qtest_add_func("/drive_del/after_failed_device_add", test_after_failed_device_add); + qtest_add_func("/blockdev/drive_del_device_del", + test_drive_del_device_del); } return g_test_run();