All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
	qemu-devel@nongnu.org, kwolf@redhat.com, hreitz@redhat.com,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Eduardo Habkost" <ehabkost@gmail.com>
Subject: Re: [RFC PATCH] qobject: Rewrite implementation of QDict for in-order traversal
Date: Mon, 18 Jul 2022 12:45:35 +0200	[thread overview]
Message-ID: <87a696d7hs.fsf@pond.sub.org> (raw)
In-Reply-To: <CAFEAcA80xNkyoPV78eJS5wdJ2CEtqkGQZMebuBSBgTJLNwpDGw@mail.gmail.com> (Peter Maydell's message of "Mon, 11 Jul 2022 12:15:22 +0100")

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 11 Jul 2022 at 12:09, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> On Mon, Jul 11, 2022 at 11:32:35AM +0100, Peter Maydell wrote:
>> > I'm pretty sure that nothing needs sparse array elements like
>> > that. The major reason for the len-PROP field is an implementation
>> > one: because there is currently no way for a QOM object to
>> > say "call this method if somebody tries to set a non-existent
>> > property", the way array properties work is that the 'set'
>> > method for the len-PROP property is the place where we then
>> > add the PROP[0], PROP[1], ... properties.
>>
>> Ahhh, I see what you mean. I totally missed this subtle detail.
>>
>> IIUC, there's essentially no such thing as array properties
>> in QOM. 'prop[0]', 'prop[1]', 'prop[2]', etc are all simply
>> scalar properties from QOM's, that just happen to follow a
>> common naming scheme, but QOM doesn't care about that.
>
> Yeah. The exception is the bit Markus pointed out where somebody
> has later added special case support for setting "prop[*]" to
> mean "whatever the next index is".
>
>> > If we either had a "call this for any property set/get attempt
>> > where there is no specific method set" or else had array
>> > properties supported by the core QOM code, we could avoid
>> > having to set len-PROP first.
>>
>> Techically arrays are already supported at the core QOM level, because
>> you can use any QAPI type as a property.  The authz/list.c object
>> has a 'rules' property that is an array of QAuthzListRule objects:
>>
>>   { 'struct': 'AuthZListProperties',
>>     'data': { '*policy': 'QAuthZListPolicy',
>>               '*rules': ['QAuthZListRule'] } }
>>
>> At the time I wrote that, we couldn't express it on the CLI though,
>> without using JSON syntax for -object. I don't think we've ever
>> made it possible to use the opts_visitor with non-scalar properties
>> though.

The opts visitor is flat by design.  Avoid it in new code, use
qobject_input_visitor_new_keyval() instead.  Typical use is via a
wrapper such as qobject_input_visitor_new_str().

> Mmm, if we had started the array-property implementation starting
> from a QAPI API (or even with the idea of setting properties from
> command line arguments) and working from there we'd probably have ended
> up with something different. The primary use case though has
> never involved QAPI or the command line, it's just C code for setting
> property values on devices created within QEMU.

Trouble with QOM is that things meant for internal use bleed to the
external interface so easily.  We've been quite cavalier about that.
Should we change our attitude?



  reply	other threads:[~2022-07-18 10:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-05  9:54 [RFC PATCH] qobject: Rewrite implementation of QDict for in-order traversal Markus Armbruster
2022-07-06 11:35 ` Markus Armbruster
2022-07-06 11:49   ` Mark Cave-Ayland
2022-07-08 11:01   ` Daniel P. Berrangé
2022-07-11 10:32     ` Peter Maydell
2022-07-11 11:09       ` Daniel P. Berrangé
2022-07-11 11:15         ` Peter Maydell
2022-07-18 10:45           ` Markus Armbruster [this message]
2022-07-07 12:57 ` Peter Maydell
2022-07-07 14:27   ` Markus Armbruster
2022-07-07 15:07     ` Daniel P. Berrangé
2022-07-07 14:43 ` Daniel P. Berrangé
2022-07-07 15:37 ` Daniel P. Berrangé
2022-07-07 15:52 ` Alex Bennée
2022-07-08 10:19   ` 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=87a696d7hs.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=ehabkost@gmail.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.