Re: [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link()

[Mark Cave-Ayland]

On 03/02/2023 18:08, Philippe Mathieu-Daudé wrote:

> Introduce qdev_prop_set_link(), equivalent of
> object_property_set_link() for QDev objects.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/core/qdev-properties.c    | 5 +++++
>   include/hw/qdev-properties.h | 1 +
>   2 files changed, 6 insertions(+)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 9789a2f5de..46236b1542 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -694,6 +694,11 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, 
> Object *obj,
>       }
>   }
>   
> +void qdev_prop_set_link(DeviceState *dev, const char *name, Object *value)
> +{
> +    object_property_set_link(OBJECT(dev), name, value, &error_abort);
> +}
> +
>   void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
>   {
>       object_property_set_bool(OBJECT(dev), name, value, &error_abort);
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 21f399e9a3..c16dbefb2f 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -191,6 +191,7 @@ bool qdev_prop_set_drive_err(DeviceState *dev, const char 
> *name,
>    * Set properties between creation and realization.
>    * @value must be valid.  Each property may be set at most once.
>    */
> +void qdev_prop_set_link(DeviceState *dev, const char *name, Object *value);
>   void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
>   void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
>   void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);

A general comment from me on this one: my feeling is that the main difference between 
QOM properties and qdev properties is that qdev properties are exposed to the user 
(for example they appear in the output of "-device foo,help") compared to QOM 
properties which tend to be used internally.

Following this thinking I'd always envisaged that an implementation of 
qdev_prop_set_link() would also be exposed to command line users so that you could 
set link properties from the command line similar to this:

  -device lance,id=lance0 -device ledma,dma=lance0

Of course this won't work in its current form (we don't have implicit ids for 
in-built devices as a starting point), but it does fit in with the recent discussions 
re: building machines completely from scratch. Certainly it feels to me as if this 
should be clarified before going ahead with a full-scale conversion for link 
properties as per this and your other related series.


ATB,

Mark.