Re: The case for array properties (was: [PULL 14/15] qdev: Base object creation on QDict rather than QemuOpts)

[Peter Maydell]

On Fri, 8 Jul 2022 at 12:40, Markus Armbruster <armbru@redhat.com> wrote:
>
> Cc'ing QOM maintainers.
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Mon, 4 Jul 2022 at 05:50, Markus Armbruster <armbru@redhat.com> wrote:
> >> My initial (knee-jerk) reaction to breaking array properties: Faster,
> >> Pussycat! Kill! Kill!
> >
> > In an ideal world, what would you replace them with?
>
> Let's first recapitulate their intended purpose.
>
> commit 339659041f87a76f8b71ad3d12cadfc5f89b4bb3q
> Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Date:   Tue Aug 19 23:55:52 2014 -0700
>
>     qom: Add automatic arrayification to object_property_add()
>
>     If "[*]" is given as the last part of a QOM property name, treat that
>     as an array property. The added property is given the first available
>     name, replacing the * with a decimal number counting from 0.
>
>     First add with name "foo[*]" will be "foo[0]". Second "foo[1]" and so
>     on.
>
>     Callers may inspect the ObjectProperty * return value to see what
>     number the added property was given.
>
>     Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>     Signed-off-by: Andreas Färber <afaerber@suse.de>
>
> This describes how they work, but sadly not why we want them.  For such
> arcane lore, we need to consult a guru.  Possibly via the mailing list
> archive.

This is not the initial array property feature addition. It's
adding a convenience extra subfeature (ability to use "[*]"
to mean "next one in the array"). The inital array property
feature commit is:

commit 0be6bfac6262900425c10847de513ee2490078d3
Author: Peter Maydell <peter.maydell@linaro.org>
Date:   Fri Mar 15 16:41:57 2013 +0000

    qdev: Implement (variable length) array properties

    Add support for declaring array properties for qdev devices.
    These work by defining an initial static property 'len-arrayname'
    which the user of the device should set to the desired size
    of the array. When this property is set, memory is allocated
    for the array elements, and dynamic properties "arrayname[0]",
    "arrayname[1]"... are created so the user of the device can
    then set the values of the individual array elements.

Admittedly this doesn't give the justification either :-(
The reason is for the benefit of the following commit (and
similar use cases):

commit 8bd4824a6122292060a318741595927e0d05ff5e
Author: Peter Maydell <peter.maydell@linaro.org>
Date:   Fri Mar 15 16:41:57 2013 +0000

    hw/arm_sysctl: Implement SYS_CFG_VOLT

    Implement the SYS_CFG_VOLT registers which return the voltage
    of various supplies on motherboard and daughterboard. Since
    QEMU implements a perfectly stable power supply these registers
    always return a constant value. The number and value of the
    daughterboard voltages is dependent on the specific daughterboard,
    so we use a property array to allow the board to configure them
    appropriately.

> We occasionally have a need for an array of properties where the length
> of the array is not fixed at compile time.  Say in code common to
> several related devices, where some have two frobs, some four, and a
> future one may have some other number.
>
> We could define properties frob0, frob1, ... frobN for some fixed N.
> Users have to set them like frob0=...,frob1=... and so forth.  We need
> code to reject frobI=... for I exeeding the actual limit.
>
> Array properties spare developers picking a fixed N, and users adding an
> index to the property name.  Whether the latter is a good idea is
> unclear.  We need code to reject usage exceeding the actual limit.

Note that the initial intention of array properties was largely
for within-QEMU-code QOM properties. The usage of array properties
for user-creatable devices is a later development.

The basic rationale for array properties is:
Sometimes the thing we want to be able to configure on a device
is naturally expressed as "we have an indexed set of things".
When writing a device implementation, it's nice to be able to
straightforwardly say "this is an indexed set of things" and have
the property system set the device struct fields accordingly
(ie setting the num_foo struct field to the number of things
and the foo field to an allocated array of foos), and for the
way that device users to set the values of that indexed set
of things not to look too awful either.

The specific implementation we have today is just a cute hack
that avoided having to mess about too much with the core QOM
property implementation. (And it's only the implementation that
imposes the "set the length first" constraint, that's not
inherent to the idea of array properties.)

I don't have a strong feeling about the "[*]" automatic-indexing
subfeature -- none of the use of array properties that I've
added uses that.

thanks
-- PMM