Re: [PATCH 2/2] hw/display: Allow injection of virtio-gpu EDID name

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> CC Markus to keep me honest in my comments below
>
> On Mon, Dec 02, 2024 at 03:31:53PM -0500, Andrew Keesler wrote:
>> Hi again Daniel. I have a follow up question. Can you help me
>> understand how I can declare this "outputs" property?
>> 
>>    -device '{"driver":"virtio-vga",
>>              "max_outputs":2,
>>              "id":"vga",
>>              "outputs":[
>>                {
>>                   "name":"AAA",
>>                },
>>                {
>>                   "name":"BBB",
>>                },
>>              ]}'
>> 
>> I thought DEFINE_PROP_ARRAY would do it, but I can't tell what PropertyInfo
>> implementation I should pass. All of the PropertyInfo implementations I can
>> find use scalar types, or simple text decoding. I am wondering if I am
>> missing
>> some sort of "JSON" encoding capabilities that can happen behind the scenes.
>
> I could have sworn we had an example of how to handle this already,
> but I'm not finding any Device class with a non-scalar property
> that isn't merely an array of scalars.

I found a few:

* TYPE_X86_CPU properties "feature-words" and "filtered-features" have
  struct type X86CPUFeatureWordInfo.  Defined in target/i386/cpu.c
  x86_cpu_initfn().

* TYPE_X86_CPU property "crash-information" has union type
  GuestPanicInformation.  Defined in target/i386/cpu.c
  x86_cpu_common_class_init().

* TYPE_VIRTIO_BLK property "iothread-vq-mapping" has type
  IOThreadVirtQueueMappingList, which is a list of struct
  IOThreadVirtQueueMapping.  Property defined in hw/block/virtio-blk.c
  virtio_blk_properties[] using macro
  DEFINE_PROP_IOTHREAD_VQ_MAPPING_LIST defined in
  qdev-properties-system.[ch].

In case you're curious how I fond them...  First, I collected device
help:

    $ for i in `upstream-qemu -nodefaults -S -display none -device help | sed 
-n 's/^name "\([^"]*\)".*/\1/p'`; do upstream-qemu -nodefaults -S -display none 
-device $i,help; done >dev-help

Then I extracted the type names:

    $ sed -n 's/.*=<\([^>]*\)>.*/\1/p' <dev-help | sort -u

Most of them "obviously" name scalars, QOM children or QOM links.
There's also crap like "list".  The promising ones are the ones
conforming to QAPI naming rules for types.

Look for them in the schema, drop the enums, and what's left are
non-scalar properties.

Since the type names are whatever the developer made up on the spot, my
search *can* miss non-scalar properties.

I didn't look at targets other than x86_64.

> We definitely have some examples elsewhere for exmaple "Machine" class
> has an SmpCacheProperties array property, and the QAuthZList class
> has an array of "QAuthZListRule" property.
>
> In both cases the struct is defined in th qapi/<blah>.json, which
> auto-generates code eg visit_type_QAuthZListRuleList, which can
> then get called from qauthz_list_prop_get_rules and
> qauthz_list_prop_set_rules, for the property.
>
> Devices use a slightly higher level wrapper so instead of calling
> object_class_property_add directly, then define the PropertyInfo
> and object_class_property_add gets called indirectly for them.
> I'm thinking it should still be possible to use the QAPI code
> generator to help though. You could either just define the struct,
> and thn use that to create  PropertyInfo to be used in combination
> with DEFINE_PROP_ARRAY, of you could define a list of structs at
> the QAPI level and use plain DEFINE_PROP. I guess the former is
> probably better aligned with other Device code.

Of the three instances I found, one uses such a qdev property machinery
(what you called "a slightly higher level wrapper"), and two do not.

Not sure what to recommend.