Re: [PATCH 1/3] hw/pci: Avoid guard loadparm with target_s390x() for s390x in class_init

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Oct 14, 2026 at 04:05:22AM +0800, Yonggang Luo wrote:
> This is for avoid call target_info() in class_init.
> Now we directly guard in pci_qdev_property_set_loadparm instead.
> Don't know if its an API break, but the smallest change.

Suggest to reword this as follows:

 With the multi-target work, the goal is to move to a single
 unified QOM tree that applies universally across all targets.
 This complements the migration away from instance level
 properties to class level properties, which again aims to
 ensure a fixed QOM object model for each class.

 This requires that properties are either unconditionally
 registered in the class, with invalid options rejected
 at runtime (for example, during realize()), or that distinct
 subclasses are used for specialized use cases that need a
 distinct impl for some targets.

 It is harmless to have the 'loadparm' property registered
 unconditionally so take the simple option.

The same can apply to the other patches

> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> ---
>  hw/pci/pci.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)

> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 0efb4eb4bb0..9d3dcd52942 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2878,6 +2878,11 @@ static void pci_qdev_property_set_loadparm(Object *obj, const char *value,
>  {
>      void *lp_str;
>  
> +    if (!target_s390x()) {
> +        error_setg(errp, "'loadparm' is only supported on s390x");
> +        return;
> +    }
> +
>      if (object_property_get_int(obj, "bootindex", NULL) < 0) {
>          error_setg(errp, "'loadparm' is only valid for boot devices");
>          return;
> @@ -2895,14 +2900,11 @@ void pci_qdev_property_add_specifics(DeviceClass *dc)
>  {
>      ObjectClass *oc = OBJECT_CLASS(dc);
>  
> -    /* The loadparm property is only supported on s390x */
> -    if (target_s390x()) {
> -        object_class_property_add_str(oc, "loadparm",
> -                                      pci_qdev_property_get_loadparm,
> -                                      pci_qdev_property_set_loadparm);
> -        object_class_property_set_description(oc, "loadparm",
> -                                              "load parameter (s390x only)");
> -    }
> +    object_class_property_add_str(oc, "loadparm",
> +                                  pci_qdev_property_get_loadparm,
> +                                  pci_qdev_property_set_loadparm);
> +    object_class_property_set_description(oc, "loadparm",
> +                                          "load parameter (s390x only)");
>  }
>  
>  MemoryRegion *pci_address_space(PCIDevice *dev)
> -- 
> 2.55.0.windows.3
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|