From 2b10a6760ee6d00dd404975549d51e079a26605c Mon Sep 17 00:00:00 2001 From: Cornelia Huck Date: Mon, 20 Nov 2023 10:42:59 +0100 Subject: [PATCH 01/19] hw: Add compat machines for 9.0 Add 9.0 machine types for arm/i440fx/m68k/q35/s390x/spapr. Signed-off-by: Cornelia Huck Message-ID: <20231120094259.1191804-1-cohuck@redhat.com> Acked-by: Thomas Huth Reviewed-by: Harsh Prateek Bora Reviewed-by: Gavin Shan Acked-by: Eric Farman # s390x Signed-off-by: Thomas Huth --- hw/arm/virt.c | 11 +++++++++-- hw/core/machine.c | 3 +++ hw/i386/pc.c | 3 +++ hw/i386/pc_piix.c | 17 ++++++++++++++--- hw/i386/pc_q35.c | 13 ++++++++++++- hw/m68k/virt.c | 11 +++++++++-- hw/ppc/spapr.c | 17 ++++++++++++++--- hw/s390x/s390-virtio-ccw.c | 14 +++++++++++++- include/hw/boards.h | 3 +++ include/hw/i386/pc.h | 3 +++ 10 files changed, 83 insertions(+), 12 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index be2856c018..efd503d45e 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -3180,10 +3180,17 @@ static void machvirt_machine_init(void) } type_init(machvirt_machine_init); -static void virt_machine_8_2_options(MachineClass *mc) +static void virt_machine_9_0_options(MachineClass *mc) { } -DEFINE_VIRT_MACHINE_AS_LATEST(8, 2) +DEFINE_VIRT_MACHINE_AS_LATEST(9, 0) + +static void virt_machine_8_2_options(MachineClass *mc) +{ + virt_machine_9_0_options(mc); + compat_props_add(mc->compat_props, hw_compat_8_2, hw_compat_8_2_len); +} +DEFINE_VIRT_MACHINE(8, 2) static void virt_machine_8_1_options(MachineClass *mc) { diff --git a/hw/core/machine.c b/hw/core/machine.c index 0c17398141..2699bcba53 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -32,6 +32,9 @@ #include "hw/virtio/virtio-net.h" #include "audio/audio.h" +GlobalProperty hw_compat_8_2[] = {}; +const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2); + GlobalProperty hw_compat_8_1[] = { { TYPE_PCI_BRIDGE, "x-pci-express-writeable-slt-bug", "true" }, { "ramfb", "x-migrate", "off" }, diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 29b9964733..496498df3a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -78,6 +78,9 @@ { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\ { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, }, +GlobalProperty pc_compat_8_2[] = {}; +const size_t pc_compat_8_2_len = G_N_ELEMENTS(pc_compat_8_2); + GlobalProperty pc_compat_8_1[] = {}; const size_t pc_compat_8_1_len = G_N_ELEMENTS(pc_compat_8_1); diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index eace854335..042c13cdbc 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -545,13 +545,26 @@ static void pc_i440fx_machine_options(MachineClass *m) "Use a different south bridge than PIIX3"); } -static void pc_i440fx_8_2_machine_options(MachineClass *m) +static void pc_i440fx_9_0_machine_options(MachineClass *m) { pc_i440fx_machine_options(m); m->alias = "pc"; m->is_default = true; } +DEFINE_I440FX_MACHINE(v9_0, "pc-i440fx-9.0", NULL, + pc_i440fx_9_0_machine_options); + +static void pc_i440fx_8_2_machine_options(MachineClass *m) +{ + pc_i440fx_9_0_machine_options(m); + m->alias = NULL; + m->is_default = false; + + compat_props_add(m->compat_props, hw_compat_8_2, hw_compat_8_2_len); + compat_props_add(m->compat_props, pc_compat_8_2, pc_compat_8_2_len); +} + DEFINE_I440FX_MACHINE(v8_2, "pc-i440fx-8.2", NULL, pc_i440fx_8_2_machine_options); @@ -560,8 +573,6 @@ static void pc_i440fx_8_1_machine_options(MachineClass *m) PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_i440fx_8_2_machine_options(m); - m->alias = NULL; - m->is_default = false; pcmc->broken_32bit_mem_addr_check = true; compat_props_add(m->compat_props, hw_compat_8_1, hw_compat_8_1_len); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 4f3e5412f6..f43d5142b8 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -383,12 +383,23 @@ static void pc_q35_machine_options(MachineClass *m) machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE); } -static void pc_q35_8_2_machine_options(MachineClass *m) +static void pc_q35_9_0_machine_options(MachineClass *m) { pc_q35_machine_options(m); m->alias = "q35"; } +DEFINE_Q35_MACHINE(v9_0, "pc-q35-9.0", NULL, + pc_q35_9_0_machine_options); + +static void pc_q35_8_2_machine_options(MachineClass *m) +{ + pc_q35_9_0_machine_options(m); + m->alias = NULL; + compat_props_add(m->compat_props, hw_compat_8_2, hw_compat_8_2_len); + compat_props_add(m->compat_props, pc_compat_8_2, pc_compat_8_2_len); +} + DEFINE_Q35_MACHINE(v8_2, "pc-q35-8.2", NULL, pc_q35_8_2_machine_options); diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c index 2e49e262ee..e2792ef46d 100644 --- a/hw/m68k/virt.c +++ b/hw/m68k/virt.c @@ -346,10 +346,17 @@ type_init(virt_machine_register_types) } \ type_init(machvirt_machine_##major##_##minor##_init); -static void virt_machine_8_2_options(MachineClass *mc) +static void virt_machine_9_0_options(MachineClass *mc) { } -DEFINE_VIRT_MACHINE(8, 2, true) +DEFINE_VIRT_MACHINE(9, 0, true) + +static void virt_machine_8_2_options(MachineClass *mc) +{ + virt_machine_9_0_options(mc); + compat_props_add(mc->compat_props, hw_compat_8_2, hw_compat_8_2_len); +} +DEFINE_VIRT_MACHINE(8, 2, false) static void virt_machine_8_1_options(MachineClass *mc) { diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index df09aa9d6a..9b6c1c129f 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4786,14 +4786,25 @@ static void spapr_machine_latest_class_options(MachineClass *mc) type_init(spapr_machine_register_##suffix) /* - * pseries-8.2 + * pseries-9.0 */ -static void spapr_machine_8_2_class_options(MachineClass *mc) +static void spapr_machine_9_0_class_options(MachineClass *mc) { /* Defaults for the latest behaviour inherited from the base class */ } -DEFINE_SPAPR_MACHINE(8_2, "8.2", true); +DEFINE_SPAPR_MACHINE(9_0, "9.0", true); + +/* + * pseries-8.2 + */ +static void spapr_machine_8_2_class_options(MachineClass *mc) +{ + spapr_machine_9_0_class_options(mc); + compat_props_add(mc->compat_props, hw_compat_8_2, hw_compat_8_2_len); +} + +DEFINE_SPAPR_MACHINE(8_2, "8.2", false); /* * pseries-8.1 diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 7262725d2e..1169e20b94 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -855,14 +855,26 @@ bool css_migration_enabled(void) } \ type_init(ccw_machine_register_##suffix) +static void ccw_machine_9_0_instance_options(MachineState *machine) +{ +} + +static void ccw_machine_9_0_class_options(MachineClass *mc) +{ +} +DEFINE_CCW_MACHINE(9_0, "9.0", true); + static void ccw_machine_8_2_instance_options(MachineState *machine) { + ccw_machine_9_0_instance_options(machine); } static void ccw_machine_8_2_class_options(MachineClass *mc) { + ccw_machine_9_0_class_options(mc); + compat_props_add(mc->compat_props, hw_compat_8_2, hw_compat_8_2_len); } -DEFINE_CCW_MACHINE(8_2, "8.2", true); +DEFINE_CCW_MACHINE(8_2, "8.2", false); static void ccw_machine_8_1_instance_options(MachineState *machine) { diff --git a/include/hw/boards.h b/include/hw/boards.h index da85f86efb..8af165f4dc 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -419,6 +419,9 @@ struct MachineState { } \ type_init(machine_initfn##_register_types) +extern GlobalProperty hw_compat_8_2[]; +extern const size_t hw_compat_8_2_len; + extern GlobalProperty hw_compat_8_1[]; extern const size_t hw_compat_8_1_len; diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index a10ceeabbf..916af29f7c 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -210,6 +210,9 @@ void pc_madt_cpu_entry(int uid, const CPUArchIdList *apic_ids, /* sgx.c */ void pc_machine_init_sgx_epc(PCMachineState *pcms); +extern GlobalProperty pc_compat_8_2[]; +extern const size_t pc_compat_8_2_len; + extern GlobalProperty pc_compat_8_1[]; extern const size_t pc_compat_8_1_len; From 6c4937245d90c0d2f8cedd2b25c2c17b8659c46a Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Tue, 7 Nov 2023 11:27:38 +0100 Subject: [PATCH 02/19] MAINTAINERS: Add some more vmware-related files to the corresponding section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These files are obviously related to Vmware emulation, so let's list them in the corresponding section in the MAINTAINERS file. Message-ID: <20231107102738.14797-1-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- MAINTAINERS | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 695e0bd34f..9cafa98a47 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2388,8 +2388,13 @@ F: hw/net/net_tx_pkt* Vmware M: Dmitry Fleytman S: Maintained +F: docs/specs/vmw_pvscsi-spec.txt +F: hw/display/vmware_vga.c F: hw/net/vmxnet* F: hw/scsi/vmw_pvscsi* +F: pc-bios/efi-vmxnet3.rom +F: pc-bios/vgabios-vmware.bin +F: roms/config.vga-vmware F: tests/qtest/vmxnet3-test.c F: docs/specs/vwm_pvscsi-spec.rst From 65eac5bd547f6e8abffb3e85bd2d793c609ab396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 12 Dec 2023 12:30:15 +0100 Subject: [PATCH 03/19] system/qtest: Include missing 'hw/core/cpu.h' header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "hw/core/cpu.h" declares 'first_cpu'. Include it to avoid when unrelated headers are refactored: system/qtest.c:548:33: error: use of undeclared identifier 'first_cpu' address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, ^ Signed-off-by: Philippe Mathieu-Daudé Message-ID: <20231212113016.29808-2-philmd@linaro.org> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- system/qtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/system/qtest.c b/system/qtest.c index 7964f0b248..6da58b3874 100644 --- a/system/qtest.c +++ b/system/qtest.c @@ -21,6 +21,7 @@ #include "exec/tswap.h" #include "hw/qdev-core.h" #include "hw/irq.h" +#include "hw/core/cpu.h" #include "qemu/accel.h" #include "sysemu/cpu-timers.h" #include "qemu/config-file.h" From 60144cf50943600c9b433924d3a98124aa4b49aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 12 Dec 2023 12:30:16 +0100 Subject: [PATCH 04/19] system/qtest: Restrict QTest API to system emulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Outside of system emulation, only qtest_enabled() can be used. Signed-off-by: Philippe Mathieu-Daudé Message-ID: <20231212113016.29808-3-philmd@linaro.org> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- include/sysemu/qtest.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h index 85f05b0e46..b5d5fd3463 100644 --- a/include/sysemu/qtest.h +++ b/include/sysemu/qtest.h @@ -23,6 +23,7 @@ static inline bool qtest_enabled(void) return qtest_allowed; } +#ifndef CONFIG_USER_ONLY void qtest_send_prefix(CharBackend *chr); void G_GNUC_PRINTF(2, 3) qtest_sendf(CharBackend *chr, const char *fmt, ...); void qtest_set_command_cb(bool (*pc_cb)(CharBackend *chr, gchar **words)); @@ -35,5 +36,6 @@ void qtest_server_set_send_handler(void (*send)(void *, const char *), void qtest_server_inproc_recv(void *opaque, const char *buf); int64_t qtest_get_virtual_clock(void); +#endif #endif From 43562e1882014c4ad1ec2c319e2c054d84c6295a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 12 Dec 2023 12:36:37 +0100 Subject: [PATCH 05/19] hw/ppc/spapr_hcall: Remove unused 'exec/exec-all.h' included header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Message-ID: <20231212113640.30287-2-philmd@linaro.org> Reviewed-by: Thomas Huth Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- hw/ppc/spapr_hcall.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 522a2396c7..fcefd1d1c7 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -8,7 +8,6 @@ #include "qemu/main-loop.h" #include "qemu/module.h" #include "qemu/error-report.h" -#include "exec/exec-all.h" #include "exec/tb-flush.h" #include "helper_regs.h" #include "hw/ppc/ppc.h" From b5570da73445aab7f67f03650d787f611f9b9ce9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 12 Dec 2023 12:36:38 +0100 Subject: [PATCH 06/19] hw/misc/mips_itu: Remove unnecessary 'exec/exec-all.h' header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mips_itu.c only requires declarations from "hw/core/cpu.h" and "cpu.h". Avoid including the huge "exec/exec-all.h" header. Signed-off-by: Philippe Mathieu-Daudé Message-ID: <20231212113640.30287-3-philmd@linaro.org> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- hw/misc/mips_itu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/misc/mips_itu.c b/hw/misc/mips_itu.c index 5a83ccc4e8..37aea0e737 100644 --- a/hw/misc/mips_itu.c +++ b/hw/misc/mips_itu.c @@ -22,9 +22,10 @@ #include "qemu/log.h" #include "qemu/module.h" #include "qapi/error.h" -#include "exec/exec-all.h" +#include "hw/core/cpu.h" #include "hw/misc/mips_itu.h" #include "hw/qdev-properties.h" +#include "target/mips/cpu.h" #define ITC_TAG_ADDRSPACE_SZ (ITC_ADDRESSMAP_NUM * 8) /* Initialize as 4kB area to fit all 32 cells with default 128B grain. From e1c85e5f0f047df2bf1bc96fa343eba6187e212a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 12 Dec 2023 12:36:39 +0100 Subject: [PATCH 07/19] hw/s390x/ipl: Remove unused 'exec/exec-all.h' included header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Message-ID: <20231212113640.30287-4-philmd@linaro.org> Reviewed-by: Eric Farman Signed-off-by: Thomas Huth --- hw/s390x/ipl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 515dcf51b5..62182d81a0 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -35,7 +35,6 @@ #include "qemu/cutils.h" #include "qemu/option.h" #include "standard-headers/linux/virtio_ids.h" -#include "exec/exec-all.h" #define KERN_IMAGE_START 0x010000UL #define LINUX_MAGIC_ADDR 0x010008UL From c0f6cd9f670888eed851cd0c06a803077020068c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Tue, 12 Dec 2023 12:36:40 +0100 Subject: [PATCH 08/19] target: Restrict 'sysemu/reset.h' to system emulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vCPU "reset" is only possible with system emulation. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Warner Losh Reviewed-by: Song Gao Message-ID: <20231212113640.30287-5-philmd@linaro.org> Reviewed-by: Thomas Huth Reviewed-by: Zhao Liu Signed-off-by: Thomas Huth --- target/i386/cpu.c | 2 +- target/loongarch/cpu.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index cd16cb893d..95d5f16cd5 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -24,7 +24,6 @@ #include "qemu/hw-version.h" #include "cpu.h" #include "tcg/helper-tcg.h" -#include "sysemu/reset.h" #include "sysemu/hvf.h" #include "hvf/hvf-i386.h" #include "kvm/kvm_i386.h" @@ -37,6 +36,7 @@ #include "hw/qdev-properties.h" #include "hw/i386/topology.h" #ifndef CONFIG_USER_ONLY +#include "sysemu/reset.h" #include "qapi/qapi-commands-machine-target.h" #include "exec/address-spaces.h" #include "hw/boards.h" diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c index fc075952e6..b26187dfde 100644 --- a/target/loongarch/cpu.c +++ b/target/loongarch/cpu.c @@ -17,7 +17,9 @@ #include "internals.h" #include "fpu/softfloat-helpers.h" #include "cpu-csr.h" +#ifndef CONFIG_USER_ONLY #include "sysemu/reset.h" +#endif #include "tcg/tcg.h" #include "vec.h" From bce9bbc3c991a087130babc243a9f2c1e6bcf8a5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 17 Nov 2023 12:44:53 +0100 Subject: [PATCH 09/19] docs/system/arm: Fix for rename of type "xlnx.bbram-ctrl" Fixes: b65b4b7ae3c8 (xlnx-bbram: hw/nvram: Use dot in device type name) Signed-off-by: Markus Armbruster [thuth: Use longhand syntax to avoid problems with the "." in the name] Reviewed-by: Peter Maydell Message-ID: <20231117114457.177308-2-thuth@redhat.com> Acked-by: Alistair Francis Signed-off-by: Thomas Huth --- docs/system/arm/xlnx-versal-virt.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/system/arm/xlnx-versal-virt.rst b/docs/system/arm/xlnx-versal-virt.rst index d2d1b26692..9a4b2ff55f 100644 --- a/docs/system/arm/xlnx-versal-virt.rst +++ b/docs/system/arm/xlnx-versal-virt.rst @@ -194,7 +194,7 @@ To use a different index value, N, from default of 0, add: .. code-block:: bash - -global xlnx,bbram-ctrl.drive-index=N + -global driver=xlnx.bbram-ctrl,property=drive-index,value=N eFUSE File Backend """""""""""""""""" From c455e011c61c91d152ac6170c2a3b7fbc89e194a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 17 Nov 2023 12:44:54 +0100 Subject: [PATCH 10/19] hw: Replace anti-social QOM type names (again) QOM type names containing ',' result in awful UI. We got rid of them in v6.0.0 (commit e178113ff64 hw: Replace anti-social QOM type names). A few have crept back since: xlnx,cframe-reg xlnx,efuse xlnx,pmc-efuse-cache xlnx,versal-cfu-apb xlnx,versal-cfu-fdro xlnx,versal-cfu-sfr xlnx,versal-crl xlnx,versal-efuse xlnx,zynqmp-efuse These are all device types. They can't be plugged with -device / device_add, except for "xlnx,efuse" (I'm not sure that one is intentional). They *can* be used with -device / device_add to request help. Usability is poor, though: you have to double the comma, like this: $ qemu-system-aarch64 -device xlnx,,pmc-efuse-cache,help They can also be used with -global, where you must *not* double the comma: $ qemu-system-aarch64 -global xlnx,efuse.drive-index=2 Trap for the unwary. "xlnx,efuse", "xlnx,versal-efuse", "xlnx,pmc-efuse-cache", "xlnx-zynqmp-efuse" are from v6.2.0, "xlnx,versal-crl" is from v7.1.0, and the remainder are new. Rename them all to "xlnx-FOO", like commit e178113ff64 did. Reported-by: Thomas Huth Signed-off-by: Markus Armbruster Reviewed-by: Thomas Huth Reviewed-by: Francisco Iglesias Message-ID: <20231117114457.177308-3-thuth@redhat.com> Reviewed-by: Alistair Francis Signed-off-by: Thomas Huth --- docs/system/arm/xlnx-versal-virt.rst | 2 +- include/hw/misc/xlnx-versal-cframe-reg.h | 2 +- include/hw/misc/xlnx-versal-cfu.h | 6 +++--- include/hw/misc/xlnx-versal-crl.h | 2 +- include/hw/nvram/xlnx-efuse.h | 2 +- include/hw/nvram/xlnx-versal-efuse.h | 4 ++-- include/hw/nvram/xlnx-zynqmp-efuse.h | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/system/arm/xlnx-versal-virt.rst b/docs/system/arm/xlnx-versal-virt.rst index 9a4b2ff55f..0bafc76469 100644 --- a/docs/system/arm/xlnx-versal-virt.rst +++ b/docs/system/arm/xlnx-versal-virt.rst @@ -212,7 +212,7 @@ To use a different index value, N, from default of 1, add: .. code-block:: bash - -global xlnx,efuse.drive-index=N + -global xlnx-efuse.drive-index=N .. warning:: In actual physical Versal, BBRAM and eFUSE contain sensitive data. diff --git a/include/hw/misc/xlnx-versal-cframe-reg.h b/include/hw/misc/xlnx-versal-cframe-reg.h index 0091505246..83f6a07744 100644 --- a/include/hw/misc/xlnx-versal-cframe-reg.h +++ b/include/hw/misc/xlnx-versal-cframe-reg.h @@ -23,7 +23,7 @@ #include "hw/misc/xlnx-versal-cfu.h" #include "qemu/fifo32.h" -#define TYPE_XLNX_VERSAL_CFRAME_REG "xlnx,cframe-reg" +#define TYPE_XLNX_VERSAL_CFRAME_REG "xlnx-cframe-reg" OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCFrameReg, XLNX_VERSAL_CFRAME_REG) #define TYPE_XLNX_VERSAL_CFRAME_BCAST_REG "xlnx.cframe-bcast-reg" diff --git a/include/hw/misc/xlnx-versal-cfu.h b/include/hw/misc/xlnx-versal-cfu.h index be62bab8c8..3de3ee4923 100644 --- a/include/hw/misc/xlnx-versal-cfu.h +++ b/include/hw/misc/xlnx-versal-cfu.h @@ -22,13 +22,13 @@ #include "hw/misc/xlnx-cfi-if.h" #include "qemu/fifo32.h" -#define TYPE_XLNX_VERSAL_CFU_APB "xlnx,versal-cfu-apb" +#define TYPE_XLNX_VERSAL_CFU_APB "xlnx-versal-cfu-apb" OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCFUAPB, XLNX_VERSAL_CFU_APB) -#define TYPE_XLNX_VERSAL_CFU_FDRO "xlnx,versal-cfu-fdro" +#define TYPE_XLNX_VERSAL_CFU_FDRO "xlnx-versal-cfu-fdro" OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCFUFDRO, XLNX_VERSAL_CFU_FDRO) -#define TYPE_XLNX_VERSAL_CFU_SFR "xlnx,versal-cfu-sfr" +#define TYPE_XLNX_VERSAL_CFU_SFR "xlnx-versal-cfu-sfr" OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCFUSFR, XLNX_VERSAL_CFU_SFR) REG32(CFU_ISR, 0x0) diff --git a/include/hw/misc/xlnx-versal-crl.h b/include/hw/misc/xlnx-versal-crl.h index 2857f4169a..dfb8dff197 100644 --- a/include/hw/misc/xlnx-versal-crl.h +++ b/include/hw/misc/xlnx-versal-crl.h @@ -13,7 +13,7 @@ #include "hw/register.h" #include "target/arm/cpu.h" -#define TYPE_XLNX_VERSAL_CRL "xlnx,versal-crl" +#define TYPE_XLNX_VERSAL_CRL "xlnx-versal-crl" OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalCRL, XLNX_VERSAL_CRL) REG32(ERR_CTRL, 0x0) diff --git a/include/hw/nvram/xlnx-efuse.h b/include/hw/nvram/xlnx-efuse.h index 58414e468b..cff7924106 100644 --- a/include/hw/nvram/xlnx-efuse.h +++ b/include/hw/nvram/xlnx-efuse.h @@ -30,7 +30,7 @@ #include "sysemu/block-backend.h" #include "hw/qdev-core.h" -#define TYPE_XLNX_EFUSE "xlnx,efuse" +#define TYPE_XLNX_EFUSE "xlnx-efuse" OBJECT_DECLARE_SIMPLE_TYPE(XlnxEFuse, XLNX_EFUSE); struct XlnxEFuse { diff --git a/include/hw/nvram/xlnx-versal-efuse.h b/include/hw/nvram/xlnx-versal-efuse.h index a873dc5cb0..86e2261b9a 100644 --- a/include/hw/nvram/xlnx-versal-efuse.h +++ b/include/hw/nvram/xlnx-versal-efuse.h @@ -29,8 +29,8 @@ #define XLNX_VERSAL_EFUSE_CTRL_R_MAX ((0x100 / 4) + 1) -#define TYPE_XLNX_VERSAL_EFUSE_CTRL "xlnx,versal-efuse" -#define TYPE_XLNX_VERSAL_EFUSE_CACHE "xlnx,pmc-efuse-cache" +#define TYPE_XLNX_VERSAL_EFUSE_CTRL "xlnx-versal-efuse" +#define TYPE_XLNX_VERSAL_EFUSE_CACHE "xlnx-pmc-efuse-cache" OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalEFuseCtrl, XLNX_VERSAL_EFUSE_CTRL); OBJECT_DECLARE_SIMPLE_TYPE(XlnxVersalEFuseCache, XLNX_VERSAL_EFUSE_CACHE); diff --git a/include/hw/nvram/xlnx-zynqmp-efuse.h b/include/hw/nvram/xlnx-zynqmp-efuse.h index 6b051ec4f1..f5beacc2e6 100644 --- a/include/hw/nvram/xlnx-zynqmp-efuse.h +++ b/include/hw/nvram/xlnx-zynqmp-efuse.h @@ -29,7 +29,7 @@ #define XLNX_ZYNQMP_EFUSE_R_MAX ((0x10fc / 4) + 1) -#define TYPE_XLNX_ZYNQMP_EFUSE "xlnx,zynqmp-efuse" +#define TYPE_XLNX_ZYNQMP_EFUSE "xlnx-zynqmp-efuse" OBJECT_DECLARE_SIMPLE_TYPE(XlnxZynqMPEFuse, XLNX_ZYNQMP_EFUSE); struct XlnxZynqMPEFuse { From a36ea38abde3bba671d95c6dff61fcc3828aee22 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 17 Nov 2023 12:44:55 +0100 Subject: [PATCH 11/19] memory: Remove "qemu:" prefix from the "qemu:ram-discard-manager" type name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Type names should not contain special characters like ":". Let's remove the whole prefix here since it does not really seem to be helpful to have such a prefix here. The type name is only used internally for an interface type, so the renaming should not affect the user interface or migration. Reviewed-by: David Hildenbrand Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20231117114457.177308-4-thuth@redhat.com> Reviewed-by: Alistair Francis Signed-off-by: Thomas Huth --- include/exec/memory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 831f7c996d..f172e82ac9 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -43,7 +43,7 @@ typedef struct IOMMUMemoryRegionClass IOMMUMemoryRegionClass; DECLARE_OBJ_CHECKERS(IOMMUMemoryRegion, IOMMUMemoryRegionClass, IOMMU_MEMORY_REGION, TYPE_IOMMU_MEMORY_REGION) -#define TYPE_RAM_DISCARD_MANAGER "qemu:ram-discard-manager" +#define TYPE_RAM_DISCARD_MANAGER "ram-discard-manager" typedef struct RamDiscardManagerClass RamDiscardManagerClass; typedef struct RamDiscardManager RamDiscardManager; DECLARE_OBJ_CHECKERS(RamDiscardManager, RamDiscardManagerClass, From 05f2320d40cfea74fad4bfc9ec5b645a3964ea05 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 17 Nov 2023 12:44:56 +0100 Subject: [PATCH 12/19] tests/unit/test-io-task: Rename "qemu:dummy" to avoid colon in the name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Type names should not contain special characters like ":" (so that they are easier to use with QAPI and other parts). We are going to forbid such names in an upcoming patch. Thus let's replace the ":" here with a "-". Reviewed-by: "Daniel P. Berrangé" Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20231117114457.177308-5-thuth@redhat.com> Reviewed-by: Alistair Francis Signed-off-by: Thomas Huth --- tests/unit/test-io-task.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test-io-task.c b/tests/unit/test-io-task.c index 953a50ae66..115dba8970 100644 --- a/tests/unit/test-io-task.c +++ b/tests/unit/test-io-task.c @@ -25,7 +25,7 @@ #include "qapi/error.h" #include "qemu/module.h" -#define TYPE_DUMMY "qemu:dummy" +#define TYPE_DUMMY "qemu-dummy" typedef struct DummyObject DummyObject; typedef struct DummyObjectClass DummyObjectClass; From b447378e121713faa4c63e8c93a8ebf25218fc40 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 17 Nov 2023 12:44:57 +0100 Subject: [PATCH 13/19] qom/object: Limit type names to alphanumerical and some few special characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QOM names currently don't have any enforced naming rules. This can be problematic, e.g. when they are used on the command line for the "-device" option (where the comma is used to separate properties). To avoid that such problematic type names come in again, let's restrict the set of acceptable characters during the type registration. Ideally, we'd apply here the same rules as for QAPI, i.e. all type names should begin with a letter, and contain only ASCII letters, digits, hyphen, and underscore. However, we already have so many pre-existing types like: 486-x86_64-cpu cfi.pflash01 power5+_v2.1-spapr-cpu-core virt-2.6-machine pc-i440fx-3.0-machine ... so that we have to allow "." and "+" for now, too. While the dot is used in a lot of places, the "+" can fortunately be limited to two classes of legacy names ("power" and "Sun-UltraSparc" CPUs). We also cannot enforce the rule that names must start with a letter yet, since there are lot of types that start with a digit. Still, at least limiting the first characters to the alphanumerical range should be way better than nothing. Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20231117114457.177308-6-thuth@redhat.com> Reviewed-by: Alistair Francis Signed-off-by: Thomas Huth --- qom/object.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/qom/object.c b/qom/object.c index 95c0dc8285..654e1afaf2 100644 --- a/qom/object.c +++ b/qom/object.c @@ -138,9 +138,50 @@ static TypeImpl *type_new(const TypeInfo *info) return ti; } +static bool type_name_is_valid(const char *name) +{ + const int slen = strlen(name); + int plen; + + g_assert(slen > 1); + + /* + * Ideally, the name should start with a letter - however, we've got + * too many names starting with a digit already, so allow digits here, + * too (except '0' which is not used yet) + */ + if (!g_ascii_isalnum(name[0]) || name[0] == '0') { + return false; + } + + plen = strspn(name, "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789-_."); + + /* Allow some legacy names with '+' in it for compatibility reasons */ + if (name[plen] == '+') { + if (plen == 6 && g_str_has_prefix(name, "power")) { + /* Allow "power5+" and "power7+" CPU names*/ + return true; + } + if (plen >= 17 && g_str_has_prefix(name, "Sun-UltraSparc-I")) { + /* Allow "Sun-UltraSparc-IV+" and "Sun-UltraSparc-IIIi+" */ + return true; + } + } + + return plen == slen; +} + static TypeImpl *type_register_internal(const TypeInfo *info) { TypeImpl *ti; + + if (!type_name_is_valid(info->name)) { + fprintf(stderr, "Registering '%s' with illegal type name\n", info->name); + abort(); + } + ti = type_new(info); type_table_add(ti); From 81c2c9dd5dc2a39b95317ca3a9baff0184f9f097 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 20 Nov 2023 12:39:51 +0100 Subject: [PATCH 14/19] tests/qtest/migration-test: Fix analyze-migration.py for s390x The migration stream on s390x contains data for the storage_attributes which the analyze-migration.py cannot handle yet. Add the basic code for handling this, so we can re-enable the check in the migration-test. Message-ID: <20231120113951.162090-1-thuth@redhat.com> Reviewed-by: Fabiano Rosas Signed-off-by: Thomas Huth --- scripts/analyze-migration.py | 35 +++++++++++++++++++++++++++++++++-- tests/qtest/migration-test.c | 4 +--- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py index de506cb8bf..a39dfb8766 100755 --- a/scripts/analyze-migration.py +++ b/scripts/analyze-migration.py @@ -263,6 +263,34 @@ class HTABSection(object): return "" +class S390StorageAttributes(object): + STATTR_FLAG_EOS = 0x01 + STATTR_FLAG_MORE = 0x02 + STATTR_FLAG_ERROR = 0x04 + STATTR_FLAG_DONE = 0x08 + + def __init__(self, file, version_id, device, section_key): + if version_id != 0: + raise Exception("Unknown storage_attributes version %d" % version_id) + + self.file = file + self.section_key = section_key + + def read(self): + while True: + addr_flags = self.file.read64() + flags = addr_flags & 0xfff + if (flags & (self.STATTR_FLAG_DONE | self.STATTR_FLAG_EOS)): + return + if (flags & self.STATTR_FLAG_ERROR): + raise Exception("Error in migration stream") + count = self.file.read64() + self.file.readvar(count) + + def getDict(self): + return "" + + class ConfigurationSection(object): def __init__(self, file, desc): self.file = file @@ -544,8 +572,11 @@ class MigrationDump(object): QEMU_VM_SECTION_FOOTER= 0x7e def __init__(self, filename): - self.section_classes = { ( 'ram', 0 ) : [ RamSection, None ], - ( 'spapr/htab', 0) : ( HTABSection, None ) } + self.section_classes = { + ( 'ram', 0 ) : [ RamSection, None ], + ( 's390-storage_attributes', 0 ) : [ S390StorageAttributes, None], + ( 'spapr/htab', 0) : ( HTABSection, None ) + } self.filename = filename self.vmsd_desc = None diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 0fbaa6a90f..d520c587f7 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -3360,9 +3360,7 @@ int main(int argc, char **argv) qtest_add_func("/migration/bad_dest", test_baddest); #ifndef _WIN32 - if (!g_str_equal(arch, "s390x")) { - qtest_add_func("/migration/analyze-script", test_analyze_script); - } + qtest_add_func("/migration/analyze-script", test_analyze_script); #endif qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain); qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle); From fd49b2153ed2448de6798fba31430a8dccea8ed8 Mon Sep 17 00:00:00 2001 From: Yihuan Pan Date: Wed, 13 Dec 2023 22:17:07 +0800 Subject: [PATCH 15/19] qemu-options: Clarify handling of commas in options parameters Provide explicit guidance on dealing with option parameters as arbitrary strings containing commas, such as in "file=my,file" and "string=a,b". The updated documentation emphasizes the need to double commas when they appear within such parameters. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1839 Signed-off-by: Yihuan Pan Message-ID: <20231213141706.629833-2-xun794@gmail.com> Signed-off-by: Thomas Huth --- docs/system/invocation.rst | 5 +++++ docs/system/qemu-manpage.rst | 5 +++++ qemu-options.hx | 4 ++++ 3 files changed, 14 insertions(+) diff --git a/docs/system/invocation.rst b/docs/system/invocation.rst index 4ba38fc23d..14b7db1c10 100644 --- a/docs/system/invocation.rst +++ b/docs/system/invocation.rst @@ -10,6 +10,11 @@ Invocation disk_image is a raw hard disk image for IDE hard disk 0. Some targets do not need a disk image. +When dealing with options parameters as arbitrary strings containing +commas, such as in "file=my,file" and "string=a,b", it's necessary to +double the commas. For instance,"-fw_cfg name=z,string=a,,b" will be +parsed as "-fw_cfg name=z,string=a,b". + .. hxtool-doc:: qemu-options.hx Device URL Syntax diff --git a/docs/system/qemu-manpage.rst b/docs/system/qemu-manpage.rst index c47a412758..3ade4ee45b 100644 --- a/docs/system/qemu-manpage.rst +++ b/docs/system/qemu-manpage.rst @@ -31,6 +31,11 @@ Options disk_image is a raw hard disk image for IDE hard disk 0. Some targets do not need a disk image. +When dealing with options parameters as arbitrary strings containing +commas, such as in "file=my,file" and "string=a,b", it's necessary to +double the commas. For instance,"-fw_cfg name=z,string=a,,b" will be +parsed as "-fw_cfg name=z,string=a,b". + .. hxtool-doc:: qemu-options.hx .. include:: keys.rst.inc diff --git a/qemu-options.hx b/qemu-options.hx index 42fd09e4de..a935aaae44 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4086,9 +4086,13 @@ DEF("fw_cfg", HAS_ARG, QEMU_OPTION_fwcfg, SRST ``-fw_cfg [name=]name,file=file`` Add named fw\_cfg entry with contents from file file. + If the filename contains comma, you must double it (for instance, + "file=my,,file" to use file "my,file"). ``-fw_cfg [name=]name,string=str`` Add named fw\_cfg entry with contents from string str. + If the string contains comma, you must double it (for instance, + "string=my,,string" to use file "my,string"). The terminating NUL character of the contents of str will not be included as part of the fw\_cfg item data. To insert contents with From 71dc6ca2a8fa0eca8ab8e87e6a55cb0114fa4c85 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 15 Dec 2023 15:35:24 +0100 Subject: [PATCH 16/19] tests/qtest/npcm7xx_pwm-test: Only do full testing in slow mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The npcm7xx_pwm-test can take quite a while when running with --enable-debug on a loaded system. The tests here are quite repetitive - by default it should be fine if we only execute some of them and only execute all when running in slow testing mode. Message-ID: <20231215143524.49241-1-thuth@redhat.com> Reviewed-by: "Daniel P. Berrangé" Signed-off-by: Thomas Huth --- tests/qtest/npcm7xx_pwm-test.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/qtest/npcm7xx_pwm-test.c b/tests/qtest/npcm7xx_pwm-test.c index ea4ca1d106..b53a43c417 100644 --- a/tests/qtest/npcm7xx_pwm-test.c +++ b/tests/qtest/npcm7xx_pwm-test.c @@ -606,6 +606,7 @@ static void test_toggle(gconstpointer test_data) uint32_t ppr, csr, pcr, cnr, cmr; int i, j, k, l; uint64_t expected_freq, expected_duty; + int cnr_step = g_test_quick() ? 2 : 1; mft_init(qts, td); @@ -618,7 +619,7 @@ static void test_toggle(gconstpointer test_data) csr = csr_list[j]; pwm_write_csr(qts, td, csr); - for (k = 0; k < ARRAY_SIZE(cnr_list); ++k) { + for (k = 0; k < ARRAY_SIZE(cnr_list); k += cnr_step) { cnr = cnr_list[k]; pwm_write_cnr(qts, td, cnr); @@ -678,6 +679,7 @@ static void pwm_add_test(const char *name, const TestData* td, int main(int argc, char **argv) { TestData test_data_list[ARRAY_SIZE(pwm_module_list) * ARRAY_SIZE(pwm_list)]; + int pwm_module_list_cnt = 1, pwm_list_cnt = 1; char *v_env = getenv("V"); @@ -687,8 +689,13 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); - for (int i = 0; i < ARRAY_SIZE(pwm_module_list); ++i) { - for (int j = 0; j < ARRAY_SIZE(pwm_list); ++j) { + if (!g_test_quick()) { + pwm_module_list_cnt = ARRAY_SIZE(pwm_module_list); + pwm_list_cnt = ARRAY_SIZE(pwm_list); + } + + for (int i = 0; i < pwm_module_list_cnt; ++i) { + for (int j = 0; j < pwm_list_cnt; ++j) { TestData *td = &test_data_list[i * ARRAY_SIZE(pwm_list) + j]; td->module = &pwm_module_list[i]; From c363764a6048f64d11c6cabf180154cf6f13ac3a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 22 Nov 2023 08:24:54 +0100 Subject: [PATCH 17/19] tests/unit/test-qmp-event: Drop superfluous mutex Mutex @test_event_lock is held from fixture setup to teardown, protecting global variable @test_event_data. But tests always run one after the other, so this is superfluous. It also confuses Coverity. Drop the mutex. Fixes: CID 1527425 Signed-off-by: Markus Armbruster Reviewed-by: Thomas Huth Message-ID: <20231122072456.2518816-2-armbru@redhat.com> Signed-off-by: Thomas Huth --- tests/unit/test-qmp-event.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c index 3626d2372f..c2c44687d5 100644 --- a/tests/unit/test-qmp-event.c +++ b/tests/unit/test-qmp-event.c @@ -30,7 +30,6 @@ typedef struct TestEventData { } TestEventData; TestEventData *test_event_data; -static GMutex test_event_lock; void test_qapi_event_emit(test_QAPIEvent event, QDict *d) { @@ -59,9 +58,6 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d) static void event_prepare(TestEventData *data, const void *unused) { - /* Global variable test_event_data was used to pass the expectation, so - test cases can't be executed at same time. */ - g_mutex_lock(&test_event_lock); test_event_data = data; } @@ -69,7 +65,6 @@ static void event_teardown(TestEventData *data, const void *unused) { test_event_data = NULL; - g_mutex_unlock(&test_event_lock); } static void event_test_add(const char *testpath, From 5712b7e4fbe661338d4581b574eb3b9da9f44915 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 22 Nov 2023 08:24:55 +0100 Subject: [PATCH 18/19] tests/unit/test-qmp-event: Simplify event emission check The generated qapi_event_send_FOO() call an event emitter function. It's test_qapi_event_emit() in this test. It compares the actual event to the expected event, and sets a flag to record it was called. The test functions set expected data and clear the flag before calling qapi_event_send_FOO(), and check the flag afterwards. Make test_qapi_event_emit() consume expected data, and the test functions check it was consumed. Delete the flag. This is simpler. It also catches extraneous calls of test_qapi_event_emit(). Catching that is not worthwhile, but since the cost is negative... Signed-off-by: Markus Armbruster Reviewed-by: Thomas Huth Message-ID: <20231122072456.2518816-3-armbru@redhat.com> Signed-off-by: Thomas Huth --- tests/unit/test-qmp-event.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c index c2c44687d5..5c9837e849 100644 --- a/tests/unit/test-qmp-event.c +++ b/tests/unit/test-qmp-event.c @@ -26,7 +26,6 @@ typedef struct TestEventData { QDict *expect; - bool emitted; } TestEventData; TestEventData *test_event_data; @@ -36,6 +35,8 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d) QDict *t; int64_t s, ms; + g_assert(test_event_data->expect); + /* Verify that we have timestamp, then remove it to compare other fields */ t = qdict_get_qdict(d, "timestamp"); g_assert(t); @@ -52,7 +53,8 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d) qdict_del(d, "timestamp"); g_assert(qobject_is_equal(QOBJECT(d), QOBJECT(test_event_data->expect))); - test_event_data->emitted = true; + qobject_unref(test_event_data->expect); + test_event_data->expect = NULL; } static void event_prepare(TestEventData *data, @@ -83,8 +85,7 @@ static void test_event_a(TestEventData *data, { data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_A' }"); qapi_event_send_event_a(); - g_assert(data->emitted); - qobject_unref(data->expect); + g_assert(!data->expect); } static void test_event_b(TestEventData *data, @@ -92,8 +93,7 @@ static void test_event_b(TestEventData *data, { data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_B' }"); qapi_event_send_event_b(); - g_assert(data->emitted); - qobject_unref(data->expect); + g_assert(!data->expect); } static void test_event_c(TestEventData *data, @@ -105,8 +105,7 @@ static void test_event_c(TestEventData *data, "{ 'event': 'EVENT_C', 'data': {" " 'a': 1, 'b': { 'integer': 2, 'string': 'test1' }, 'c': 'test2' } }"); qapi_event_send_event_c(true, 1, &b, "test2"); - g_assert(data->emitted); - qobject_unref(data->expect); + g_assert(!data->expect); } /* Complex type */ @@ -131,8 +130,7 @@ static void test_event_d(TestEventData *data, " 'string': 'test2', 'enum2': 'value2' }," " 'b': 'test3', 'enum3': 'value3' } }"); qapi_event_send_event_d(&a, "test3", NULL, true, ENUM_ONE_VALUE3); - g_assert(data->emitted); - qobject_unref(data->expect); + g_assert(!data->expect); } static void test_event_deprecated(TestEventData *data, const void *unused) @@ -142,15 +140,11 @@ static void test_event_deprecated(TestEventData *data, const void *unused) memset(&compat_policy, 0, sizeof(compat_policy)); qapi_event_send_test_event_features1(); - g_assert(data->emitted); + g_assert(!data->expect); compat_policy.has_deprecated_output = true; compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; - data->emitted = false; qapi_event_send_test_event_features1(); - g_assert(!data->emitted); - - qobject_unref(data->expect); } static void test_event_deprecated_data(TestEventData *data, const void *unused) @@ -160,17 +154,13 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," " 'data': { 'foo': 42 } }"); qapi_event_send_test_event_features0(42); - g_assert(data->emitted); + g_assert(!data->expect); - qobject_unref(data->expect); compat_policy.has_deprecated_output = true; compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); qapi_event_send_test_event_features0(42); - g_assert(data->emitted); - - qobject_unref(data->expect); } int main(int argc, char **argv) From 17b2ecc331eab274d448446aa51883040f9185ed Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 22 Nov 2023 08:24:56 +0100 Subject: [PATCH 19/19] tests/unit/test-qmp-event: Replace fixture by global variables The fixture buys us exactly nothing, as we need a global variable anyway, for test_qapi_event_emit(). Drop it. Signed-off-by: Markus Armbruster Reviewed-by: Thomas Huth Message-ID: <20231122072456.2518816-4-armbru@redhat.com> Signed-off-by: Thomas Huth --- tests/unit/test-qmp-event.c | 91 ++++++++++++------------------------- 1 file changed, 30 insertions(+), 61 deletions(-) diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c index 5c9837e849..08e95a382b 100644 --- a/tests/unit/test-qmp-event.c +++ b/tests/unit/test-qmp-event.c @@ -24,18 +24,14 @@ #include "test-qapi-events.h" #include "test-qapi-emit-events.h" -typedef struct TestEventData { - QDict *expect; -} TestEventData; - -TestEventData *test_event_data; +static QDict *expected_event; void test_qapi_event_emit(test_QAPIEvent event, QDict *d) { QDict *t; int64_t s, ms; - g_assert(test_event_data->expect); + g_assert(expected_event); /* Verify that we have timestamp, then remove it to compare other fields */ t = qdict_get_qdict(d, "timestamp"); @@ -52,65 +48,38 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d) qdict_del(d, "timestamp"); - g_assert(qobject_is_equal(QOBJECT(d), QOBJECT(test_event_data->expect))); - qobject_unref(test_event_data->expect); - test_event_data->expect = NULL; + g_assert(qobject_is_equal(QOBJECT(d), QOBJECT(expected_event))); + qobject_unref(expected_event); + expected_event = NULL; } -static void event_prepare(TestEventData *data, - const void *unused) +static void test_event_a(void) { - test_event_data = data; -} - -static void event_teardown(TestEventData *data, - const void *unused) -{ - test_event_data = NULL; -} - -static void event_test_add(const char *testpath, - void (*test_func)(TestEventData *data, - const void *user_data)) -{ - g_test_add(testpath, TestEventData, NULL, event_prepare, test_func, - event_teardown); -} - - -/* Test cases */ - -static void test_event_a(TestEventData *data, - const void *unused) -{ - data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_A' }"); + expected_event = qdict_from_jsonf_nofail("{ 'event': 'EVENT_A' }"); qapi_event_send_event_a(); - g_assert(!data->expect); + g_assert(!expected_event); } -static void test_event_b(TestEventData *data, - const void *unused) +static void test_event_b(void) { - data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_B' }"); + expected_event = qdict_from_jsonf_nofail("{ 'event': 'EVENT_B' }"); qapi_event_send_event_b(); - g_assert(!data->expect); + g_assert(!expected_event); } -static void test_event_c(TestEventData *data, - const void *unused) +static void test_event_c(void) { UserDefOne b = { .integer = 2, .string = (char *)"test1" }; - data->expect = qdict_from_jsonf_nofail( + expected_event = qdict_from_jsonf_nofail( "{ 'event': 'EVENT_C', 'data': {" " 'a': 1, 'b': { 'integer': 2, 'string': 'test1' }, 'c': 'test2' } }"); qapi_event_send_event_c(true, 1, &b, "test2"); - g_assert(!data->expect); + g_assert(!expected_event); } /* Complex type */ -static void test_event_d(TestEventData *data, - const void *unused) +static void test_event_d(void) { UserDefOne struct1 = { .integer = 2, .string = (char *)"test1", @@ -123,43 +92,43 @@ static void test_event_d(TestEventData *data, .enum2 = ENUM_ONE_VALUE2, }; - data->expect = qdict_from_jsonf_nofail( + expected_event = qdict_from_jsonf_nofail( "{ 'event': 'EVENT_D', 'data': {" " 'a': {" " 'struct1': { 'integer': 2, 'string': 'test1', 'enum1': 'value1' }," " 'string': 'test2', 'enum2': 'value2' }," " 'b': 'test3', 'enum3': 'value3' } }"); qapi_event_send_event_d(&a, "test3", NULL, true, ENUM_ONE_VALUE3); - g_assert(!data->expect); + g_assert(!expected_event); } -static void test_event_deprecated(TestEventData *data, const void *unused) +static void test_event_deprecated(void) { - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); + expected_event = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); memset(&compat_policy, 0, sizeof(compat_policy)); qapi_event_send_test_event_features1(); - g_assert(!data->expect); + g_assert(!expected_event); compat_policy.has_deprecated_output = true; compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; qapi_event_send_test_event_features1(); } -static void test_event_deprecated_data(TestEventData *data, const void *unused) +static void test_event_deprecated_data(void) { memset(&compat_policy, 0, sizeof(compat_policy)); - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," + expected_event = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," " 'data': { 'foo': 42 } }"); qapi_event_send_test_event_features0(42); - g_assert(!data->expect); + g_assert(!expected_event); compat_policy.has_deprecated_output = true; compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); + expected_event = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); qapi_event_send_test_event_features0(42); } @@ -167,12 +136,12 @@ int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); - event_test_add("/event/event_a", test_event_a); - event_test_add("/event/event_b", test_event_b); - event_test_add("/event/event_c", test_event_c); - event_test_add("/event/event_d", test_event_d); - event_test_add("/event/deprecated", test_event_deprecated); - event_test_add("/event/deprecated_data", test_event_deprecated_data); + g_test_add_func("/event/event_a", test_event_a); + g_test_add_func("/event/event_b", test_event_b); + g_test_add_func("/event/event_c", test_event_c); + g_test_add_func("/event/event_d", test_event_d); + g_test_add_func("/event/deprecated", test_event_deprecated); + g_test_add_func("/event/deprecated_data", test_event_deprecated_data); g_test_run(); return 0;