From ce0abca3e35a9f95e9edcb5d6b2910b2fcd52099 Mon Sep 17 00:00:00 2001 From: Amos Kong Date: Wed, 16 Apr 2014 09:57:14 +0800 Subject: [PATCH 01/20] qdev: Fix crash by validating the object type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QEMU crashed when I try to list device parameters and the driver name is actually an available bus name. # qemu -device virtio-pci-bus,? # qemu -device virtio-bus,? # qemu -device virtio-serial-bus,? qdev-monitor.c:212:qdev_device_help: Object 0x7fd932f50620 is not an instance of type device Aborted (core dumped) We can also reproduce this bug by adding device from monitor, so it's worth to fix the crash. (qemu) device_add virtio-serial-bus qdev-monitor.c:491:qdev_device_add: Object 0x7f5e89530920 is not an instance of type device Aborted (core dumped) Cc: qemu-stable@nongnu.org Signed-off-by: Amos Kong Reviewed-by: Markus Armbruster Signed-off-by: Andreas Färber --- qdev-monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qdev-monitor.c b/qdev-monitor.c index 6189780fd7..02cbe43bce 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -206,7 +206,7 @@ int qdev_device_help(QemuOpts *opts) } } - if (!klass) { + if (!object_class_dynamic_cast(klass, TYPE_DEVICE)) { return 0; } do { From 9e1d668ba9783483284e081662b8a19fd48846b0 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Wed, 9 Apr 2014 20:34:49 +0300 Subject: [PATCH 02/20] machine: Remove obsoleted field from QEMUMachine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This field shouldn't be used any more since we adopted the QOM way of iterating over the types. The commit that obsoleted it is: commit 261747f176f6f2d88f8268aaebfdd1a1afe887e2 vl: Use MachineClass instead of global QEMUMachine list The machine registration flow is refactored to use the QOM functionality. Instead of linking the machines into a list, each machine has a type and the types can be traversed in the QOM way. Signed-off-by: Marcel Apfelbaum Signed-off-by: Andreas Färber --- include/hw/boards.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/hw/boards.h b/include/hw/boards.h index dd2c70da36..aaaa871687 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -46,7 +46,6 @@ struct QEMUMachine { const char *default_machine_opts; const char *default_boot_order; GlobalProperty *compat_props; - struct QEMUMachine *next; const char *hw_version; }; From 00b4fbe27452ddc346f7f38d5690686166932588 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Wed, 9 Apr 2014 20:34:50 +0300 Subject: [PATCH 03/20] machine: Copy QEMUMachine's fields to MachineClass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to eliminate the QEMUMachine indirection, add its fields directly to MachineClass. Do not yet remove qemu_machine field because it is still in use by sPAPR. Signed-off-by: Marcel Apfelbaum [AF: Copied fields for sPAPR, too] Signed-off-by: Andreas Färber --- hw/ppc/spapr.c | 24 ++++++++++++++++++++++++ include/hw/boards.h | 24 ++++++++++++++++++++++++ vl.c | 23 +++++++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a11e1217b9..5192702504 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1493,8 +1493,32 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc); + QEMUMachine *qm = data; mc->qemu_machine = data; + + mc->name = qm->name; + mc->alias = qm->alias; + mc->desc = qm->desc; + mc->init = qm->init; + mc->reset = qm->reset; + mc->hot_add_cpu = qm->hot_add_cpu; + mc->kvm_type = qm->kvm_type; + mc->block_default_type = qm->block_default_type; + mc->max_cpus = qm->max_cpus; + mc->no_serial = qm->no_serial; + mc->no_parallel = qm->no_parallel; + mc->use_virtcon = qm->use_virtcon; + mc->use_sclp = qm->use_sclp; + mc->no_floppy = qm->no_floppy; + mc->no_cdrom = qm->no_cdrom; + mc->no_sdcard = qm->no_sdcard; + mc->is_default = qm->is_default; + mc->default_machine_opts = qm->default_machine_opts; + mc->default_boot_order = qm->default_boot_order; + mc->compat_props = qm->compat_props; + mc->hw_version = qm->hw_version; + fwc->get_dev_path = spapr_get_fw_dev_path; } diff --git a/include/hw/boards.h b/include/hw/boards.h index aaaa871687..c5f503d0f0 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -77,6 +77,30 @@ struct MachineClass { /*< public >*/ QEMUMachine *qemu_machine; + + const char *name; + const char *alias; + const char *desc; + + void (*init)(QEMUMachineInitArgs *args); + void (*reset)(void); + void (*hot_add_cpu)(const int64_t id, Error **errp); + int (*kvm_type)(const char *arg); + + BlockInterfaceType block_default_type; + int max_cpus; + unsigned int no_serial:1, + no_parallel:1, + use_virtcon:1, + use_sclp:1, + no_floppy:1, + no_cdrom:1, + no_sdcard:1; + int is_default; + const char *default_machine_opts; + const char *default_boot_order; + GlobalProperty *compat_props; + const char *hw_version; }; /** diff --git a/vl.c b/vl.c index 236f95efd7..3229c2ff12 100644 --- a/vl.c +++ b/vl.c @@ -1588,8 +1588,31 @@ MachineState *current_machine; static void machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); + QEMUMachine *qm = data; mc->qemu_machine = data; + + mc->name = qm->name; + mc->alias = qm->alias; + mc->desc = qm->desc; + mc->init = qm->init; + mc->reset = qm->reset; + mc->hot_add_cpu = qm->hot_add_cpu; + mc->kvm_type = qm->kvm_type; + mc->block_default_type = qm->block_default_type; + mc->max_cpus = qm->max_cpus; + mc->no_serial = qm->no_serial; + mc->no_parallel = qm->no_parallel; + mc->use_virtcon = qm->use_virtcon; + mc->use_sclp = qm->use_sclp; + mc->no_floppy = qm->no_floppy; + mc->no_cdrom = qm->no_cdrom; + mc->no_sdcard = qm->no_sdcard; + mc->is_default = qm->is_default; + mc->default_machine_opts = qm->default_machine_opts; + mc->default_boot_order = qm->default_boot_order; + mc->compat_props = qm->compat_props; + mc->hw_version = qm->hw_version; } int qemu_register_machine(QEMUMachine *m) From aaa663916d78aeb51a97842735052e7c8859dc9e Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Wed, 9 Apr 2014 20:34:51 +0300 Subject: [PATCH 04/20] vl.c: Replace QEMUMachine with MachineClass in QEMUMachineInitArgs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QEMUMachine's fields are already in MachineClass. We can safely make the switch because we copy them in machine_class_init() and spapr_machine_class_init(). Signed-off-by: Marcel Apfelbaum Signed-off-by: Andreas Färber --- include/hw/boards.h | 5 +++-- vl.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/hw/boards.h b/include/hw/boards.h index c5f503d0f0..be2e4329b7 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -7,8 +7,10 @@ #include "hw/qdev.h" #include "qom/object.h" +typedef struct MachineClass MachineClass; + typedef struct QEMUMachineInitArgs { - const QEMUMachine *machine; + const MachineClass *machine; ram_addr_t ram_size; const char *boot_order; const char *kernel_filename; @@ -62,7 +64,6 @@ int qemu_register_machine(QEMUMachine *m); OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE) typedef struct MachineState MachineState; -typedef struct MachineClass MachineClass; MachineClass *find_default_machine(void); extern MachineState *current_machine; diff --git a/vl.c b/vl.c index 3229c2ff12..2c2b625b37 100644 --- a/vl.c +++ b/vl.c @@ -4430,7 +4430,7 @@ int main(int argc, char **argv, char **envp) qdev_machine_init(); current_machine->init_args = (QEMUMachineInitArgs) { - .machine = machine, + .machine = machine_class, .ram_size = ram_size, .boot_order = boot_order, .kernel_filename = kernel_filename, From f1e298794daea46e7f52995887c865ac6ada10b9 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Wed, 9 Apr 2014 20:34:52 +0300 Subject: [PATCH 05/20] machine: Replace QEMUMachine by MachineClass in accelerator configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This minimizes QEMUMachine usage, as part of machine QOM-ification. Signed-off-by: Marcel Apfelbaum Signed-off-by: Andreas Färber --- include/hw/boards.h | 3 +-- include/hw/xen/xen.h | 2 +- include/qemu/typedefs.h | 1 + include/sysemu/kvm.h | 2 +- include/sysemu/qtest.h | 2 +- kvm-all.c | 6 +++--- kvm-stub.c | 2 +- qtest.c | 2 +- vl.c | 10 +++++----- xen-all.c | 2 +- xen-stub.c | 2 +- 11 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/hw/boards.h b/include/hw/boards.h index be2e4329b7..8f53334111 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -3,12 +3,11 @@ #ifndef HW_BOARDS_H #define HW_BOARDS_H +#include "qemu/typedefs.h" #include "sysemu/blockdev.h" #include "hw/qdev.h" #include "qom/object.h" -typedef struct MachineClass MachineClass; - typedef struct QEMUMachineInitArgs { const MachineClass *machine; ram_addr_t ram_size; diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h index 9d549fc83d..85fda3dee4 100644 --- a/include/hw/xen/xen.h +++ b/include/hw/xen/xen.h @@ -36,7 +36,7 @@ void xen_cmos_set_s3_resume(void *opaque, int irq, int level); qemu_irq *xen_interrupt_controller_init(void); -int xen_init(QEMUMachine *machine); +int xen_init(MachineClass *mc); int xen_hvm_init(MemoryRegion **ram_memory); void xenstore_store_pv_console_info(int i, struct CharDriverState *chr); diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index bf8daac659..86bab123a4 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -31,6 +31,7 @@ typedef struct MemoryListener MemoryListener; typedef struct MemoryMappingList MemoryMappingList; typedef struct QEMUMachine QEMUMachine; +typedef struct MachineClass MachineClass; typedef struct NICInfo NICInfo; typedef struct HCIInfo HCIInfo; typedef struct AudioState AudioState; diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 192fe893b7..5ad4e0e1e2 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -152,7 +152,7 @@ extern KVMState *kvm_state; /* external API */ -int kvm_init(QEMUMachine *machine); +int kvm_init(MachineClass *mc); int kvm_has_sync_mmu(void); int kvm_has_vcpu_events(void); diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h index 224131f298..95c9ade778 100644 --- a/include/sysemu/qtest.h +++ b/include/sysemu/qtest.h @@ -26,7 +26,7 @@ static inline bool qtest_enabled(void) bool qtest_driver(void); -int qtest_init_accel(QEMUMachine *machine); +int qtest_init_accel(MachineClass *mc); void qtest_init(const char *qtest_chrdev, const char *qtest_log, Error **errp); static inline int qtest_available(void) diff --git a/kvm-all.c b/kvm-all.c index 82a91199e1..5cb7f26f4f 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1341,7 +1341,7 @@ static int kvm_max_vcpus(KVMState *s) return (ret) ? ret : kvm_recommended_vcpus(s); } -int kvm_init(QEMUMachine *machine) +int kvm_init(MachineClass *mc) { static const char upgrade_note[] = "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n" @@ -1433,8 +1433,8 @@ int kvm_init(QEMUMachine *machine) } kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type"); - if (machine->kvm_type) { - type = machine->kvm_type(kvm_type); + if (mc->kvm_type) { + type = mc->kvm_type(kvm_type); } else if (kvm_type) { fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type); goto err; diff --git a/kvm-stub.c b/kvm-stub.c index ccdba62d67..8acda86ced 100644 --- a/kvm-stub.c +++ b/kvm-stub.c @@ -34,7 +34,7 @@ int kvm_init_vcpu(CPUState *cpu) return -ENOSYS; } -int kvm_init(QEMUMachine *machine) +int kvm_init(MachineClass *mc) { return -ENOSYS; } diff --git a/qtest.c b/qtest.c index 0ac9f429f5..2aba20d104 100644 --- a/qtest.c +++ b/qtest.c @@ -500,7 +500,7 @@ static void qtest_event(void *opaque, int event) } } -int qtest_init_accel(QEMUMachine *machine) +int qtest_init_accel(MachineClass *mc) { configure_icount("0"); diff --git a/vl.c b/vl.c index 2c2b625b37..f423b2e116 100644 --- a/vl.c +++ b/vl.c @@ -2725,7 +2725,7 @@ static MachineClass *machine_parse(const char *name) exit(!name || !is_help_option(name)); } -static int tcg_init(QEMUMachine *machine) +static int tcg_init(MachineClass *mc) { tcg_exec_init(tcg_tb_size * 1024 * 1024); return 0; @@ -2735,7 +2735,7 @@ static struct { const char *opt_name; const char *name; int (*available)(void); - int (*init)(QEMUMachine *); + int (*init)(MachineClass *mc); bool *allowed; } accel_list[] = { { "tcg", "tcg", tcg_available, tcg_init, &tcg_allowed }, @@ -2744,7 +2744,7 @@ static struct { { "qtest", "QTest", qtest_available, qtest_init_accel, &qtest_allowed }, }; -static int configure_accelerator(QEMUMachine *machine) +static int configure_accelerator(MachineClass *mc) { const char *p; char buf[10]; @@ -2771,7 +2771,7 @@ static int configure_accelerator(QEMUMachine *machine) break; } *(accel_list[i].allowed) = true; - ret = accel_list[i].init(machine); + ret = accel_list[i].init(mc); if (ret < 0) { init_failed = true; fprintf(stderr, "failed to initialize %s: %s\n", @@ -4222,7 +4222,7 @@ int main(int argc, char **argv, char **envp) exit(0); } - configure_accelerator(machine); + configure_accelerator(machine_class); if (qtest_chrdev) { Error *local_err = NULL; diff --git a/xen-all.c b/xen-all.c index ba3473901e..a63b53152a 100644 --- a/xen-all.c +++ b/xen-all.c @@ -1001,7 +1001,7 @@ static void xen_exit_notifier(Notifier *n, void *data) xs_daemon_close(state->xenstore); } -int xen_init(QEMUMachine *machine) +int xen_init(MachineClass *mc) { xen_xc = xen_xc_interface_open(0, 0, 0); if (xen_xc == XC_HANDLER_INITIAL_VALUE) { diff --git a/xen-stub.c b/xen-stub.c index 59927cb5d6..de26583a23 100644 --- a/xen-stub.c +++ b/xen-stub.c @@ -47,7 +47,7 @@ qemu_irq *xen_interrupt_controller_init(void) return NULL; } -int xen_init(QEMUMachine *machine) +int xen_init(MachineClass *mc) { return -ENOSYS; } From 958db90cd54823c33345000c995453a8c9b7a005 Mon Sep 17 00:00:00 2001 From: Marcel Apfelbaum Date: Wed, 9 Apr 2014 20:34:53 +0300 Subject: [PATCH 06/20] machine: Remove QEMUMachine indirection from MachineClass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No need to go through qemu_machine field. Use MachineClass fields directly. Signed-off-by: Marcel Apfelbaum Signed-off-by: Andreas Färber --- device-hotplug.c | 2 +- hw/ppc/spapr.c | 48 ++++++--------------------- include/hw/boards.h | 2 -- qmp.c | 4 +-- vl.c | 79 +++++++++++++++++++++------------------------ 5 files changed, 49 insertions(+), 86 deletions(-) diff --git a/device-hotplug.c b/device-hotplug.c index ebfa6b1016..eecb08e2b1 100644 --- a/device-hotplug.c +++ b/device-hotplug.c @@ -40,7 +40,7 @@ DriveInfo *add_init_drive(const char *optstr) return NULL; mc = MACHINE_GET_CLASS(current_machine); - dinfo = drive_init(opts, mc->qemu_machine->block_default_type); + dinfo = drive_init(opts, mc->block_default_type); if (!dinfo) { qemu_opts_del(opts); return NULL; diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 5192702504..b4ce950bbd 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1419,19 +1419,6 @@ static int spapr_kvm_type(const char *vm_type) exit(1); } -static QEMUMachine spapr_machine = { - .name = "pseries", - .desc = "pSeries Logical Partition (PAPR compliant)", - .is_default = 1, - .init = ppc_spapr_init, - .reset = ppc_spapr_reset, - .block_default_type = IF_SCSI, - .max_cpus = MAX_CPUS, - .no_parallel = 1, - .default_boot_order = NULL, - .kvm_type = spapr_kvm_type, -}; - /* * Implementation of an interface to adjust firmware patch * for the bootindex property handling. @@ -1493,31 +1480,17 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc); - QEMUMachine *qm = data; - mc->qemu_machine = data; - - mc->name = qm->name; - mc->alias = qm->alias; - mc->desc = qm->desc; - mc->init = qm->init; - mc->reset = qm->reset; - mc->hot_add_cpu = qm->hot_add_cpu; - mc->kvm_type = qm->kvm_type; - mc->block_default_type = qm->block_default_type; - mc->max_cpus = qm->max_cpus; - mc->no_serial = qm->no_serial; - mc->no_parallel = qm->no_parallel; - mc->use_virtcon = qm->use_virtcon; - mc->use_sclp = qm->use_sclp; - mc->no_floppy = qm->no_floppy; - mc->no_cdrom = qm->no_cdrom; - mc->no_sdcard = qm->no_sdcard; - mc->is_default = qm->is_default; - mc->default_machine_opts = qm->default_machine_opts; - mc->default_boot_order = qm->default_boot_order; - mc->compat_props = qm->compat_props; - mc->hw_version = qm->hw_version; + mc->name = "pseries"; + mc->desc = "pSeries Logical Partition (PAPR compliant)"; + mc->is_default = 1; + mc->init = ppc_spapr_init; + mc->reset = ppc_spapr_reset; + mc->block_default_type = IF_SCSI; + mc->max_cpus = MAX_CPUS; + mc->no_parallel = 1; + mc->default_boot_order = NULL; + mc->kvm_type = spapr_kvm_type; fwc->get_dev_path = spapr_get_fw_dev_path; } @@ -1526,7 +1499,6 @@ static const TypeInfo spapr_machine_info = { .name = TYPE_SPAPR_MACHINE, .parent = TYPE_MACHINE, .class_init = spapr_machine_class_init, - .class_data = &spapr_machine, .interfaces = (InterfaceInfo[]) { { TYPE_FW_PATH_PROVIDER }, { } diff --git a/include/hw/boards.h b/include/hw/boards.h index 8f53334111..4345bd04fa 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -76,8 +76,6 @@ struct MachineClass { ObjectClass parent_class; /*< public >*/ - QEMUMachine *qemu_machine; - const char *name; const char *alias; const char *desc; diff --git a/qmp.c b/qmp.c index 74107be41b..c4836dfb6c 100644 --- a/qmp.c +++ b/qmp.c @@ -117,8 +117,8 @@ void qmp_cpu_add(int64_t id, Error **errp) MachineClass *mc; mc = MACHINE_GET_CLASS(current_machine); - if (mc->qemu_machine->hot_add_cpu) { - mc->qemu_machine->hot_add_cpu(id, errp); + if (mc->hot_add_cpu) { + mc->hot_add_cpu(id, errp); } else { error_setg(errp, "Not supported"); } diff --git a/vl.c b/vl.c index f423b2e116..c4505dc8c1 100644 --- a/vl.c +++ b/vl.c @@ -1590,8 +1590,6 @@ static void machine_class_init(ObjectClass *oc, void *data) MachineClass *mc = MACHINE_CLASS(oc); QEMUMachine *qm = data; - mc->qemu_machine = data; - mc->name = qm->name; mc->alias = qm->alias; mc->desc = qm->desc; @@ -1639,12 +1637,12 @@ static MachineClass *find_machine(const char *name) for (el = machines; el; el = el->next) { MachineClass *temp = el->data; - if (!strcmp(temp->qemu_machine->name, name)) { + if (!strcmp(temp->name, name)) { mc = temp; break; } - if (temp->qemu_machine->alias && - !strcmp(temp->qemu_machine->alias, name)) { + if (temp->alias && + !strcmp(temp->alias, name)) { mc = temp; break; } @@ -1662,7 +1660,7 @@ MachineClass *find_default_machine(void) for (el = machines; el; el = el->next) { MachineClass *temp = el->data; - if (temp->qemu_machine->is_default) { + if (temp->is_default) { mc = temp; break; } @@ -1676,27 +1674,25 @@ MachineInfoList *qmp_query_machines(Error **errp) { GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false); MachineInfoList *mach_list = NULL; - QEMUMachine *m; for (el = machines; el; el = el->next) { MachineClass *mc = el->data; MachineInfoList *entry; MachineInfo *info; - m = mc->qemu_machine; info = g_malloc0(sizeof(*info)); - if (m->is_default) { + if (mc->is_default) { info->has_is_default = true; info->is_default = true; } - if (m->alias) { + if (mc->alias) { info->has_alias = true; - info->alias = g_strdup(m->alias); + info->alias = g_strdup(mc->alias); } - info->name = g_strdup(m->name); - info->cpu_max = !m->max_cpus ? 1 : m->max_cpus; + info->name = g_strdup(mc->name); + info->cpu_max = !mc->max_cpus ? 1 : mc->max_cpus; entry = g_malloc0(sizeof(*entry)); entry->value = info; @@ -1902,8 +1898,8 @@ void qemu_system_reset(bool report) mc = current_machine ? MACHINE_GET_CLASS(current_machine) : NULL; - if (mc && mc->qemu_machine->reset) { - mc->qemu_machine->reset(); + if (mc && mc->reset) { + mc->reset(); } else { qemu_devices_reset(); } @@ -2712,12 +2708,11 @@ static MachineClass *machine_parse(const char *name) printf("Supported machines are:\n"); for (el = machines; el; el = el->next) { MachineClass *mc = el->data; - QEMUMachine *m = mc->qemu_machine; - if (m->alias) { - printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name); + if (mc->alias) { + printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name); } - printf("%-20s %s%s\n", m->name, m->desc, - m->is_default ? " (default)" : ""); + printf("%-20s %s%s\n", mc->name, mc->desc, + mc->is_default ? " (default)" : ""); } } @@ -2971,7 +2966,6 @@ int main(int argc, char **argv, char **envp) const char *optarg; const char *loadvm = NULL; MachineClass *machine_class; - QEMUMachine *machine; const char *cpu_model; const char *vga_model = NULL; const char *qtest_chrdev = NULL; @@ -3999,9 +3993,8 @@ int main(int argc, char **argv, char **envp) object_property_add_child(object_get_root(), "machine", OBJECT(current_machine), &error_abort); - machine = machine_class->qemu_machine; - if (machine->hw_version) { - qemu_set_version(machine->hw_version); + if (machine_class->hw_version) { + qemu_set_version(machine_class->hw_version); } if (qemu_opts_foreach(qemu_find_opts("object"), @@ -4061,11 +4054,11 @@ int main(int argc, char **argv, char **envp) smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL)); - machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */ - if (smp_cpus > machine->max_cpus) { + machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */ + if (smp_cpus > machine_class->max_cpus) { fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus " - "supported by machine `%s' (%d)\n", smp_cpus, machine->name, - machine->max_cpus); + "supported by machine `%s' (%d)\n", smp_cpus, + machine_class->name, machine_class->max_cpus); exit(1); } @@ -4073,9 +4066,9 @@ int main(int argc, char **argv, char **envp) * Get the default machine options from the machine if it is not already * specified either by the configuration file or by the command line. */ - if (machine->default_machine_opts) { + if (machine_class->default_machine_opts) { qemu_opts_set_defaults(qemu_find_opts("machine"), - machine->default_machine_opts, 0); + machine_class->default_machine_opts, 0); } qemu_opts_foreach(qemu_find_opts("device"), default_driver_check, NULL, 0); @@ -4084,25 +4077,25 @@ int main(int argc, char **argv, char **envp) if (!vga_model && !default_vga) { vga_interface_type = VGA_DEVICE; } - if (!has_defaults || machine->no_serial) { + if (!has_defaults || machine_class->no_serial) { default_serial = 0; } - if (!has_defaults || machine->no_parallel) { + if (!has_defaults || machine_class->no_parallel) { default_parallel = 0; } - if (!has_defaults || !machine->use_virtcon) { + if (!has_defaults || !machine_class->use_virtcon) { default_virtcon = 0; } - if (!has_defaults || !machine->use_sclp) { + if (!has_defaults || !machine_class->use_sclp) { default_sclp = 0; } - if (!has_defaults || machine->no_floppy) { + if (!has_defaults || machine_class->no_floppy) { default_floppy = 0; } - if (!has_defaults || machine->no_cdrom) { + if (!has_defaults || machine_class->no_cdrom) { default_cdrom = 0; } - if (!has_defaults || machine->no_sdcard) { + if (!has_defaults || machine_class->no_sdcard) { default_sdcard = 0; } if (!has_defaults) { @@ -4240,7 +4233,7 @@ int main(int argc, char **argv, char **envp) kernel_cmdline = qemu_opt_get(machine_opts, "append"); bios_name = qemu_opt_get(machine_opts, "firmware"); - boot_order = machine->default_boot_order; + boot_order = machine_class->default_boot_order; opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL); if (opts) { char *normal_boot_order; @@ -4334,11 +4327,11 @@ int main(int argc, char **argv, char **envp) if (snapshot) qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0); if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func, - &machine->block_default_type, 1) != 0) { + &machine_class->block_default_type, 1) != 0) { exit(1); } - default_drive(default_cdrom, snapshot, machine->block_default_type, 2, + default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2, CDROM_OPTS); default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS); default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS); @@ -4422,8 +4415,8 @@ int main(int argc, char **argv, char **envp) exit (i == 1 ? 1 : 0); } - if (machine->compat_props) { - qdev_prop_register_global_list(machine->compat_props); + if (machine_class->compat_props) { + qdev_prop_register_global_list(machine_class->compat_props); } qemu_add_globals(); @@ -4438,7 +4431,7 @@ int main(int argc, char **argv, char **envp) .initrd_filename = initrd_filename, .cpu_model = cpu_model }; - machine->init(¤t_machine->init_args); + machine_class->init(¤t_machine->init_args); audio_init(); From 2f719f195cfdacb0cbc42ec03e5172eeefff1726 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 25 Apr 2014 12:44:20 +0200 Subject: [PATCH 07/20] hw: Consistently name Error * objects err, and not errp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Markus Armbruster Signed-off-by: Andreas Färber --- hw/core/qdev-properties-system.c | 10 +++++----- hw/dma/xilinx_axidma.c | 16 ++++++++-------- hw/net/xilinx_axienet.c | 16 ++++++++-------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index de835612f0..404cf1843d 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -338,13 +338,13 @@ PropertyInfo qdev_prop_vlan = { int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value) { - Error *errp = NULL; + Error *err = NULL; const char *bdrv_name = value ? bdrv_get_device_name(value) : ""; object_property_set_str(OBJECT(dev), bdrv_name, - name, &errp); - if (errp) { - qerror_report_err(errp); - error_free(errp); + name, &err); + if (err) { + qerror_report_err(err); + error_free(err); return -1; } return 0; diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c index 14b887bfa8..cc90eb5110 100644 --- a/hw/dma/xilinx_axidma.c +++ b/hw/dma/xilinx_axidma.c @@ -534,24 +534,24 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp) XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(&s->rx_data_dev); XilinxAXIDMAStreamSlave *cs = XILINX_AXI_DMA_CONTROL_STREAM( &s->rx_control_dev); - Error *local_errp = NULL; + Error *local_err = NULL; object_property_add_link(OBJECT(ds), "dma", TYPE_XILINX_AXI_DMA, (Object **)&ds->dma, object_property_allow_set_link, OBJ_PROP_LINK_UNREF_ON_RELEASE, - &local_errp); + &local_err); object_property_add_link(OBJECT(cs), "dma", TYPE_XILINX_AXI_DMA, (Object **)&cs->dma, object_property_allow_set_link, OBJ_PROP_LINK_UNREF_ON_RELEASE, - &local_errp); - if (local_errp) { + &local_err); + if (local_err) { goto xilinx_axidma_realize_fail; } - object_property_set_link(OBJECT(ds), OBJECT(s), "dma", &local_errp); - object_property_set_link(OBJECT(cs), OBJECT(s), "dma", &local_errp); - if (local_errp) { + object_property_set_link(OBJECT(ds), OBJECT(s), "dma", &local_err); + object_property_set_link(OBJECT(cs), OBJECT(s), "dma", &local_err); + if (local_err) { goto xilinx_axidma_realize_fail; } @@ -567,7 +567,7 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp) xilinx_axidma_realize_fail: if (!*errp) { - *errp = local_errp; + *errp = local_err; } } diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c index dbeb3c9a25..cd952d2514 100644 --- a/hw/net/xilinx_axienet.c +++ b/hw/net/xilinx_axienet.c @@ -945,24 +945,24 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp) XilinxAXIEnetStreamSlave *ds = XILINX_AXI_ENET_DATA_STREAM(&s->rx_data_dev); XilinxAXIEnetStreamSlave *cs = XILINX_AXI_ENET_CONTROL_STREAM( &s->rx_control_dev); - Error *local_errp = NULL; + Error *local_err = NULL; object_property_add_link(OBJECT(ds), "enet", "xlnx.axi-ethernet", (Object **) &ds->enet, object_property_allow_set_link, OBJ_PROP_LINK_UNREF_ON_RELEASE, - &local_errp); + &local_err); object_property_add_link(OBJECT(cs), "enet", "xlnx.axi-ethernet", (Object **) &cs->enet, object_property_allow_set_link, OBJ_PROP_LINK_UNREF_ON_RELEASE, - &local_errp); - if (local_errp) { + &local_err); + if (local_err) { goto xilinx_enet_realize_fail; } - object_property_set_link(OBJECT(ds), OBJECT(s), "enet", &local_errp); - object_property_set_link(OBJECT(cs), OBJECT(s), "enet", &local_errp); - if (local_errp) { + object_property_set_link(OBJECT(ds), OBJECT(s), "enet", &local_err); + object_property_set_link(OBJECT(cs), OBJECT(s), "enet", &local_err); + if (local_err) { goto xilinx_enet_realize_fail; } @@ -981,7 +981,7 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp) xilinx_enet_realize_fail: if (!*errp) { - *errp = local_errp; + *errp = local_err; } } From a7737e4496aa3c1c8c3a4b4b9d5e44875fe21e12 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 25 Apr 2014 12:44:21 +0200 Subject: [PATCH 08/20] hw: Consistently name Error ** objects errp, and not err MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Markus Armbruster Signed-off-by: Andreas Färber --- hw/core/qdev.c | 20 ++++++++++---------- hw/intc/i8259.c | 4 ++-- hw/timer/i8254.c | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 60f9df1ed9..2fd5100522 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -174,14 +174,14 @@ int qdev_init(DeviceState *dev) return 0; } -static void device_realize(DeviceState *dev, Error **err) +static void device_realize(DeviceState *dev, Error **errp) { DeviceClass *dc = DEVICE_GET_CLASS(dev); if (dc->init) { int rc = dc->init(dev); if (rc < 0) { - error_setg(err, "Device initialization failed."); + error_setg(errp, "Device initialization failed."); return; } } @@ -504,14 +504,14 @@ static void bus_unparent(Object *obj) } } -static bool bus_get_realized(Object *obj, Error **err) +static bool bus_get_realized(Object *obj, Error **errp) { BusState *bus = BUS(obj); return bus->realized; } -static void bus_set_realized(Object *obj, bool value, Error **err) +static void bus_set_realized(Object *obj, bool value, Error **errp) { BusState *bus = BUS(obj); BusClass *bc = BUS_GET_CLASS(bus); @@ -540,7 +540,7 @@ static void bus_set_realized(Object *obj, bool value, Error **err) return; error: - error_propagate(err, local_err); + error_propagate(errp, local_err); } void qbus_create_inplace(void *bus, size_t size, const char *typename, @@ -724,13 +724,13 @@ void qdev_property_add_static(DeviceState *dev, Property *prop, } } -static bool device_get_realized(Object *obj, Error **err) +static bool device_get_realized(Object *obj, Error **errp) { DeviceState *dev = DEVICE(obj); return dev->realized; } -static void device_set_realized(Object *obj, bool value, Error **err) +static void device_set_realized(Object *obj, bool value, Error **errp) { DeviceState *dev = DEVICE(obj); DeviceClass *dc = DEVICE_GET_CLASS(dev); @@ -738,7 +738,7 @@ static void device_set_realized(Object *obj, bool value, Error **err) Error *local_err = NULL; if (dev->hotplugged && !dc->hotpluggable) { - error_set(err, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj)); + error_set(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj)); return; } @@ -797,14 +797,14 @@ static void device_set_realized(Object *obj, bool value, Error **err) } if (local_err != NULL) { - error_propagate(err, local_err); + error_propagate(errp, local_err); return; } dev->realized = value; } -static bool device_get_hotpluggable(Object *obj, Error **err) +static bool device_get_hotpluggable(Object *obj, Error **errp) { DeviceClass *dc = DEVICE_GET_CLASS(obj); DeviceState *dev = DEVICE(obj); diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c index c6f248b145..ec01393e4f 100644 --- a/hw/intc/i8259.c +++ b/hw/intc/i8259.c @@ -412,7 +412,7 @@ static const MemoryRegionOps pic_elcr_ioport_ops = { }, }; -static void pic_realize(DeviceState *dev, Error **err) +static void pic_realize(DeviceState *dev, Error **errp) { PICCommonState *s = PIC_COMMON(dev); PICClass *pc = PIC_GET_CLASS(dev); @@ -425,7 +425,7 @@ static void pic_realize(DeviceState *dev, Error **err) qdev_init_gpio_out(dev, s->int_out, ARRAY_SIZE(s->int_out)); qdev_init_gpio_in(dev, pic_set_irq, 8); - pc->parent_realize(dev, err); + pc->parent_realize(dev, errp); } void pic_info(Monitor *mon, const QDict *qdict) diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c index 28152d88ea..3450c98637 100644 --- a/hw/timer/i8254.c +++ b/hw/timer/i8254.c @@ -322,7 +322,7 @@ static void pit_post_load(PITCommonState *s) } } -static void pit_realizefn(DeviceState *dev, Error **err) +static void pit_realizefn(DeviceState *dev, Error **errp) { PITCommonState *pit = PIT_COMMON(dev); PITClass *pc = PIT_GET_CLASS(dev); @@ -338,7 +338,7 @@ static void pit_realizefn(DeviceState *dev, Error **err) qdev_init_gpio_in(dev, pit_irq_control, 1); - pc->parent_realize(dev, err); + pc->parent_realize(dev, errp); } static Property pit_properties[] = { From 65cd9064e13f2388d3bd9c815826f5db6b0b13ec Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 25 Apr 2014 12:44:22 +0200 Subject: [PATCH 09/20] qom: Clean up fragile use of error_is_set() in set() methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using error_is_set(ERRP) to find out whether a function failed is either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP may be null, because errors go undetected when it is. It's fragile when proving ERRP non-null involves a non-local argument. Else, it's unnecessarily opaque (see commit 84d18f0). I guess the error_is_set(errp) in the ObjectProperty set() methods are merely fragile right now, because I can't find a call chain that passes a null errp argument. Make the code more robust and more obviously correct: receive the error in a local variable, then propagate it through the parameter. Signed-off-by: Markus Armbruster Signed-off-by: Andreas Färber --- backends/rng.c | 11 +++++++---- backends/tpm.c | 11 +++++++---- hw/core/qdev-properties.c | 11 +++++++---- hw/misc/tmp105.c | 6 ++++-- hw/virtio/virtio-balloon.c | 6 ++++-- target-i386/cpu.c | 24 ++++++++++++++++-------- 6 files changed, 45 insertions(+), 24 deletions(-) diff --git a/backends/rng.c b/backends/rng.c index 8b8d5a4973..0f2fc11dd8 100644 --- a/backends/rng.c +++ b/backends/rng.c @@ -50,6 +50,7 @@ static void rng_backend_prop_set_opened(Object *obj, bool value, Error **errp) { RngBackend *s = RNG_BACKEND(obj); RngBackendClass *k = RNG_BACKEND_GET_CLASS(s); + Error *local_err = NULL; if (value == s->opened) { return; @@ -61,12 +62,14 @@ static void rng_backend_prop_set_opened(Object *obj, bool value, Error **errp) } if (k->opened) { - k->opened(s, errp); + k->opened(s, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } } - if (!error_is_set(errp)) { - s->opened = value; - } + s->opened = true; } static void rng_backend_init(Object *obj) diff --git a/backends/tpm.c b/backends/tpm.c index b73580134c..01860c4acb 100644 --- a/backends/tpm.c +++ b/backends/tpm.c @@ -112,6 +112,7 @@ static void tpm_backend_prop_set_opened(Object *obj, bool value, Error **errp) { TPMBackend *s = TPM_BACKEND(obj); TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); + Error *local_err = NULL; if (value == s->opened) { return; @@ -123,12 +124,14 @@ static void tpm_backend_prop_set_opened(Object *obj, bool value, Error **errp) } if (k->opened) { - k->opened(s, errp); + k->opened(s, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } } - if (!error_is_set(errp)) { - s->opened = value; - } + s->opened = true; } static void tpm_backend_instance_init(Object *obj) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 585a8e902e..d8cb5408c3 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -751,6 +751,7 @@ static void set_prop_arraylen(Object *obj, Visitor *v, void *opaque, Property *prop = opaque; uint32_t *alenptr = qdev_get_prop_ptr(dev, prop); void **arrayptr = (void *)dev + prop->arrayoffset; + Error *local_err = NULL; void *eltptr; const char *arrayname; int i; @@ -764,8 +765,9 @@ static void set_prop_arraylen(Object *obj, Visitor *v, void *opaque, name); return; } - visit_type_uint32(v, alenptr, name, errp); - if (error_is_set(errp)) { + visit_type_uint32(v, alenptr, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } if (!*alenptr) { @@ -802,8 +804,9 @@ static void set_prop_arraylen(Object *obj, Visitor *v, void *opaque, arrayprop->prop.info->get, arrayprop->prop.info->set, array_element_release, - arrayprop, errp); - if (error_is_set(errp)) { + arrayprop, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } } diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c index 63aa3d6277..636ee97b16 100644 --- a/hw/misc/tmp105.c +++ b/hw/misc/tmp105.c @@ -68,10 +68,12 @@ static void tmp105_set_temperature(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { TMP105State *s = TMP105(obj); + Error *local_err = NULL; int64_t temp; - visit_type_int(v, &temp, name, errp); - if (error_is_set(errp)) { + visit_type_int(v, &temp, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } if (temp >= 128000 || temp < -128000) { diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index a470a0b3a6..971a921777 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -142,10 +142,12 @@ static void balloon_stats_set_poll_interval(Object *obj, struct Visitor *v, Error **errp) { VirtIOBalloon *s = opaque; + Error *local_err = NULL; int64_t value; - visit_type_int(v, &value, name, errp); - if (error_is_set(errp)) { + visit_type_int(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8fd1497dc4..8f193a9330 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1300,10 +1300,12 @@ static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque, CPUX86State *env = &cpu->env; const int64_t min = 0; const int64_t max = 0xff + 0xf; + Error *local_err = NULL; int64_t value; - visit_type_int(v, &value, name, errp); - if (error_is_set(errp)) { + visit_type_int(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } if (value < min || value > max) { @@ -1339,10 +1341,12 @@ static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque, CPUX86State *env = &cpu->env; const int64_t min = 0; const int64_t max = 0xff; + Error *local_err = NULL; int64_t value; - visit_type_int(v, &value, name, errp); - if (error_is_set(errp)) { + visit_type_int(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } if (value < min || value > max) { @@ -1375,10 +1379,12 @@ static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v, CPUX86State *env = &cpu->env; const int64_t min = 0; const int64_t max = 0xf; + Error *local_err = NULL; int64_t value; - visit_type_int(v, &value, name, errp); - if (error_is_set(errp)) { + visit_type_int(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } if (value < min || value > max) { @@ -1511,10 +1517,12 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque, X86CPU *cpu = X86_CPU(obj); const int64_t min = 0; const int64_t max = INT64_MAX; + Error *local_err = NULL; int64_t value; - visit_type_int(v, &value, name, errp); - if (error_is_set(errp)) { + visit_type_int(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } if (value < min || value > max) { From 0175ba109eb0c098a00903887328393c2e17d4ae Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 25 Apr 2014 12:44:23 +0200 Subject: [PATCH 10/20] arm: Clean up fragile use of error_is_set() in realize() methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using error_is_set(ERRP) to find out whether a function failed is either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP may be null, because errors go undetected when it is. It's fragile when proving ERRP non-null involves a non-local argument. Else, it's unnecessarily opaque (see commit 84d18f0). I guess the error_is_set(errp) in the DeviceClass realize() methods are merely fragile right now, because I can't find a call chain that passes a null errp argument. Make the code more robust and more obviously correct: receive the error in a local variable, then propagate it through the parameter. Signed-off-by: Markus Armbruster Acked-by: Peter Maydell Signed-off-by: Andreas Färber --- hw/intc/arm_gic.c | 6 ++++-- hw/intc/arm_gic_kvm.c | 6 ++++-- hw/intc/armv7m_nvic.c | 6 ++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index 955b8d4945..1532ef9482 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -797,9 +797,11 @@ static void arm_gic_realize(DeviceState *dev, Error **errp) GICState *s = ARM_GIC(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); ARMGICClass *agc = ARM_GIC_GET_CLASS(s); + Error *local_err = NULL; - agc->parent_realize(dev, errp); - if (error_is_set(errp)) { + agc->parent_realize(dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c index 719d2277ec..5038885afd 100644 --- a/hw/intc/arm_gic_kvm.c +++ b/hw/intc/arm_gic_kvm.c @@ -517,10 +517,12 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp) GICState *s = KVM_ARM_GIC(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); KVMARMGICClass *kgc = KVM_ARM_GIC_GET_CLASS(s); + Error *local_err = NULL; int ret; - kgc->parent_realize(dev, errp); - if (error_is_set(errp)) { + kgc->parent_realize(dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index f5b0c3be6f..9aa8ab208f 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -474,14 +474,16 @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) { nvic_state *s = NVIC(dev); NVICClass *nc = NVIC_GET_CLASS(s); + Error *local_err = NULL; /* The NVIC always has only one CPU */ s->gic.num_cpu = 1; /* Tell the common code we're an NVIC */ s->gic.revision = 0xffffffff; s->num_irq = s->gic.num_irq; - nc->parent_realize(dev, errp); - if (error_is_set(errp)) { + nc->parent_realize(dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); return; } gic_init_irqs_and_distributor(&s->gic, s->num_irq); From 4688c94c1f59b8df45079391e26569d0615ab7af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Thu, 24 Apr 2014 15:00:14 +0200 Subject: [PATCH 11/20] MAINTAINERS: Document QOM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invented by Anthony. Maintained through my qom-next tree lately. Cc: Anthony Liguori Acked-by: Paolo Bonzini Acked-by: Peter Maydell Signed-off-by: Andreas Färber --- MAINTAINERS | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index b287ef8939..c43643cd9b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -780,6 +780,17 @@ S: Supported F: qapi-schema.json T: git git://repo.or.cz/qemu/qmp-unstable.git queue/qmp +QOM +M: Anthony Liguori +M: Andreas Färber +S: Supported +T: git git://github.com/afaerber/qemu-cpu.git qom-next +F: include/qom/ +X: include/qom/cpu.h +F: qom/ +X: qom/cpu.c +F: tests/qom-test.c + QMP M: Luiz Capitulino S: Maintained From a7ded163dbe548075d65219c6189f059da2d99bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Thu, 17 Apr 2014 18:19:14 +0200 Subject: [PATCH 12/20] qtest: Assure that init_socket()'s listen() does not fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In practice this seems very unlikely, so cleanup is neglected, as done for bind(). Reviewed-by: Stefan Hajnoczi Signed-off-by: Andreas Färber --- tests/libqtest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index 8155695848..232f781639 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -72,7 +72,8 @@ static int init_socket(const char *socket_path) ret = bind(sock, (struct sockaddr *)&addr, sizeof(addr)); } while (ret == -1 && errno == EINTR); g_assert_no_errno(ret); - listen(sock, 1); + ret = listen(sock, 1); + g_assert_no_errno(ret); return sock; } From a7d915f388355f5353f2b692a1abd7868b8307a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Thu, 17 Apr 2014 18:38:25 +0200 Subject: [PATCH 13/20] qtest: Add error reporting to socket_accept() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We're not using the GLib infrastructure here, to allow cleaning up the sockets. Still, knowing why a certain test run failed can be valuable. Reviewed-by: Stefan Hajnoczi Signed-off-by: Andreas Färber --- tests/libqtest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/libqtest.c b/tests/libqtest.c index 232f781639..4b90d9112a 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -93,6 +93,9 @@ static int socket_accept(int sock) do { ret = accept(sock, (struct sockaddr *)&addr, &addrlen); } while (ret == -1 && errno == EINTR); + if (ret == -1) { + fprintf(stderr, "%s failed: %s\n", __func__, strerror(errno)); + } close(sock); return ret; From 535b45631ad7176e10dab89d55443b5a096500b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Thu, 17 Apr 2014 19:21:12 +0200 Subject: [PATCH 14/20] qtest: Be paranoid about accept() addrlen argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POSIX specifies that address_len shall on output specify the length of the stored address; it does not however specify whether it may get updated on failure as well to, e.g., zero. In case EINTR occurs, re-initialize the variable to the desired value. Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- tests/libqtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index 4b90d9112a..71468ac9c7 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -89,8 +89,8 @@ static int socket_accept(int sock) setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, (void *)&timeout, sizeof(timeout)); - addrlen = sizeof(addr); do { + addrlen = sizeof(addr); ret = accept(sock, (struct sockaddr *)&addr, &addrlen); } while (ret == -1 && errno == EINTR); if (ret == -1) { From d7b50c0cc052f705eceb17018ff36a453a6c60c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Sun, 30 Mar 2014 19:00:05 +0200 Subject: [PATCH 15/20] tests: Add ac97 qtest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Andreas Färber --- MAINTAINERS | 1 + tests/Makefile | 3 +++ tests/ac97-test.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 tests/ac97-test.c diff --git a/MAINTAINERS b/MAINTAINERS index c43643cd9b..e81967c1e9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -666,6 +666,7 @@ M: Gerd Hoffmann S: Maintained F: audio/ F: hw/audio/ +F: tests/ac97-test.c Block M: Kevin Wolf diff --git a/tests/Makefile b/tests/Makefile index c6b661472b..aeaa665c63 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -107,6 +107,8 @@ check-qtest-pci-y += tests/ne2000-test$(EXESUF) gcov-files-pci-y += hw/net/ne2000.c check-qtest-pci-y += tests/nvme-test$(EXESUF) gcov-files-pci-y += hw/block/nvme.c +check-qtest-pci-y += tests/ac97-test$(EXESUF) +gcov-files-pci-y += hw/audio/ac97.c check-qtest-pci-y += $(check-qtest-virtio-y) gcov-files-pci-y += $(gcov-files-virtio-y) hw/virtio/virtio-pci.c check-qtest-pci-y += tests/tpci200-test$(EXESUF) @@ -292,6 +294,7 @@ 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 tests/i82801b11-test$(EXESUF): tests/i82801b11-test.o +tests/ac97-test$(EXESUF): tests/ac97-test.o tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o # QTest rules diff --git a/tests/ac97-test.c b/tests/ac97-test.c new file mode 100644 index 0000000000..af30ea1dd6 --- /dev/null +++ b/tests/ac97-test.c @@ -0,0 +1,33 @@ +/* + * QTest testcase for AC97 + * + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include +#include +#include "libqtest.h" +#include "qemu/osdep.h" + +/* Tests only initialization so far. TODO: Replace with functional tests */ +static void nop(void) +{ +} + +int main(int argc, char **argv) +{ + int ret; + + g_test_init(&argc, &argv, NULL); + qtest_add_func("/ac97/nop", nop); + + qtest_start("-device AC97"); + ret = g_test_run(); + + qtest_end(); + + return ret; +} From 8fa74c947df001d411452ea8e7d528e23235134d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Sun, 30 Mar 2014 19:05:20 +0200 Subject: [PATCH 16/20] tests: Add es1370 qtest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Andreas Färber --- MAINTAINERS | 1 + tests/Makefile | 3 +++ tests/es1370-test.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 tests/es1370-test.c diff --git a/MAINTAINERS b/MAINTAINERS index e81967c1e9..034238c51e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -667,6 +667,7 @@ S: Maintained F: audio/ F: hw/audio/ F: tests/ac97-test.c +F: tests/es1370-test.c Block M: Kevin Wolf diff --git a/tests/Makefile b/tests/Makefile index aeaa665c63..23d5d4a774 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -109,6 +109,8 @@ check-qtest-pci-y += tests/nvme-test$(EXESUF) gcov-files-pci-y += hw/block/nvme.c check-qtest-pci-y += tests/ac97-test$(EXESUF) gcov-files-pci-y += hw/audio/ac97.c +check-qtest-pci-y += tests/es1370-test$(EXESUF) +gcov-files-pci-y += hw/audio/es1370.c check-qtest-pci-y += $(check-qtest-virtio-y) gcov-files-pci-y += $(gcov-files-virtio-y) hw/virtio/virtio-pci.c check-qtest-pci-y += tests/tpci200-test$(EXESUF) @@ -295,6 +297,7 @@ tests/nvme-test$(EXESUF): tests/nvme-test.o tests/pvpanic-test$(EXESUF): tests/pvpanic-test.o tests/i82801b11-test$(EXESUF): tests/i82801b11-test.o tests/ac97-test$(EXESUF): tests/ac97-test.o +tests/es1370-test$(EXESUF): tests/es1370-test.o tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o # QTest rules diff --git a/tests/es1370-test.c b/tests/es1370-test.c new file mode 100644 index 0000000000..cc23fb5c67 --- /dev/null +++ b/tests/es1370-test.c @@ -0,0 +1,33 @@ +/* + * QTest testcase for ES1370 + * + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include +#include +#include "libqtest.h" +#include "qemu/osdep.h" + +/* Tests only initialization so far. TODO: Replace with functional tests */ +static void nop(void) +{ +} + +int main(int argc, char **argv) +{ + int ret; + + g_test_init(&argc, &argv, NULL); + qtest_add_func("/es1370/nop", nop); + + qtest_start("-device ES1370"); + ret = g_test_run(); + + qtest_end(); + + return ret; +} From fbaf445a899ec763cdd99f32c733ef2d080ff8ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Sun, 30 Mar 2014 19:22:48 +0200 Subject: [PATCH 17/20] tests: Add intel-hda qtests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test both the ich6 and the ich9 version (cf. q35 config) and all the codecs. Cc: Gerd Hoffmann Signed-off-by: Andreas Färber --- MAINTAINERS | 1 + tests/Makefile | 3 +++ tests/intel-hda-test.c | 45 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 tests/intel-hda-test.c diff --git a/MAINTAINERS b/MAINTAINERS index 034238c51e..aa83a4cc09 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -668,6 +668,7 @@ F: audio/ F: hw/audio/ F: tests/ac97-test.c F: tests/es1370-test.c +F: tests/intel-hda-test.c Block M: Kevin Wolf diff --git a/tests/Makefile b/tests/Makefile index 23d5d4a774..c926c606bf 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -121,6 +121,8 @@ check-qtest-pci-y += tests/display-vga-test$(EXESUF) gcov-files-pci-y += hw/display/vga.c gcov-files-pci-y += hw/display/cirrus_vga.c gcov-files-pci-y += hw/display/vga-pci.c +check-qtest-pci-y += tests/intel-hda-test$(EXESUF) +gcov-files-pci-y += hw/audio/intel-hda.c hw/audio/hda-codec.c check-qtest-i386-y = tests/endianness-test$(EXESUF) check-qtest-i386-y += tests/fdc-test$(EXESUF) @@ -298,6 +300,7 @@ tests/pvpanic-test$(EXESUF): tests/pvpanic-test.o tests/i82801b11-test$(EXESUF): tests/i82801b11-test.o tests/ac97-test$(EXESUF): tests/ac97-test.o tests/es1370-test$(EXESUF): tests/es1370-test.o +tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o # QTest rules diff --git a/tests/intel-hda-test.c b/tests/intel-hda-test.c new file mode 100644 index 0000000000..d89b407dcc --- /dev/null +++ b/tests/intel-hda-test.c @@ -0,0 +1,45 @@ +/* + * QTest testcase for Intel HDA + * + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include +#include +#include "libqtest.h" +#include "qemu/osdep.h" + +#define HDA_ID "hda0" +#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0" \ + " -device hda-micro,bus=" HDA_ID ".0" \ + " -device hda-duplex,bus=" HDA_ID ".0" + +/* Tests only initialization so far. TODO: Replace with functional tests */ +static void ich6_test(void) +{ + qtest_start("-device intel-hda,id=" HDA_ID CODEC_DEVICES); + qtest_end(); +} + +static void ich9_test(void) +{ + qtest_start("-machine q35 -device ich9-intel-hda,bus=pcie.0,addr=1b.0,id=" + HDA_ID CODEC_DEVICES); + qtest_end(); +} + +int main(int argc, char **argv) +{ + int ret; + + g_test_init(&argc, &argv, NULL); + qtest_add_func("/intel-hda/ich6", ich6_test); + qtest_add_func("/intel-hda/ich9", ich9_test); + + ret = g_test_run(); + + return ret; +} From efbf5df02007e6754593bd542296c4005a04d192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Sun, 30 Mar 2014 20:02:00 +0200 Subject: [PATCH 18/20] tests: Add ioh3420 qtest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Andreas Färber --- tests/Makefile | 3 +++ tests/ioh3420-test.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 tests/ioh3420-test.c diff --git a/tests/Makefile b/tests/Makefile index c926c606bf..5935e8ad25 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -147,6 +147,8 @@ check-qtest-i386-y += tests/pvpanic-test$(EXESUF) gcov-files-i386-y += i386-softmmu/hw/misc/pvpanic.c check-qtest-i386-y += tests/i82801b11-test$(EXESUF) gcov-files-i386-y += hw/pci-bridge/i82801b11.c +check-qtest-i386-y += tests/ioh3420-test$(EXESUF) +gcov-files-i386-y += hw/pci-bridge/ioh3420.c check-qtest-x86_64-y = $(check-qtest-i386-y) gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y)) @@ -301,6 +303,7 @@ tests/i82801b11-test$(EXESUF): tests/i82801b11-test.o tests/ac97-test$(EXESUF): tests/ac97-test.o tests/es1370-test$(EXESUF): tests/es1370-test.o tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o +tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o # QTest rules diff --git a/tests/ioh3420-test.c b/tests/ioh3420-test.c new file mode 100644 index 0000000000..c991a5f873 --- /dev/null +++ b/tests/ioh3420-test.c @@ -0,0 +1,34 @@ +/* + * QTest testcase for Intel X58 north bridge IOH + * + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include +#include +#include "libqtest.h" +#include "qemu/osdep.h" + +/* Tests only initialization so far. TODO: Replace with functional tests */ +static void nop(void) +{ +} + +int main(int argc, char **argv) +{ + int ret; + + g_test_init(&argc, &argv, NULL); + qtest_add_func("/ioh3420/nop", nop); + + qtest_start("-machine q35 -device ioh3420,bus=pcie.0,addr=1c.0,port=1," + "chassis=1,multifunction=on"); + ret = g_test_run(); + + qtest_end(); + + return ret; +} From cc900d34e7f298dd4b41c5626307de15416bb045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Sun, 30 Mar 2014 20:25:38 +0200 Subject: [PATCH 19/20] tests: Add EHCI qtest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Andreas Färber --- MAINTAINERS | 1 + tests/Makefile | 4 ++++ tests/usb-hcd-ehci-test.c | 40 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 tests/usb-hcd-ehci-test.c diff --git a/MAINTAINERS b/MAINTAINERS index aa83a4cc09..bf77713776 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -601,6 +601,7 @@ USB M: Gerd Hoffmann S: Maintained F: hw/usb/* +F: tests/usb-hcd-ehci-test.c VFIO M: Alex Williamson diff --git a/tests/Makefile b/tests/Makefile index 5935e8ad25..14ecf05b5d 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -149,6 +149,9 @@ check-qtest-i386-y += tests/i82801b11-test$(EXESUF) gcov-files-i386-y += hw/pci-bridge/i82801b11.c check-qtest-i386-y += tests/ioh3420-test$(EXESUF) gcov-files-i386-y += hw/pci-bridge/ioh3420.c +check-qtest-i386-y += tests/usb-hcd-ehci-test$(EXESUF) +gcov-files-i386-y += hw/usb/hcd-ehci.c +gcov-files-i386-y += hw/usb/hcd-uhci.c check-qtest-x86_64-y = $(check-qtest-i386-y) gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y)) @@ -304,6 +307,7 @@ tests/ac97-test$(EXESUF): tests/ac97-test.o tests/es1370-test$(EXESUF): tests/es1370-test.o tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o +tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o # QTest rules diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c new file mode 100644 index 0000000000..bc56ba7707 --- /dev/null +++ b/tests/usb-hcd-ehci-test.c @@ -0,0 +1,40 @@ +/* + * QTest testcase for USB EHCI + * + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include +#include +#include "libqtest.h" +#include "qemu/osdep.h" + +/* Tests only initialization so far. TODO: Replace with functional tests */ +static void pci_nop(void) +{ +} + +int main(int argc, char **argv) +{ + int ret; + + g_test_init(&argc, &argv, NULL); + qtest_add_func("/ehci/pci/nop", pci_nop); + + qtest_start("-machine q35 -device ich9-usb-ehci1,bus=pcie.0,addr=1d.7," + "multifunction=on,id=ich9-ehci-1 " + "-device ich9-usb-uhci1,bus=pcie.0,addr=1d.0," + "multifunction=on,masterbus=ich9-ehci-1.0,firstport=0 " + "-device ich9-usb-uhci2,bus=pcie.0,addr=1d.1," + "multifunction=on,masterbus=ich9-ehci-1.0,firstport=2 " + "-device ich9-usb-uhci3,bus=pcie.0,addr=1d.2," + "multifunction=on,masterbus=ich9-ehci-1.0,firstport=4"); + ret = g_test_run(); + + qtest_end(); + + return ret; +} From 848696bf353750899832c51005f1bd3540da5c29 Mon Sep 17 00:00:00 2001 From: Kirill Batuzov Date: Tue, 29 Apr 2014 17:38:39 +0400 Subject: [PATCH 20/20] PortioList: Store PortioList in device state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PortioList is an abstraction used for construction of MemoryRegionPortioList from MemoryRegionPortio. It can be used later to unmap created memory regions. It also requires proper cleanup because some of the memory inside is allocated dynamically. By moving PortioList ot device state we make it possible to cleanup later and avoid leaking memory. This change spans several target platforms. The following testcases cover all changed lines: qemu-system-ppc -M prep qemu-system-i386 -vga qxl qemu-system-i386 -M isapc -soundhw adlib -device ib700,id=watchdog0,bus=isa.0 Signed-off-by: Kirill Batuzov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/audio/adlib.c | 6 +++--- hw/display/qxl.c | 7 +++---- hw/display/qxl.h | 1 + hw/display/vga.c | 12 +++++------- hw/display/vga_int.h | 2 ++ hw/dma/i82374.c | 7 ++++--- hw/isa/isa-bus.c | 11 ++++++++--- hw/ppc/prep.c | 7 ++++--- hw/watchdog/wdt_ib700.c | 7 ++++--- 9 files changed, 34 insertions(+), 26 deletions(-) diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c index 28eed81280..5dd739e541 100644 --- a/hw/audio/adlib.c +++ b/hw/audio/adlib.c @@ -86,6 +86,7 @@ typedef struct { #ifndef HAS_YMF262 FM_OPL *opl; #endif + PortioList port_list; } AdlibState; static AdlibState *glob_adlib; @@ -293,7 +294,6 @@ static MemoryRegionPortio adlib_portio_list[] = { static void adlib_realizefn (DeviceState *dev, Error **errp) { AdlibState *s = ADLIB(dev); - PortioList *port_list = g_new(PortioList, 1); struct audsettings as; if (glob_adlib) { @@ -349,8 +349,8 @@ static void adlib_realizefn (DeviceState *dev, Error **errp) adlib_portio_list[0].offset = s->port; adlib_portio_list[1].offset = s->port + 8; - portio_list_init (port_list, OBJECT(s), adlib_portio_list, s, "adlib"); - portio_list_add (port_list, isa_address_space_io(&s->parent_obj), 0); + portio_list_init (&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib"); + portio_list_add (&s->port_list, isa_address_space_io(&s->parent_obj), 0); } static Property adlib_properties[] = { diff --git a/hw/display/qxl.c b/hw/display/qxl.c index e9c54d7399..7fb83e4eaf 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2055,7 +2055,6 @@ static int qxl_init_primary(PCIDevice *dev) { PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev); VGACommonState *vga = &qxl->vga; - PortioList *qxl_vga_port_list = g_new(PortioList, 1); int rc; qxl->id = 0; @@ -2064,10 +2063,10 @@ static int qxl_init_primary(PCIDevice *dev) vga_common_init(vga, OBJECT(dev), true); vga_init(vga, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev), false); - portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, + portio_list_init(&qxl->vga_port_list, OBJECT(dev), qxl_vga_portio_list, vga, "vga"); - portio_list_set_flush_coalesced(qxl_vga_port_list); - portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); + portio_list_set_flush_coalesced(&qxl->vga_port_list); + portio_list_add(&qxl->vga_port_list, pci_address_space_io(dev), 0x3b0); vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl); qemu_spice_display_init_common(&qxl->ssd); diff --git a/hw/display/qxl.h b/hw/display/qxl.h index c5de3d7075..412e346b68 100644 --- a/hw/display/qxl.h +++ b/hw/display/qxl.h @@ -32,6 +32,7 @@ enum qxl_mode { typedef struct PCIQXLDevice { PCIDevice pci; + PortioList vga_port_list; SimpleSpiceDisplay ssd; int id; uint32_t debug; diff --git a/hw/display/vga.c b/hw/display/vga.c index c4c3238d5f..8cd6afe83a 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2355,8 +2355,6 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, { MemoryRegion *vga_io_memory; const MemoryRegionPortio *vga_ports, *vbe_ports; - PortioList *vga_port_list = g_new(PortioList, 1); - PortioList *vbe_port_list = g_new(PortioList, 1); qemu_register_reset(vga_reset, s); @@ -2371,13 +2369,13 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, 1); memory_region_set_coalescing(vga_io_memory); if (init_vga_ports) { - portio_list_init(vga_port_list, obj, vga_ports, s, "vga"); - portio_list_set_flush_coalesced(vga_port_list); - portio_list_add(vga_port_list, address_space_io, 0x3b0); + portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga"); + portio_list_set_flush_coalesced(&s->vga_port_list); + portio_list_add(&s->vga_port_list, address_space_io, 0x3b0); } if (vbe_ports) { - portio_list_init(vbe_port_list, obj, vbe_ports, s, "vbe"); - portio_list_add(vbe_port_list, address_space_io, 0x1ce); + portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe"); + portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce); } } diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h index d42ac926e3..5320abdc07 100644 --- a/hw/display/vga_int.h +++ b/hw/display/vga_int.h @@ -124,6 +124,8 @@ typedef struct VGACommonState { void (*get_resolution)(struct VGACommonState *s, int *pwidth, int *pheight); + PortioList vga_port_list; + PortioList vbe_port_list; /* bochs vbe state */ uint16_t vbe_index; uint16_t vbe_regs[VBE_DISPI_INDEX_NB]; diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c index dc7a767ee2..b8ad2e64ec 100644 --- a/hw/dma/i82374.c +++ b/hw/dma/i82374.c @@ -39,6 +39,7 @@ do { fprintf(stderr, "i82374 ERROR: " fmt , ## __VA_ARGS__); } while (0) typedef struct I82374State { uint8_t commands[8]; qemu_irq out; + PortioList port_list; } I82374State; static const VMStateDescription vmstate_i82374 = { @@ -137,10 +138,10 @@ static void i82374_isa_realize(DeviceState *dev, Error **errp) { ISAi82374State *isa = I82374(dev); I82374State *s = &isa->state; - PortioList *port_list = g_new(PortioList, 1); - portio_list_init(port_list, OBJECT(isa), i82374_portio_list, s, "i82374"); - portio_list_add(port_list, isa_address_space_io(&isa->parent_obj), + portio_list_init(&s->port_list, OBJECT(isa), i82374_portio_list, s, + "i82374"); + portio_list_add(&s->port_list, isa_address_space_io(&isa->parent_obj), isa->iobase); i82374_realize(s, errp); diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 55d01008d3..b28981bfde 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -108,15 +108,20 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start, const MemoryRegionPortio *pio_start, void *opaque, const char *name) { - PortioList *piolist = g_new(PortioList, 1); + PortioList piolist; /* START is how we should treat DEV, regardless of the actual contents of the portio array. This is how the old code actually handled e.g. the FDC device. */ isa_init_ioport(dev, start); - portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name); - portio_list_add(piolist, isabus->address_space_io, start); + /* FIXME: the device should store created PortioList in its state. Note + that DEV can be NULL here and that single device can register several + portio lists. Current implementation is leaking memory allocated + in portio_list_init. The leak is not critical because it happens only + at initialization time. */ + portio_list_init(&piolist, OBJECT(dev), pio_start, opaque, name); + portio_list_add(&piolist, isabus->address_space_io, start); } static void isa_device_init(Object *obj) diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c index e2436512f7..585937321f 100644 --- a/hw/ppc/prep.c +++ b/hw/ppc/prep.c @@ -361,6 +361,8 @@ static const MemoryRegionPortio prep_portio_list[] = { PORTIO_END_OF_LIST(), }; +static PortioList prep_port_list; + /* PowerPC PREP hardware initialisation */ static void ppc_prep_init(QEMUMachineInitArgs *args) { @@ -375,7 +377,6 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) CPUPPCState *env = NULL; nvram_t nvram; M48t59State *m48t59; - PortioList *port_list = g_new(PortioList, 1); #if 0 MemoryRegion *xcsr = g_new(MemoryRegion, 1); #endif @@ -542,8 +543,8 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) cpu = POWERPC_CPU(first_cpu); sysctrl->reset_irq = cpu->env.irq_inputs[PPC6xx_INPUT_HRESET]; - portio_list_init(port_list, NULL, prep_portio_list, sysctrl, "prep"); - portio_list_add(port_list, isa_address_space_io(isa), 0x0); + portio_list_init(&prep_port_list, NULL, prep_portio_list, sysctrl, "prep"); + portio_list_add(&prep_port_list, isa_address_space_io(isa), 0x0); /* PowerPC control and status register group */ #if 0 diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c index bc994a4c32..68b33e12be 100644 --- a/hw/watchdog/wdt_ib700.c +++ b/hw/watchdog/wdt_ib700.c @@ -42,6 +42,8 @@ typedef struct IB700state { ISADevice parent_obj; QEMUTimer *timer; + + PortioList port_list; } IB700State; /* This is the timer. We use a global here because the watchdog @@ -106,14 +108,13 @@ static const MemoryRegionPortio wdt_portio_list[] = { static void wdt_ib700_realize(DeviceState *dev, Error **errp) { IB700State *s = IB700(dev); - PortioList *port_list = g_new(PortioList, 1); ib700_debug("watchdog init\n"); s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s); - portio_list_init(port_list, OBJECT(s), wdt_portio_list, s, "ib700"); - portio_list_add(port_list, isa_address_space_io(&s->parent_obj), 0); + portio_list_init(&s->port_list, OBJECT(s), wdt_portio_list, s, "ib700"); + portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj), 0); } static void wdt_ib700_reset(DeviceState *dev)