Re: [PATCH 0/1] qom: fix setting of qdev array properties

[Kevin Wolf]

Am 08.09.2023 um 11:53 hat Peter Maydell geschrieben:
> On Fri, 8 Sept 2023 at 10:26, Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > Am 07.09.2023 um 11:35 hat Peter Maydell geschrieben:
> > > On Thu, 7 Sept 2023 at 10:33, Markus Armbruster <armbru@redhat.com> wrote:
> > > >
> > > > Kevin Wolf <kwolf@redhat.com> writes:
> > > >
> > > > > Am 04.09.2023 um 18:25 hat Daniel P. Berrangé geschrieben:
> > > > >> By the time of the 8.2.0 release, it will have been 2 years and 6
> > > > >> releases since we accidentally broke setting of array properties
> > > > >> for user creatable devices:
> > > > >>
> > > > >>   https://gitlab.com/qemu-project/qemu/-/issues/1090
> > > > >
> > > > > Oh, nice!
> > > >
> > > > Nice?  *Awesome*!
> > > >
> > > > > Well, maybe that sounds a bit wrong, but the syntax that was broken 
> > > > > was
> > > > > problematic and more of a hack,
> > > >
> > > > A monstrosity, in my opinion.  I tried to strangle it in the crib, but
> > > > its guardians wouldn't let me.  Can dig up references for the morbidly
> > > > curious.
> > >
> > > I don't care about the syntax on the command line much (AFAIK that's
> > > just the rocker device). But the actual feature is used more widely
> > > within QEMU itself for devices created in C code, which is what it
> > > was intended for. If you want to get rid of it you need to provide
> > > an adequate replacement.
> >
> > I have a patch to use QList (i.e. JSON lists) that seems to work for the
> > rocker case. Now I need to find and update all of those internal
> > callers. Should grepping for '"len-' find all instances that need to be
> > changed or are you aware of other ways to access the feature?
> 
> AFAIK the only way to use the feature is to set the len-foo and
> then foo[0], foo[1], ... properties, using any of the usual APIs.
> So git grep '\<len-[^-]' should find them all.
> 
> If you want a cross-check, the devices that use it are easy
> to find (search for DEFINE_PROP_ARRAY), and almost all of them
> picked property names that are easy to grep for.
> 
> But as Daniel says, if you haven't changed the behaviour for
> "code sets the properties in the right order" they may not
> need updating.

I'm replacing the 'len-foo' and 'foo[0]' etc. properties with a single
'foo' property that takes a list, so the callers need to be adjusted.
The devices can stay unchanged, though.

> (I would be happy to see the rather hacky implementation replaced
> with true support for list properties at the qom/qdev level.
> But the hack is there because it was simpler :-))

It's not really that hard, but it would probably have been even easier
if we had just used the list support in the visitor interface from the
beginning instead of adding this hack. (Though obviously that wouldn't
have worked for user creatable devices before we added JSON support to
-device.)

Kevin