Re: [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link() - Mark Cave-Ayland

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>, qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	qemu-riscv@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Subject: Re: [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link()
Date: Sun, 5 Feb 2023 22:53:02 +0000	[thread overview]
Message-ID: <6a7298cd-9013-c3a8-ecf8-f8043448d207@ilande.co.uk> (raw)
In-Reply-To: <20230203180914.49112-3-philmd@linaro.org>

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.