Re: [PATCH] docs: fix typo
On Thu, Dec 14, 2023 at 11:53:18PM +0100, Samuel Tardieu wrote: > Date: Thu, 14 Dec 2023 23:53:18 +0100 > From: Samuel Tardieu > Subject: [PATCH] docs: fix typo > X-Mailer: git-send-email 2.42.0 > > Signed-off-by: Samuel Tardieu > --- Reviewed-by: Zhao Liu > docs/tools/qemu-img.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst > index 4459c065f1..3653adb963 100644 > --- a/docs/tools/qemu-img.rst > +++ b/docs/tools/qemu-img.rst > @@ -406,7 +406,7 @@ Command description: >Compare exits with ``0`` in case the images are equal and with ``1`` >in case the images differ. Other exit codes mean an error occurred during >execution and standard error output should contain an error message. > - The following table sumarizes all exit codes of the compare subcommand: > + The following table summarizes all exit codes of the compare subcommand: > >0 > Images are identical (or requested help was printed) > -- > 2.42.0 > >
Re: [PATCH 01/21] hw/i386/pc: Do not use C99 mixed-declarations style
Hi Philippe, On Fri, Feb 16, 2024 at 12:02:52PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:02:52 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 01/21] hw/i386/pc: Do not use C99 mixed-declarations style > X-Mailer: git-send-email 2.41.0 > > QEMU's coding style generally forbids C99 mixed declarations. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/i386/pc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 196827531a..3c00a87317 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1227,6 +1227,7 @@ void pc_basic_device_init(struct PCMachineState *pcms, > */ > if (pcms->hpet_enabled) { > qemu_irq rtc_irq; > +uint8_t compat; > > hpet = qdev_try_new(TYPE_HPET); > if (!hpet) { > @@ -1238,8 +1239,7 @@ void pc_basic_device_init(struct PCMachineState *pcms, > * use IRQ16~23, IRQ8 and IRQ2. If the user has already set > * the property, use whatever mask they specified. > */ > -uint8_t compat = object_property_get_uint(OBJECT(hpet), > -HPET_INTCAP, NULL); > +compat = object_property_get_uint(OBJECT(hpet), HPET_INTCAP, NULL); > if (!compat) { > qdev_prop_set_uint32(hpet, HPET_INTCAP, hpet_irqs); > } "compat" is only used here to check. So, what about getting rid of this variable? if (!object_property_get_uint(OBJECT(hpet), HPET_INTCAP, NULL)) { qdev_prop_set_uint32(hpet, HPET_INTCAP, hpet_irqs); }
Re: [PATCH 02/21] hw/i386/pc_sysfw: Use qdev_is_realized() instead of QOM API
On Fri, Feb 16, 2024 at 12:02:53PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:02:53 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 02/21] hw/i386/pc_sysfw: Use qdev_is_realized() instead of > QOM API > X-Mailer: git-send-email 2.41.0 > > Prefer QDev API for QDev objects, avoid the underlying QOM layer. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/i386/pc_sysfw.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Zhao Liu > > diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c > index c8d9e71b88..3efabbbab2 100644 > --- a/hw/i386/pc_sysfw.c > +++ b/hw/i386/pc_sysfw.c > @@ -107,17 +107,15 @@ void pc_system_flash_cleanup_unused(PCMachineState > *pcms) > { > char *prop_name; > int i; > -Object *dev_obj; > > assert(PC_MACHINE_GET_CLASS(pcms)->pci_enabled); > > for (i = 0; i < ARRAY_SIZE(pcms->flash); i++) { > -dev_obj = OBJECT(pcms->flash[i]); > -if (!object_property_get_bool(dev_obj, "realized", &error_abort)) { > +if (!qdev_is_realized(DEVICE(pcms->flash[i]))) { > prop_name = g_strdup_printf("pflash%d", i); > object_property_del(OBJECT(pcms), prop_name); > g_free(prop_name); > -object_unparent(dev_obj); > +object_unparent(OBJECT(pcms->flash[i])); > pcms->flash[i] = NULL; > } > } > -- > 2.41.0 > >
Re: [PATCH 17/21] hw/i386/iommu: Prefer object_initialize_child over object_initialize
Hi Philippe, On Fri, Feb 16, 2024 at 12:03:08PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:03:08 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 17/21] hw/i386/iommu: Prefer object_initialize_child over > object_initialize > X-Mailer: git-send-email 2.41.0 > > When the QOM parent is available, prefer object_initialize_child() > over object_initialize(), since it create the parent relationship. > > Rename the 'klass' variable as 'obj' since the argument holds a > reference to an instance object and not a class one. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/i386/amd_iommu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c > index 7329553ad3..c3afbc4130 100644 > --- a/hw/i386/amd_iommu.c > +++ b/hw/i386/amd_iommu.c > @@ -1616,11 +1616,11 @@ static const VMStateDescription vmstate_amdvi_sysbus > = { > .unmigratable = 1 > }; > > -static void amdvi_sysbus_instance_init(Object *klass) > +static void amdvi_sysbus_instance_init(Object *obj) > { > -AMDVIState *s = AMD_IOMMU_DEVICE(klass); > +AMDVIState *s = AMD_IOMMU_DEVICE(obj); > > -object_initialize(&s->pci, sizeof(s->pci), TYPE_AMD_IOMMU_PCI); > +object_initialize_child(obj, "iommu", &s->pci, TYPE_AMD_IOMMU_PCI); What about this name "amd-iommu"? This is more accurate and differentiates it from the other intel-iommu related implementations. > } > > static void amdvi_sysbus_class_init(ObjectClass *klass, void *data) > -- > 2.41.0 > >
Re: [PATCH 11/21] hw/usb: Add QOM parentship relation with hub devices
Hi Philippe, On Fri, Feb 16, 2024 at 12:03:02PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:03:02 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 11/21] hw/usb: Add QOM parentship relation with hub devices > X-Mailer: git-send-email 2.41.0 > > QDev objects created with qdev_*new() need to manually add > their parent relationship with object_property_add_child(). > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/usb/bus.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/usb/bus.c b/hw/usb/bus.c > index a599e2552b..baad04f466 100644 > --- a/hw/usb/bus.c > +++ b/hw/usb/bus.c > @@ -439,6 +439,7 @@ void usb_claim_port(USBDevice *dev, Error **errp) > /* Create a new hub and chain it on */ > hub = USB_DEVICE(qdev_try_new("usb-hub")); One additional question comes to mind, should we use the qdev_new() here? The difference between qdev_try_new() and qdev_new() is the latter uses assert() to ensure the passed type exists. So if we know that type parameter is correct, we should just use qdev_new(). Only when the caller is not sure whether the type is valid, qdev_try_new() should be preferred. Am I understand correctly? ;-) > if (hub) { > +object_property_add_child(OBJECT(dev), "hub", OBJECT(hub)); >From the comment above the code: /* Create a new hub and chain it on */ this only creates a new usb-hub, should the new usb-hub become the child object of the original usb-hub? > usb_realize_and_unref(hub, bus, NULL); > } > } > -- > 2.41.0 > >
Re: [PATCH 06/21] hw: Replace DEVICE(object_new) -> qdev_new()
On Fri, Feb 16, 2024 at 12:02:57PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:02:57 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 06/21] hw: Replace DEVICE(object_new) -> qdev_new() > X-Mailer: git-send-email 2.41.0 > > Prefer QDev API for QDev objects, avoid the underlying QOM layer. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/arm/musicpal.c | 2 +- > hw/core/qdev.c| 2 +- > hw/sparc/sun4m.c | 4 ++-- > 3 files changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Zhao Liu > > diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c > index 2020f73a57..74e4d24aab 100644 > --- a/hw/arm/musicpal.c > +++ b/hw/arm/musicpal.c > @@ -1238,7 +1238,7 @@ static void musicpal_init(MachineState *machine) >qdev_get_gpio_in(pic, MP_TIMER4_IRQ), NULL); > > /* Logically OR both UART IRQs together */ > -uart_orgate = DEVICE(object_new(TYPE_OR_IRQ)); > +uart_orgate = qdev_new(TYPE_OR_IRQ); > object_property_set_int(OBJECT(uart_orgate), "num-lines", 2, > &error_fatal); > qdev_realize_and_unref(uart_orgate, NULL, &error_fatal); > qdev_connect_gpio_out(uart_orgate, 0, > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index c68d0f7c51..a271380d20 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -171,7 +171,7 @@ DeviceState *qdev_try_new(const char *name) > if (!module_object_class_by_name(name)) { > return NULL; > } > -return DEVICE(object_new(name)); > +return qdev_new(name); > } > > static QTAILQ_HEAD(, DeviceListener) device_listeners > diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c > index d52e6a7213..fedc4b8b3f 100644 > --- a/hw/sparc/sun4m.c > +++ b/hw/sparc/sun4m.c > @@ -979,7 +979,7 @@ static void sun4m_hw_init(MachineState *machine) > sysbus_mmio_map(s, 0, hwdef->ms_kb_base); > > /* Logically OR both its IRQs together */ > -ms_kb_orgate = DEVICE(object_new(TYPE_OR_IRQ)); > +ms_kb_orgate = qdev_new(TYPE_OR_IRQ); > object_property_set_int(OBJECT(ms_kb_orgate), "num-lines", 2, > &error_fatal); > qdev_realize_and_unref(ms_kb_orgate, NULL, &error_fatal); > sysbus_connect_irq(s, 0, qdev_get_gpio_in(ms_kb_orgate, 0)); > @@ -1000,7 +1000,7 @@ static void sun4m_hw_init(MachineState *machine) > sysbus_mmio_map(s, 0, hwdef->serial_base); > > /* Logically OR both its IRQs together */ > -serial_orgate = DEVICE(object_new(TYPE_OR_IRQ)); > +serial_orgate = qdev_new(TYPE_OR_IRQ); > object_property_set_int(OBJECT(serial_orgate), "num-lines", 2, > &error_fatal); > qdev_realize_and_unref(serial_orgate, NULL, &error_fatal); > -- > 2.41.0 > >
Re: [PATCH 09/21] hw/usb: Inline usb_try_new()
On Fri, Feb 16, 2024 at 12:03:00PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:03:00 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 09/21] hw/usb: Inline usb_try_new() > X-Mailer: git-send-email 2.41.0 > > Inline the single use of usb_try_new(). > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/usb/bus.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) Reviewed-by: Zhao Liu > > diff --git a/hw/usb/bus.c b/hw/usb/bus.c > index 59c39945dd..148224f06a 100644 > --- a/hw/usb/bus.c > +++ b/hw/usb/bus.c > @@ -334,11 +334,6 @@ USBDevice *usb_new(const char *name) > return USB_DEVICE(qdev_new(name)); > } > > -static USBDevice *usb_try_new(const char *name) > -{ > -return USB_DEVICE(qdev_try_new(name)); > -} > - > bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp) > { > return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp); > @@ -447,7 +442,7 @@ void usb_claim_port(USBDevice *dev, Error **errp) > } else { > if (bus->nfree == 1 && strcmp(object_get_typename(OBJECT(dev)), > "usb-hub") != 0) { > /* Create a new hub and chain it on */ > -hub = usb_try_new("usb-hub"); > +hub = USB_DEVICE(qdev_try_new("usb-hub")); > if (hub) { > usb_realize_and_unref(hub, bus, NULL); > } > -- > 2.41.0 > >
Re: [PATCH 07/21] target: Replace DEVICE(object_new) -> qdev_new()
On Fri, Feb 16, 2024 at 12:02:58PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:02:58 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 07/21] target: Replace DEVICE(object_new) -> qdev_new() > X-Mailer: git-send-email 2.41.0 > > Prefer QDev API for QDev objects, avoid the underlying QOM layer. > > Signed-off-by: Philippe Mathieu-Daudé > --- > target/mips/cpu.c | 2 +- > target/xtensa/cpu.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Zhao Liu > > diff --git a/target/mips/cpu.c b/target/mips/cpu.c > index d644adbc77..6b3909ee08 100644 > --- a/target/mips/cpu.c > +++ b/target/mips/cpu.c > @@ -649,7 +649,7 @@ MIPSCPU *mips_cpu_create_with_clock(const char *cpu_type, > Clock *cpu_refclk) > { > DeviceState *cpu; > > -cpu = DEVICE(object_new(cpu_type)); > +cpu = qdev_new(cpu_type); > qdev_connect_clock_in(cpu, "clk-in", cpu_refclk); > qdev_realize(cpu, NULL, &error_abort); > > diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c > index 79f91819df..4f9408e1a0 100644 > --- a/target/xtensa/cpu.c > +++ b/target/xtensa/cpu.c > @@ -205,7 +205,7 @@ XtensaCPU *xtensa_cpu_create_with_clock(const char > *cpu_type, Clock *cpu_refclk) > { > DeviceState *cpu; > > -cpu = DEVICE(object_new(cpu_type)); > +cpu = qdev_new(cpu_type); > qdev_connect_clock_in(cpu, "clk-in", cpu_refclk); > qdev_realize(cpu, NULL, &error_abort); > > -- > 2.41.0 > >
Re: [PATCH 10/21] hw/usb: Inline usb_new()
On Fri, Feb 16, 2024 at 12:03:01PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:03:01 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 10/21] hw/usb: Inline usb_new() > X-Mailer: git-send-email 2.41.0 > > Inline the 2 uses of usb_new(). > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/hw/usb.h| 1 - > hw/usb/bus.c| 9 ++--- > hw/usb/dev-serial.c | 2 +- > 3 files changed, 3 insertions(+), 9 deletions(-) Reviewed-by: Zhao Liu > > diff --git a/include/hw/usb.h b/include/hw/usb.h > index 32c23a5ca2..2d820685cc 100644 > --- a/include/hw/usb.h > +++ b/include/hw/usb.h > @@ -500,7 +500,6 @@ void usb_bus_release(USBBus *bus); > USBBus *usb_bus_find(int busnr); > void usb_legacy_register(const char *typename, const char *usbdevice_name, > USBDevice *(*usbdevice_init)(void)); > -USBDevice *usb_new(const char *name); > bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp); > USBDevice *usb_create_simple(USBBus *bus, const char *name); > USBDevice *usbdevice_create(const char *cmdline); > diff --git a/hw/usb/bus.c b/hw/usb/bus.c > index 148224f06a..a599e2552b 100644 > --- a/hw/usb/bus.c > +++ b/hw/usb/bus.c > @@ -329,11 +329,6 @@ void usb_legacy_register(const char *typename, const > char *usbdevice_name, > } > } > > -USBDevice *usb_new(const char *name) > -{ > -return USB_DEVICE(qdev_new(name)); > -} > - > bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp) > { > return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp); > @@ -341,7 +336,7 @@ bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, > Error **errp) > > USBDevice *usb_create_simple(USBBus *bus, const char *name) > { > -USBDevice *dev = usb_new(name); > +USBDevice *dev = USB_DEVICE(qdev_new(name)); > > usb_realize_and_unref(dev, bus, &error_abort); > return dev; > @@ -693,7 +688,7 @@ USBDevice *usbdevice_create(const char *driver) > return NULL; > } > > -dev = f->usbdevice_init ? f->usbdevice_init() : usb_new(f->name); > +dev = f->usbdevice_init ? f->usbdevice_init() : > USB_DEVICE(qdev_new(f->name)); > if (!dev) { > error_report("Failed to create USB device '%s'", f->name); > return NULL; > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c > index 63047d79cf..6e79c46d53 100644 > --- a/hw/usb/dev-serial.c > +++ b/hw/usb/dev-serial.c > @@ -624,7 +624,7 @@ static USBDevice *usb_braille_init(void) > return NULL; > } > > -dev = usb_new("usb-braille"); > +dev = USB_DEVICE(qdev_new("usb-braille")); > qdev_prop_set_chr(&dev->qdev, "chardev", cdrv); > return dev; > } > -- > 2.41.0 > >
Re: [PATCH 03/21] hw/ppc/spapr_cpu: Use qdev_is_realized() instead of QOM API
On Fri, Feb 16, 2024 at 12:02:54PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:02:54 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 03/21] hw/ppc/spapr_cpu: Use qdev_is_realized() instead of > QOM API > X-Mailer: git-send-email 2.41.0 > > Prefer QDev API for QDev objects, avoid the underlying QOM layer. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/ppc/spapr_cpu_core.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Zhao Liu > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 0c0fb3f1b0..40b7c52f7f 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -245,8 +245,7 @@ static void spapr_cpu_core_unrealize(DeviceState *dev) > * spapr_cpu_core_realize(), make sure we only unrealize > * vCPUs that have already been realized. > */ > -if (object_property_get_bool(OBJECT(sc->threads[i]), "realized", > - &error_abort)) { > +if (qdev_is_realized(DEVICE(sc->threads[i]))) { > spapr_unrealize_vcpu(sc->threads[i], sc); > } > spapr_delete_vcpu(sc->threads[i]); > -- > 2.41.0 > >
Re: [PATCH 21/21] hw: Add QOM parentship relation with CPUs
On Fri, Feb 16, 2024 at 12:03:12PM +0100, Philippe Mathieu-Daudé wrote: > Date: Fri, 16 Feb 2024 12:03:12 +0100 > From: Philippe Mathieu-Daudé > Subject: [PATCH 21/21] hw: Add QOM parentship relation with CPUs > X-Mailer: git-send-email 2.41.0 > > QDev objects created with object_new() need to manually add > their parent relationship with object_property_add_child(). > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/i386/x86.c| 1 + > hw/microblaze/petalogix_ml605_mmu.c | 1 + > hw/microblaze/petalogix_s3adsp1800_mmu.c | 1 + > hw/mips/cps.c| 1 + > hw/nios2/10m50_devboard.c| 1 + > hw/ppc/e500.c| 1 + > hw/ppc/spapr.c | 1 + > 7 files changed, 7 insertions(+) Reviewed-by: Zhao Liu > > diff --git a/hw/i386/x86.c b/hw/i386/x86.c > index 684dce90e9..7021419d91 100644 > --- a/hw/i386/x86.c > +++ b/hw/i386/x86.c > @@ -102,6 +102,7 @@ void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, > Error **errp) > if (!object_property_set_uint(cpu, "apic-id", apic_id, errp)) { > goto out; > } > +object_property_add_child(OBJECT(x86ms), "cpu[*]", OBJECT(cpu)); > qdev_realize(DEVICE(cpu), NULL, errp); > > out: > diff --git a/hw/microblaze/petalogix_ml605_mmu.c > b/hw/microblaze/petalogix_ml605_mmu.c > index 0f5fabc32e..dfd881322d 100644 > --- a/hw/microblaze/petalogix_ml605_mmu.c > +++ b/hw/microblaze/petalogix_ml605_mmu.c > @@ -83,6 +83,7 @@ petalogix_ml605_init(MachineState *machine) > > /* init CPUs */ > cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU)); > +object_property_add_child(OBJECT(machine), "cpu", OBJECT(cpu)); > object_property_set_str(OBJECT(cpu), "version", "8.10.a", &error_abort); > /* Use FPU but don't use floating point conversion and square > * root instructions > diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c > b/hw/microblaze/petalogix_s3adsp1800_mmu.c > index dad46bd7f9..255d8d4d47 100644 > --- a/hw/microblaze/petalogix_s3adsp1800_mmu.c > +++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c > @@ -70,6 +70,7 @@ petalogix_s3adsp1800_init(MachineState *machine) > MemoryRegion *sysmem = get_system_memory(); > > cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU)); > +object_property_add_child(OBJECT(machine), "cpu", OBJECT(cpu)); > object_property_set_str(OBJECT(cpu), "version", "7.10.d", &error_abort); > qdev_realize(DEVICE(cpu), NULL, &error_abort); > > diff --git a/hw/mips/cps.c b/hw/mips/cps.c > index 07b73b0a1f..6b4e918807 100644 > --- a/hw/mips/cps.c > +++ b/hw/mips/cps.c > @@ -84,6 +84,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp) > /* All cores use the same clock tree */ > qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock); > > +object_property_add_child(OBJECT(dev), "cpu[*]", OBJECT(cpu)); > if (!qdev_realize_and_unref(DEVICE(cpu), NULL, errp)) { > return; > } > diff --git a/hw/nios2/10m50_devboard.c b/hw/nios2/10m50_devboard.c > index 6cb32f777b..f6a691d340 100644 > --- a/hw/nios2/10m50_devboard.c > +++ b/hw/nios2/10m50_devboard.c > @@ -95,6 +95,7 @@ static void nios2_10m50_ghrd_init(MachineState *machine) > cpu->exception_addr = 0xc8000120; > cpu->fast_tlb_miss_addr = 0xc100; > > +object_property_add_child(OBJECT(machine), "cpu", OBJECT(cpu)); > qdev_realize_and_unref(DEVICE(cpu), NULL, &error_fatal); > > if (nms->vic) { > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > index 3bd12b54ab..77b7d2858c 100644 > --- a/hw/ppc/e500.c > +++ b/hw/ppc/e500.c > @@ -956,6 +956,7 @@ void ppce500_init(MachineState *machine) > */ > object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0, > &error_abort); > +object_property_add_child(OBJECT(machine), "cpu[*]", OBJECT(cpu)); > qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal); > > if (!firstenv) { > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 0d72d286d8..b6e5caa0d2 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2715,6 +2715,7 @@ static void spapr_init_cpus(SpaprMachineState *spapr) > &error_fatal); > object_property_set_int(core, CPU_CORE_PROP_CORE_ID, core_id, > &error_fatal); > +object_property_add_child(OBJECT(spapr), "cpu[*]", OBJECT(core)); > qdev_realize(DEVICE(core), NULL, &error_fatal); > > object_unref(core); > -- > 2.41.0 > >
[PATCH v2 03/29] block: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. In block.c, there are 4 functions passing @errp to error_prepend() without ERRP_GUARD(): - bdrv_co_create_opts_simple() - parse_json_filename() - bdrv_open_backing_file() - bdrv_append_temp_snapshot() bdrv_co_create_opts_simple(), is an implementation of BlockDriver.bdrv_co_create_opts(). There are too many possible callers to check the impact of this defect; it may or may not be harmless. Thus it is necessary to protect @errp with ERRP_GUARD(). Though the @errp parameters passed to parse_json_filename(), bdrv_open_backing_file() and bdrv_append_temp_snapshot() points to their callers' local_err, to follow the requirement of @errp, also add missing ERRP_GUARD() at their beginning. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu Reviewed-by: Eric Blake --- v2: * Use Markus' sentence to polish commit message. (Markus) * Fix typo. (Eric) --- block.c | 4 1 file changed, 4 insertions(+) diff --git a/block.c b/block.c index 1ed9214f66ed..cf66c767a011 100644 --- a/block.c +++ b/block.c @@ -633,6 +633,7 @@ int coroutine_fn bdrv_co_create_opts_simple(BlockDriver *drv, QemuOpts *opts, Error **errp) { +ERRP_GUARD(); BlockBackend *blk; QDict *options; int64_t size = 0; @@ -1998,6 +1999,7 @@ fail_opts: static QDict *parse_json_filename(const char *filename, Error **errp) { +ERRP_GUARD(); QObject *options_obj; QDict *options; int ret; @@ -3585,6 +3587,7 @@ int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, const char *bdref_key, Error **errp) { +ERRP_GUARD(); char *backing_filename = NULL; char *bdref_key_dot; const char *reference = NULL; @@ -3851,6 +3854,7 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs, QDict *snapshot_options, Error **errp) { +ERRP_GUARD(); g_autofree char *tmp_filename = NULL; int64_t total_size; QemuOpts *opts = NULL; -- 2.34.1
[PATCH v2 11/29] block/vdi: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The vdi_co_do_create() passes @errp to error_prepend() without ERRP_GUARD(), and its @errp parameter is so widely sourced that it is necessary to protect it with ERRP_GUARD(). To avoid the potential issues as [1] said, add missing ERRP_GUARD() at the beginning of this function. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Stefan Weil Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu --- block/vdi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/vdi.c b/block/vdi.c index 3b57becb9fe0..6363da08cee9 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -738,6 +738,7 @@ static int coroutine_fn GRAPH_UNLOCKED vdi_co_do_create(BlockdevCreateOptions *create_options, size_t block_size, Error **errp) { +ERRP_GUARD(); BlockdevCreateOptionsVdi *vdi_opts; int ret = 0; uint64_t bytes = 0; -- 2.34.1
[PATCH v2 07/29] block/qcow2-bitmap: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The qcow2_co_can_store_new_dirty_bitmap() passes @errp to error_prepend(). As a BlockDriver.bdrv_co_can_store_new_dirty_bitmap method, it's called by bdrv_co_can_store_new_dirty_bitmap(). Its caller is not being called anywhere, but as the API in include/block/block-io.h, we can't ensure what kind of @errp future users will pass in. To avoid potential issues as [1] said, add missing ERRP_GUARD() at the beginning of qcow2_co_can_store_new_dirty_bitmap(). [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Kevin Wolf Cc: Hanna Reitz Cc: Eric Blake Cc: Vladimir Sementsov-Ogievskiy Cc: John Snow Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/qcow2-bitmap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 0e567ed588d7..874ea5694851 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1710,6 +1710,7 @@ bool coroutine_fn qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs, uint32_t granularity, Error **errp) { +ERRP_GUARD(); BDRVQcow2State *s = bs->opaque; BdrvDirtyBitmap *bitmap; uint64_t bitmap_directory_size = 0; -- 2.34.1
[PATCH v2 08/29] block/qcow2: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. In block/qcow2.c, there are 2 functions passing @errp to error_prepend() without ERRP_GUARD(): - qcow2_co_create() - qcow2_co_truncate() There are too many possible callers to check the impact of the defect; it may or may not be harmless. Thus it is necessary to protect @errp with ERRP_GUARD(). Therefore, to avoid the issue like [1] said, add missing ERRP_GUARD() at their beginning. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu Reviewed-by: Eric Blake --- v2: * Use Markus' sentence to polish commit message. (Markus) * Fix typo. (Eric) --- block/qcow2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index 204f5854cff2..956128b40948 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3483,6 +3483,7 @@ static uint64_t qcow2_opt_get_refcount_bits_del(QemuOpts *opts, int version, static int coroutine_fn GRAPH_UNLOCKED qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp) { +ERRP_GUARD(); BlockdevCreateOptionsQcow2 *qcow2_opts; QDict *options; @@ -4283,6 +4284,7 @@ static int coroutine_fn GRAPH_RDLOCK qcow2_co_truncate(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, BdrvRequestFlags flags, Error **errp) { +ERRP_GUARD(); BDRVQcow2State *s = bs->opaque; uint64_t old_length; int64_t new_l1_size; -- 2.34.1
[PATCH v2 05/29] block/nbd: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The nbd_co_do_receive_one_chunk() passes @errp to error_prepend() without ERRP_GUARD(), and though its @errp parameter points to its caller's local_err, to follow the requirement of @errp, add missing ERRP_GUARD() at the beginning of this function. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Eric Blake Cc: Vladimir Sementsov-Ogievskiy Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/nbd.c b/block/nbd.c index b9d4f935e017..ef05f7cdfd65 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -852,6 +852,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk( BDRVNBDState *s, uint64_t cookie, bool only_structured, int *request_ret, QEMUIOVector *qiov, void **payload, Error **errp) { +ERRP_GUARD(); int ret; int i = COOKIE_TO_INDEX(cookie); void *local_payload = NULL; -- 2.34.1
[PATCH v2 09/29] block/qed: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The bdrv_qed_co_invalidate_cache() passes @errp to error_prepend() without ERRP_GUARD(). Though it is a BlockDriver.bdrv_co_invalidate_cache() method, and currently its @errp parameter only points to callers' local_err, to follow the requirement of @errp, add missing ERRP_GUARD() at the beginning of this function. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Stefan Hajnoczi Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu Reviewed-by: Stefan Hajnoczi --- block/qed.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/qed.c b/block/qed.c index bc2f0a61c0a9..fa5bc1108552 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1579,6 +1579,7 @@ bdrv_qed_co_change_backing_file(BlockDriverState *bs, const char *backing_file, static void coroutine_fn GRAPH_RDLOCK bdrv_qed_co_invalidate_cache(BlockDriverState *bs, Error **errp) { +ERRP_GUARD(); BDRVQEDState *s = bs->opaque; int ret; -- 2.34.1
[PATCH v2 10/29] block/snapshot: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. In block/snapshot.c, there are 2 functions passing @errp to error_prepend() without ERRP_GUARD(): - bdrv_all_delete_snapshot() - bdrv_all_goto_snapshot() As the APIs exposed in include/block/snapshot.h, they could be called by other modules. To avoid potential issues as [1] said, add missing ERRP_GUARD() at the beginning of these 2 functions. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu --- block/snapshot.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/snapshot.c b/block/snapshot.c index 8694fc0a3eba..8242b4abac41 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -566,6 +566,7 @@ int bdrv_all_delete_snapshot(const char *name, bool has_devices, strList *devices, Error **errp) { +ERRP_GUARD(); g_autoptr(GList) bdrvs = NULL; GList *iterbdrvs; @@ -605,6 +606,7 @@ int bdrv_all_goto_snapshot(const char *name, bool has_devices, strList *devices, Error **errp) { +ERRP_GUARD(); g_autoptr(GList) bdrvs = NULL; GList *iterbdrvs; int ret; -- 2.34.1
[PATCH v2 13/29] block/virtio-blk: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The virtio_blk_vq_aio_context_init() passes @errp to error_prepend(). Though its @errp points its caller's local @err variable, to follow the requirement of @errp, add missing ERRP_GUARD() at the beginning of virtio_blk_vq_aio_context_init(). [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: Stefan Hajnoczi Cc: "Michael S. Tsirkin" Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu Reviewed-by: Stefan Hajnoczi Acked-by: Michael S. Tsirkin --- hw/block/virtio-blk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 738cb2ac367d..92de315f17f7 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -1682,6 +1682,7 @@ static bool apply_iothread_vq_mapping( /* Context: BQL held */ static bool virtio_blk_vq_aio_context_init(VirtIOBlock *s, Error **errp) { +ERRP_GUARD(); VirtIODevice *vdev = VIRTIO_DEVICE(s); VirtIOBlkConf *conf = &s->conf; BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); -- 2.34.1
[PATCH v2 04/29] block/copy-before-write: Fix missing ERRP_GUARD() for error_prepend()
From: Zhao Liu As the comment in qapi/error, passing @errp to error_prepend() requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: ... * - It should not be passed to error_prepend(), error_vprepend() or * error_append_hint(), because that doesn't work with &error_fatal. * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. ERRP_GUARD() could avoid the case when @errp is &error_fatal, the user can't see this additional information, because exit() happens in error_setg earlier than information is added [1]. The cbw_open() passes @errp to error_prepend() without ERRP_GUARD(). Though it is the BlockDriver.bdrv_open() method, and currently its @errp parameter only points to callers' local_err, to follow the requirement of @errp, add missing ERRP_GUARD() at the beginning of this function. [1]: Issue description in the commit message of commit ae7c80a7bd73 ("error: New macro ERRP_GUARD()"). Cc: John Snow Cc: Vladimir Sementsov-Ogievskiy Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-block@nongnu.org Signed-off-by: Zhao Liu Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/copy-before-write.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/copy-before-write.c b/block/copy-before-write.c index 0842a1a6dfbe..8aba27a71d6d 100644 --- a/block/copy-before-write.c +++ b/block/copy-before-write.c @@ -407,6 +407,7 @@ out: static int cbw_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { +ERRP_GUARD(); BDRVCopyBeforeWriteState *s = bs->opaque; BdrvDirtyBitmap *bitmap = NULL; int64_t cluster_size; -- 2.34.1
Re: [PATCH 01/10] error: Drop superfluous #include "qapi/qmp/qerror.h"
On Tue, Mar 12, 2024 at 03:13:34PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:34 +0100 > From: Markus Armbruster > Subject: [PATCH 01/10] error: Drop superfluous #include "qapi/qmp/qerror.h" > > Signed-off-by: Markus Armbruster > --- > backends/iommufd.c | 1 - > chardev/char-fe.c | 1 - > system/rtc.c | 1 - > 3 files changed, 3 deletions(-) > Reviewed-by: Zhao Liu
Re: [PATCH 02/10] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition
On Tue, Mar 12, 2024 at 03:13:35PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:35 +0100 > From: Markus Armbruster > Subject: [PATCH 02/10] qapi: Inline and remove QERR_BUS_NO_HOTPLUG > definition > > From: Philippe Mathieu-Daudé > > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using sed, and manual cleanup. > > Signed-off-by: Philippe Mathieu-Daudé > Reviewed-by: Cédric Le Goater > Signed-off-by: Markus Armbruster > --- > include/qapi/qmp/qerror.h | 3 --- > hw/ppc/spapr_pci.c| 5 ++--- > system/qdev-monitor.c | 8 +--- > 3 files changed, 7 insertions(+), 9 deletions(-) > Reviewed-by: Zhao Liu
Re: [PATCH 03/10] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition
On Tue, Mar 12, 2024 at 03:13:36PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:36 +0100 > From: Markus Armbruster > Subject: [PATCH 03/10] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM > definition > > From: Philippe Mathieu-Daudé > > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using sed, and manual cleanup. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Markus Armbruster > --- > include/qapi/qmp/qerror.h | 3 --- > block/snapshot.c | 7 --- > blockdev.c | 2 +- > 3 files changed, 5 insertions(+), 7 deletions(-) > Reviewed-by: Zhao Liu
Re: [PATCH 04/10] qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition
On Tue, Mar 12, 2024 at 03:13:37PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:37 +0100 > From: Markus Armbruster > Subject: [PATCH 04/10] qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG > definition > > From: Philippe Mathieu-Daudé > > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using sed, and manual cleanup. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Markus Armbruster > --- > include/qapi/qmp/qerror.h | 3 --- > hw/core/qdev.c| 4 ++-- > system/qdev-monitor.c | 2 +- > 3 files changed, 3 insertions(+), 6 deletions(-) Reviewed-by: Zhao Liu
Re: [PATCH 05/10] qapi: Inline and remove QERR_INVALID_PARAMETER definition
On Tue, Mar 12, 2024 at 03:13:38PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:38 +0100 > From: Markus Armbruster > Subject: [PATCH 05/10] qapi: Inline and remove QERR_INVALID_PARAMETER > definition > > From: Philippe Mathieu-Daudé > > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using: > > $ sed -i -e "s/QERR_INVALID_PARAMETER,/\"Invalid parameter '%s'\",/" \ > $(git grep -lw QERR_INVALID_PARAMETER) > > Manually simplify qemu_opts_create(), and remove the macro definition > in include/qapi/qmp/qerror.h. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Markus Armbruster > --- > include/qapi/qmp/qerror.h | 3 --- > qapi/opts-visitor.c | 2 +- > util/qemu-option.c| 10 +- > 3 files changed, 6 insertions(+), 9 deletions(-) Reviewed-by: Zhao Liu
Re: [PATCH 06/10] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant value)
On Tue, Mar 12, 2024 at 03:13:39PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:39 +0100 > From: Markus Armbruster > Subject: [PATCH 06/10] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition > (constant value) > > From: Philippe Mathieu-Daudé > > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using the following > coccinelle semantic patch: > > @match@ > expression errp; > expression param; > constant value; > @@ > error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); > > @script:python strformat depends on match@ > value << match.value; > fixedfmt; // new var > @@ > fixedfmt = f'"Invalid parameter type for \'%s\', expected: {value[1:-1]}"' > coccinelle.fixedfmt = cocci.make_ident(fixedfmt) > > @replace@ > expression match.errp; > expression match.param; > constant match.value; > identifier strformat.fixedfmt; > @@ > -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); > +error_setg(errp, fixedfmt, param); > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Markus Armbruster > --- > qapi/qobject-input-visitor.c | 32 ++++---- > qapi/string-input-visitor.c | 8 > qom/object.c | 12 > 3 files changed, 28 insertions(+), 24 deletions(-) Reviewed-by: Zhao Liu
Re: [PATCH 07/10] qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE definition
On Tue, Mar 12, 2024 at 03:13:40PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:40 +0100 > From: Markus Armbruster > Subject: [PATCH 07/10] qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE > definition > > From: Philippe Mathieu-Daudé > > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Manual changes (escaping the format in qapi/visit.py). > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Markus Armbruster > --- > include/qapi/qmp/qerror.h | 3 --- > qom/object.c | 4 ++-- > scripts/qapi/visit.py | 5 ++--- > 3 files changed, 4 insertions(+), 8 deletions(-) > Reviewed-by: Zhao Liu
Re: [PATCH 09/10] qapi: Inline and remove QERR_MIGRATION_ACTIVE definition
On Tue, Mar 12, 2024 at 03:13:42PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:42 +0100 > From: Markus Armbruster > Subject: [PATCH 09/10] qapi: Inline and remove QERR_MIGRATION_ACTIVE > definition > > From: Philippe Mathieu-Daudé > > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using sed, manually > removing the definition in include/qapi/qmp/qerror.h. > > Signed-off-by: Philippe Mathieu-Daudé > Reviewed-by: Juan Quintela > Signed-off-by: Markus Armbruster > --- > include/qapi/qmp/qerror.h | 3 --- > migration/migration.c | 2 +- > migration/options.c | 4 ++-- > migration/savevm.c| 2 +- > 4 files changed, 4 insertions(+), 7 deletions(-) Reviewed-by: Zhao Liu
Re: [PATCH 10/10] qapi: Inline and remove QERR_PROPERTY_VALUE_BAD definition
On Tue, Mar 12, 2024 at 03:13:43PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:43 +0100 > From: Markus Armbruster > Subject: [PATCH 10/10] qapi: Inline and remove QERR_PROPERTY_VALUE_BAD > definition > > From: Philippe Mathieu-Daudé > > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Manual change. Remove the definition in > include/qapi/qmp/qerror.h. > > Signed-off-by: Philippe Mathieu-Daudé > Signed-off-by: Markus Armbruster > --- > include/qapi/qmp/qerror.h | 3 --- > hw/core/qdev-properties.c | 3 +-- > 2 files changed, 1 insertion(+), 5 deletions(-) Reviewed-by: Zhao Liu
Re: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit' parameter
On Tue, Mar 12, 2024 at 03:13:41PM +0100, Markus Armbruster wrote: > Date: Tue, 12 Mar 2024 15:13:41 +0100 > From: Markus Armbruster > Subject: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit' > parameter > > From: Philippe Mathieu-Daudé > > QERR_INVALID_PARAMETER_VALUE is defined as: > > #define QERR_INVALID_PARAMETER_VALUE \ > "Parameter '%s' expects %s" > > The current error is formatted as: > > "Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1 > MB/s" > > Replace by: > > "Parameter 'vcpu_dirty_limit' is invalid, it must greater than 1 MB/s" Is there a grammar error here? Maybe s/it must greater/it must be greater/ > Signed-off-by: Philippe Mathieu-Daudé > Reviewed-by: Juan Quintela > Signed-off-by: Markus Armbruster > --- > migration/options.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/migration/options.c b/migration/options.c > index 40eb930940..c5115f1b5c 100644 > --- a/migration/options.c > +++ b/migration/options.c > @@ -1264,9 +1264,8 @@ bool migrate_params_check(MigrationParameters *params, > Error **errp) > > if (params->has_vcpu_dirty_limit && > (params->vcpu_dirty_limit < 1)) { > -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, > - "vcpu_dirty_limit", > - "is invalid, it must greater then 1 MB/s"); > +error_setg(errp, "Parameter 'vcpu_dirty_limit' is invalid," > + " it must greater than 1 MB/s"); > return false; > } > Otherwise, Reviewed-by: Zhao Liu
Re: [PATCH 09/18] qapi/machine: Rename CpuS390* to S390Cpu, and drop 'prefix'
On Tue, Jul 30, 2024 at 10:10:23AM +0200, Markus Armbruster wrote: > Date: Tue, 30 Jul 2024 10:10:23 +0200 > From: Markus Armbruster > Subject: [PATCH 09/18] qapi/machine: Rename CpuS390* to S390Cpu, and drop > 'prefix' > > QAPI's 'prefix' feature can make the connection between enumeration > type and its constants less than obvious. It's best used with > restraint. > > CpuS390Entitlement has a 'prefix' to change the generated enumeration > constants' prefix from CPU_S390_POLARIZATION to S390_CPU_POLARIZATION. ^^ CPU_S390_ENTITLEMENT S390_CPU_ENTITLEMENT > Rename the type to S390CpuEntitlement, so that 'prefix' is not needed. > > Likewise change CpuS390Polarization to S390CpuPolarization, and > CpuS390State to S390CpuState. > > Signed-off-by: Markus Armbruster > --- > qapi/machine-common.json| 5 ++--- > qapi/machine-target.json| 11 +-- > qapi/machine.json | 9 - > qapi/pragma.json| 6 +++--- > include/hw/qdev-properties-system.h | 2 +- > include/hw/s390x/cpu-topology.h | 2 +- > target/s390x/cpu.h | 2 +- > hw/core/qdev-properties-system.c| 6 +++--- > hw/s390x/cpu-topology.c | 6 +++--- > 9 files changed, 23 insertions(+), 26 deletions(-) [snip] > diff --git a/qapi/pragma.json b/qapi/pragma.json > index 59fbe74b8c..beddea5ca4 100644 > --- a/qapi/pragma.json > +++ b/qapi/pragma.json > @@ -47,9 +47,9 @@ > 'BlockdevSnapshotWrapper', > 'BlockdevVmdkAdapterType', > 'ChardevBackendKind', > -'CpuS390Entitlement', > -'CpuS390Polarization', > -'CpuS390State', > +'S390CpuEntitlement', > +'S390CpuPolarization', > +'S390CpuState', > 'CxlCorErrorType', > 'DisplayProtocol', > 'DriveBackupWrapper', It seems to be in alphabetical order. The new names don't follow the original order. Just the above nits, Reviewed-by: Zhao Liu
Re: [PATCH 07/18] qapi/machine: Drop temporary 'prefix'
On Tue, Jul 30, 2024 at 10:10:21AM +0200, Markus Armbruster wrote: > Date: Tue, 30 Jul 2024 10:10:21 +0200 > From: Markus Armbruster > Subject: [PATCH 07/18] qapi/machine: Drop temporary 'prefix' > > Recent commit "qapi: Smarter camel_to_upper() to reduce need for > 'prefix'" added a temporary 'prefix' to delay changing the generated > code. > > Revert it. This improves HmatLBDataType's generated enumeration > constant prefix from HMATLB_DATA_TYPE to HMAT_LB_DATA_TYPE. > > Signed-off-by: Markus Armbruster > --- > qapi/machine.json| 1 - > hw/core/numa.c | 4 ++-- > hw/pci-bridge/cxl_upstream.c | 4 ++-- > 3 files changed, 4 insertions(+), 5 deletions(-) Reviewed-by: Zhao Liu