From fbf28a4328123b3259d100eedc0e6f5b7f8bf186 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 29 Sep 2014 16:07:55 +0200 Subject: 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 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'blockdev.c') 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); -- cgit 1.4.1 From 20d6cd47d0459bdc6a8b64e7abe8f713e0d4718e Mon Sep 17 00:00:00 2001 From: Jun Li Date: Wed, 24 Sep 2014 13:45:27 +0800 Subject: 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(+) (limited to 'blockdev.c') 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); } -- cgit 1.4.1 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 --- blockdev.c | 21 +++++++++++++++++++++ include/sysemu/blockdev.h | 2 ++ vl.c | 10 +++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) (limited to 'blockdev.c') 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(); -- cgit 1.4.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 --- blockdev.c | 26 +++++++++++++++++++++++++- include/sysemu/blockdev.h | 2 ++ 2 files changed, 27 insertions(+), 1 deletion(-) (limited to 'blockdev.c') 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); -- cgit 1.4.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 --- 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(-) (limited to 'blockdev.c') 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); -- cgit 1.4.1