From a66c9dc734fb30de1e18e9dc217f2d37e16c492a Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:24 -0400 Subject: 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 --- include/sysemu/blockdev.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index abec381..3040286 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); -- cgit v1.1 From 21dff8cf38d311a917ab33f19d5cea7696f0c354 Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:25 -0400 Subject: 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 --- include/sysemu/blockdev.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index 3040286..a4033d4 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); -- cgit v1.1 From 1602651833c081e32366c9e534ad72e4287840c5 Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:26 -0400 Subject: 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 --- include/hw/boards.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/hw/boards.h b/include/hw/boards.h index dfb6718..663f16a 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, -- cgit v1.1 From d8f94e1bb275ab6a14a15220fd6afd0d04324aeb Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 1 Oct 2014 14:19:27 -0400 Subject: 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 --- include/sysemu/blockdev.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index a4033d4..09716de 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); -- cgit v1.1 From f5bebbbb28dc7a149a891f0f1e112fb50bb72664 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 30 Sep 2014 13:59:30 +0200 Subject: 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 --- include/qemu-common.h | 3 +++ include/qemu/option.h | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/qemu-common.h b/include/qemu-common.h index dcb57ab..b87e9c2 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 945347c..59bea75 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); -- cgit v1.1