qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, armbru@redhat.com, berrange@redhat.com,
	pbonzini@redhat.com
Subject: Re: [PATCH 11/11] qdev: Rework array properties based on list visitor
Date: Fri, 8 Sep 2023 19:33:55 +0200	[thread overview]
Message-ID: <ZPtbAw18MfRpZDY4@redhat.com> (raw)
In-Reply-To: <CAFEAcA9N3c1=Q-75iA-k4r-5FK0b3=yj+FtPFaj3Fg8aT4J3ZA@mail.gmail.com>

Am 08.09.2023 um 17:18 hat Peter Maydell geschrieben:
> On Fri, 8 Sept 2023 at 15:37, Kevin Wolf <kwolf@redhat.com> 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.
> >
> > 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.
> >
> > 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.
> 
> Could we have a specific example in the commit message of:
> 
> The old (currently broken) syntax for setting the ports
> property on the rocker device is:
>  -device rocker,len-ports=2,ports[0]=dev0,ports[1]=dev1
> The new syntax that works as of this commit is:
>  [whatever]
> 
> ?

Sure, that's a good idea.

> I would expect most users have no idea what the JSON list
> syntax is.

To fill in your "[whatever]", it's something like this:

-device '{"driver":"rocker","ports":["dev0","dev1"]}'

If we can eventually get -device converted to the keyval parser instead
of QemuOpts, the non-JSON syntax will look like this:

-device rocker,ports.0=dev0,ports.1=dev1

But I assume we'll have to solve some compatibility problems with other
devices before this can be done.

Kevin



  reply	other threads:[~2023-09-08 17:35 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-08 14:36 [PATCH 00/11] qdev: Make array properties user accessible again Kevin Wolf
2023-09-08 14:36 ` [PATCH 01/11] qdev: Add qdev_prop_set_array() Kevin Wolf
2023-09-11 15:41   ` Peter Maydell
2023-09-11 20:54   ` Philippe Mathieu-Daudé
2023-09-22 14:25     ` Markus Armbruster
2023-10-27 18:06   ` Peter Maydell
2023-10-30 11:29     ` Kevin Wolf
2023-09-08 14:36 ` [PATCH 02/11] hw/i386/pc: Use qdev_prop_set_array() Kevin Wolf
2023-09-11 15:42   ` Peter Maydell
2023-09-11 16:42     ` Kevin Wolf
2023-09-12  9:17       ` Peter Maydell
2023-09-19  8:35         ` Markus Armbruster
2023-09-08 14:36 ` [PATCH 03/11] hw/arm/mps2-tz: " Kevin Wolf
2023-09-11 15:43   ` Peter Maydell
2023-09-08 14:36 ` [PATCH 04/11] hw/arm/mps2: " Kevin Wolf
2023-09-11 15:44   ` Peter Maydell
2023-09-08 14:36 ` [PATCH 05/11] hw/arm/sbsa-ref: " Kevin Wolf
2023-09-11 15:44   ` Peter Maydell
2023-09-08 14:36 ` [PATCH 06/11] hw/arm/vexpress: " Kevin Wolf
2023-09-11 15:44   ` Peter Maydell
2023-09-08 14:36 ` [PATCH 07/11] hw/arm/virt: " Kevin Wolf
2023-09-11 15:48   ` Peter Maydell
2023-09-08 14:36 ` [PATCH 08/11] hw/arm/xlnx-versal: " Kevin Wolf
2023-09-11 15:49   ` Peter Maydell
2023-09-08 14:37 ` [PATCH 09/11] hw/rx/rx62n: " Kevin Wolf
2023-09-11 15:50   ` Peter Maydell
2023-09-11 20:52   ` Philippe Mathieu-Daudé
2023-09-22 13:59   ` Markus Armbruster
2023-09-08 14:37 ` [PATCH 10/11] qom: Add object_property_set_default_list() Kevin Wolf
2023-09-11 15:52   ` Peter Maydell
2023-09-08 14:37 ` [PATCH 11/11] qdev: Rework array properties based on list visitor Kevin Wolf
2023-09-08 15:18   ` Peter Maydell
2023-09-08 17:33     ` Kevin Wolf [this message]
2023-09-14 10:24   ` Peter Maydell
2023-09-14 11:58     ` Kevin Wolf
2023-09-22 15:05   ` Markus Armbruster
2023-10-13 17:03     ` Kevin Wolf
2023-10-14  6:36       ` Markus Armbruster
2023-10-16 12:34         ` Kevin Wolf
2023-10-23 11:41           ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZPtbAw18MfRpZDY4@redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).