Re: [PATCH v2 12/12] qdev: Rework array properties based on list visitor

[Kevin Wolf <kwolf@redhat.com>]

Am 30.10.2023 um 21:48 hat Mark Cave-Ayland geschrieben:
> On 30/10/2023 14:26, Kevin Wolf wrote:
> 
> > Until now, array properties are actually implemented with a hack that
> > uses multiple properties on the QOM level: a static "foo-len" property
> > and after it is set, dynamically created "foo[i]" properties.
> > 
> > In external interfaces (-device on the command line and device_add in
> > QMP), this interface was broken by commit f3558b1b ('qdev: Base object
> > creation on QDict rather than QemuOpts') because QDicts are unordered
> > and therefore it could happen that QEMU tried to set the indexed
> > properties before setting the length, which fails and effectively makes
> > array properties inaccessible. In particular, this affects the 'ports'
> > property of the 'rocker' device, which used to be configured like this:
> > 
> > -device rocker,len-ports=2,ports[0]=dev0,ports[1]=dev1
> > 
> > This patch reworks the external interface so that instead of using a
> > separate top-level property for the length and for each element, we use
> > a single true array property that accepts a list value. In the external
> > interfaces, this is naturally expressed as a JSON list and makes array
> > properties accessible again. The new syntax looks like this:
> > 
> > -device '{"driver":"rocker","ports":["dev0","dev1"]}'
> > 
> > Creating an array property on the command line without using JSON format
> > is currently not possible. This could be fixed by switching from
> > QemuOpts to a keyval parser, which however requires consideration of the
> > compatibility implications.
> > 
> > All internal users of devices with array properties go through
> > qdev_prop_set_array() at this point, so updating it takes care of all of
> > them.
> 
> Is it possible to find a suitable place in the documentation to show
> how the new array syntax is used with -device on the command line? The
> description above is really useful, but I can see this being quite
> hard for users to find if it is only documented in the commit message.

Actually, it seems that the documentation doesn't explicitly mention
JSON syntax for command line options anywhere. Support for it is quite
widespread meanwhile. I can see it for at least:

-audiodev
-blockdev
-compat
-device
-display
-netdev
-object

In qemu-storage-daemon, it's additionally supported for:

--export
--monitor
--nbd-server

I think the manpage should at least mention for each of these options
that they support JSON.

Ideally, we'd then have a generic section that describes the mapping
between JSON and human syntax, but unfortunately, the human oriented
parsers differ quite a lot between these options, so there is nothing we
can describe and that is valid for all options. So maybe things like
"Specifying values for list properties is only possible with JSON
syntax" must be specified for each option where it applies.

Kevin