Re: [PATCH 00/19] hw: Set QDev properties using QDev API (part 1/3)

From: Igor Mammedov <imammedo@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>
Cc: qemu-devel@nongnu.org, qemu-arm@nongnu.org,
	Markus Armbruster <armbru@redhat.com>,
	Eduardo Habkost <eduardo@habkost.net>,
	qemu-riscv@nongnu.org
Subject: Re: [PATCH 00/19] hw: Set QDev properties using QDev API (part 1/3)
Date: Wed, 1 Mar 2023 14:42:39 +0100	[thread overview]
Message-ID: <20230301144239.7dc58097@imammedo.users.ipa.redhat.com> (raw)
In-Reply-To: <20230203180914.49112-1-philmd@linaro.org>

On Fri,  3 Feb 2023 19:08:55 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> QEMU provides the QOM API for core objects.
> Devices are modelled on top of QOM as QDev objects.
> 
> There is no point in using the lower level QOM API with
> QDev; it makes the code more complex and harder to review.
> 
> I first converted all the calls using errp=&error_abort or
> &errp=NULL, then noticed the other uses weren't really
> consistent.
> 
> A QDev property defined with the DEFINE_PROP_xxx() macros
> is always available, thus can't fail. When using hot-plug
> devices, we only need to check for optional properties
> registered at runtime with the object_property_add_XXX()
> API. Some are even always registered in device instance_init.
> 
> I have probably been overzealous, so I tagged the patches
> not using errp=&error_abort|&error_fatal|NULL as RFC.

such patches are wrong (12/19 is definitely broken),
you can do above only if failure is not expected
otherwise you are killing QEMU process during runtime.

So if you do this, plse describe in commit message why
it isn't breaking anything for each case.

> 
> PPC and ARM conversions will follow as two different series.
> 
> Philippe Mathieu-Daudé (19):
>   scripts/coccinelle: Add qom-qdev-prop.cocci
>   hw/qdev: Introduce qdev_prop_set_link()
>   hw/acpi: Set QDev properties using QDev API
>   hw/audio: Set QDev properties using QDev API
>   hw/core/numa: Set QDev properties using QDev API
>   hw/core/gpio: Set QDev properties using QDev API
>   hw/scsi: Set QDev properties using QDev API
>   hw/usb: Set QDev properties using QDev API
>   hw/virtio: Set QDev properties using QDev API
>   hw/avr: Set QDev properties using QDev API
>   hw/hppa: Set QDev properties using QDev API
>   hw/i386: Set QDev properties using QDev API
>   hw/m68k: Set QDev properties using QDev API
>   hw/microblaze: Set QDev properties using QDev API
>   hw/mips: Set QDev properties using QDev API
>   hw/nios2: Set QDev properties using QDev API
>   hw/riscv: Set QDev properties using QDev API
>   hw/rx: Set QDev properties using QDev API
>   hw/sparc: Set QDev properties using QDev API
> 
>  hw/acpi/cpu_hotplug.c                    |  7 ++--
>  hw/acpi/ich9.c                           |  4 +--
>  hw/acpi/piix4.c                          |  4 +--
>  hw/avr/arduino.c                         |  4 +--
>  hw/avr/atmega.c                          |  4 +--
>  hw/core/gpio.c                           |  8 ++---
>  hw/core/numa.c                           |  4 +--
>  hw/core/qdev-properties.c                |  5 +++
>  hw/display/virtio-gpu-pci.c              |  4 +--
>  hw/display/virtio-vga.c                  |  4 +--
>  hw/dma/sparc32_dma.c                     |  6 ++--
>  hw/hppa/machine.c                        |  3 +-
>  hw/i386/pc.c                             |  8 ++---
>  hw/i386/pc_q35.c                         | 25 +++++++-------
>  hw/i386/sgx.c                            |  3 +-
>  hw/i386/x86.c                            | 12 +++----
>  hw/m68k/next-cube.c                      |  2 +-
>  hw/m68k/q800.c                           |  7 ++--
>  hw/mem/pc-dimm.c                         |  5 ++-
>  hw/microblaze/petalogix_ml605_mmu.c      | 40 ++++++++++------------
>  hw/microblaze/petalogix_s3adsp1800_mmu.c |  2 +-
>  hw/microblaze/xlnx-zynqmp-pmu.c          | 40 +++++++++-------------
>  hw/mips/boston.c                         |  6 ++--
>  hw/mips/cps.c                            | 42 ++++++++----------------
>  hw/mips/jazz.c                           |  3 +-
>  hw/mips/malta.c                          |  6 ++--
>  hw/nios2/10m50_devboard.c                |  2 +-
>  hw/riscv/microchip_pfsoc.c               |  8 ++---
>  hw/riscv/opentitan.c                     |  9 ++---
>  hw/riscv/shakti_c.c                      |  7 ++--
>  hw/riscv/sifive_e.c                      |  8 ++---
>  hw/riscv/sifive_u.c                      |  9 ++---
>  hw/riscv/spike.c                         | 10 +++---
>  hw/riscv/virt.c                          |  9 ++---
>  hw/rx/rx-gdbsim.c                        | 11 +++----
>  hw/scsi/scsi-bus.c                       |  6 +---
>  hw/sparc/sun4m.c                         | 10 +++---
>  hw/sparc64/sun4u.c                       |  3 +-
>  hw/usb/hcd-xhci-pci.c                    |  2 +-
>  hw/usb/hcd-xhci-sysbus.c                 |  2 +-
>  hw/virtio/virtio-iommu-pci.c             |  3 +-
>  hw/virtio/virtio-rng.c                   |  3 +-
>  include/hw/audio/pcspk.h                 |  2 +-
>  include/hw/qdev-properties.h             |  1 +
>  target/i386/cpu.c                        |  6 ++--
>  target/i386/host-cpu.c                   |  7 ++--
>  46 files changed, 155 insertions(+), 221 deletions(-)
>  create mode 100644 scripts/coccinelle/qom-qdev-prop.cocci
> 