Re: [PATCH v2 4/6] qdev: Change values of PropertyInfo member @type to be QAPI types

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

On Thu, Feb 27, 2025 at 09:55:59AM +0100, Markus Armbruster wrote:
> PropertyInfo member @type is externally visible via QMP
> device-list-properties and qom-list-properies.
> 
> Its meaning is not documented at its definition.
> 
> It gets passed to as @type argument to object_property_add() and

                 ^^^^^  "passed as the @type argument" ?

> object_class_property_add().  This argument's documentation isn't of
> much help, either:
> 
>  * @type: the type name of the property.  This namespace is pretty loosely
>  *   defined.  Sub namespaces are constructed by using a prefix and then
>  *   to angle brackets.  For instance, the type 'virtio-net-pci' in the
>  *   'link' namespace would be 'link<virtio-net-pci>'.
> 
> The two QMP commands document it as
> 
>  # @type: the type of the property.  This will typically come in one of
>  #     four forms:
>  #
>  #     1) A primitive type such as 'u8', 'u16', 'bool', 'str', or
>  #        'double'.  These types are mapped to the appropriate JSON
>  #        type.
>  #
>  #     2) A child type in the form 'child<subtype>' where subtype is a
>  #        qdev device type name.  Child properties create the
>  #        composition tree.
>  #
>  #     3) A link type in the form 'link<subtype>' where subtype is a
>  #        qdev device type name.  Link properties form the device model
>  #        graph.
> 
> "Typically come in one of four forms" followed by three items inspires
> the level of trust that is appropriate here.

> 
> Clean up a bunch of funnies:
> 
> * qdev_prop_fdc_drive_type.type is "FdcDriveType".  Its .enum_table
>   refers to QAPI type "FloppyDriveType".  So use that.
> 
> * qdev_prop_reserved_region is "reserved_region".  Its only user is an
>   array property called "reserved-regions".  Its .set() visits str.
>   So change @type to "str".
> 
> * trng_prop_fault_event_set.type is "uint32:bits".  Its .set() visits
>   uint32, so change @type to "uint32".  If we believe mentioning it's
>   actually bits is useful, the proper place would be .description.
> 
> * ccw_loadparm.type is "ccw_loadparm".  It's users are properties
>   called "loadparm".  Its .set() visits str.  So change @type to
>   "str".
> 
> * qdev_prop_nv_gpudirect_clique.type is "uint4".  Its set() visits
>   uint8, so change @type to "uint8".  If we believe mentioning the
>   range is useful, the proper place would be .description.
> 
> * s390_pci_fid_propinfo.type is "zpci_fid".  Its .set() visits uint32.
>   So change type to that, and move the "zpci_fid" to .description.
>   This is admittedly a lousy description, but it's still an
>   improvement; for instance, output of -device zpci,help changes from
> 
>       fid=<zpci_fid>
> 
>   to
> 
>       fid=<uint32>           - zpci_fid
> 
> * Similarly for a raft of PropertyInfo in target/riscv/cpu.c.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/core/qdev-properties-system.c |  4 +--
>  hw/misc/xlnx-versal-trng.c       |  2 +-
>  hw/s390x/ccw-device.c            |  2 +-
>  hw/s390x/s390-pci-bus.c          |  3 ++-
>  hw/vfio/pci-quirks.c             |  2 +-
>  target/riscv/cpu.c               | 44 ++++++++++++++++++++++----------
>  6 files changed, 37 insertions(+), 20 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|