Re: [PATCH 03/19] hw/acpi: Set QDev properties using QDev API

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, "Michael S. Tsirkin" <mst@redhat.com>,
	Ani Sinha <ani@anisinha.ca>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [PATCH 03/19] hw/acpi: Set QDev properties using QDev API
Date: Wed, 1 Mar 2023 15:07:13 +0100	[thread overview]
Message-ID: <20230301150713.48d4ef36@imammedo.users.ipa.redhat.com> (raw)
In-Reply-To: <20230203180914.49112-4-philmd@linaro.org>

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

> No need to use the low-level QOM API when an object
> inherits from QDev. Directly use the QDev API to set
> its properties.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/acpi/cpu_hotplug.c | 7 +++----
>  hw/acpi/ich9.c        | 4 ++--
>  hw/acpi/piix4.c       | 4 ++--
>  3 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index b8c9081738..5bc5f435f4 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -10,6 +10,7 @@
>   * See the COPYING file in the top-level directory.
>   */
>  #include "qemu/osdep.h"
> +#include "hw/qdev-properties.h"
>  #include "hw/acpi/cpu_hotplug.h"
>  #include "qapi/error.h"
>  #include "hw/core/cpu.h"
> @@ -41,8 +42,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
>       */
>      if (addr == 0 && data == 0) {
>          AcpiCpuHotplug *gpe = opaque;
> -        object_property_set_bool(OBJECT(gpe->parent), "cpu-hotplug-legacy",
> -                                 false, &error_abort);
> +        qdev_prop_set_bit(gpe->parent, "cpu-hotplug-legacy", false);
>      }
>  }
>  
> @@ -66,8 +66,7 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *gpe, CPUState *cpu)
>  
>      cpu_id = k->get_arch_id(cpu);
>      if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> -        object_property_set_bool(OBJECT(gpe->parent), "cpu-hotplug-legacy",
> -                                 false, &error_abort);
> +        qdev_prop_set_bit(gpe->parent, "cpu-hotplug-legacy", false);
>          return;
>      }
>  
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 6c9a737479..9759119b32 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -30,6 +30,7 @@
>  #include "hw/pci/pci.h"
>  #include "migration/vmstate.h"
>  #include "qemu/timer.h"
> +#include "hw/qdev-properties.h"
>  #include "hw/core/cpu.h"
>  #include "sysemu/reset.h"
>  #include "sysemu/runstate.h"
> @@ -197,8 +198,7 @@ static bool vmstate_test_use_cpuhp(void *opaque)
>  static int vmstate_cpuhp_pre_load(void *opaque)
>  {
>      ICH9LPCPMRegs *s = opaque;
> -    Object *obj = OBJECT(s->gpe.parent);
> -    object_property_set_bool(obj, "cpu-hotplug-legacy", false, &error_abort);
> +    qdev_prop_set_bit(s->gpe.parent, "cpu-hotplug-legacy", false);
>      return 0;
>  }
>  
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 3a61d89f92..9a86d506de 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -206,8 +206,8 @@ static bool vmstate_test_use_cpuhp(void *opaque)
>  
>  static int vmstate_cpuhp_pre_load(void *opaque)
>  {
> -    Object *obj = OBJECT(opaque);
> -    object_property_set_bool(obj, "cpu-hotplug-legacy", false, &error_abort);
> +    PIIX4PMState *s = opaque;

In other series you were aiming for type safety converting Object to DeviceState
and here you do opposite of that

> +    qdev_prop_set_bit(DEVICE(s), "cpu-hotplug-legacy", false);
if you have to, just use DEVICE(opaque) here and drop above pointer casting.

>      return 0;
>  }
>