On 02/02/18 18:37, Markus Armbruster wrote:
> Alexey Kardashevskiy <a...@ozlabs.ru> writes:
>> On 01/02/18 04:22, Markus Armbruster wrote:
>>> Alexey Kardashevskiy <a...@ozlabs.ru> writes:
>>>> There is already 'device-list-properties' which does most of the job,
>>>> however it does not handle everything returned by qom-list-types such
>>>> as machines as they inherit directly from TYPE_OBJECT and not TYPE_DEVICE.
>>>> This adds a new qom-list-properties command which prints properties
>>>> of a specific class and its instance. It is pretty much a simplified copy
>>>> of the device-list-properties handler.
>>>> Since it creates an object instance, device properties should appear
>>>> in the output as they are copied to QOM properties at the instance_init
>>>> hook.
>>>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>>> Related: qom-list, which lists "any properties of a object given a path
>>> in the object model."  qom-list-properties takes a type name, which
>>> qom-list takes the path to an instance.  In other words,
>>> qom-list-properties is like instantiate with default configuration and
>>> without realizing + qom-list + destroy.
>> True. Same as device-list-properties.
> device-list-properties does a bit more, like skipping "uninteresting"
> properties, and special magic for qdev properties (that's the
> make_device_property_info() you asked about below).  But that's detail.

It is not clear at all why would we want to skip some properties (yes,
question to Stefan, not you) but this does not explain
make_device_property_info() - this thing walks through the device
properties even though they are already copied to the Object properties and
we could enumerate those, all of them, I compared device-list-properties
and qom-list-properties:

-*** Sending {"execute": "device-list-properties", "arguments":
{"typename": "vfio-pci"}}
+*** Sending {"arguments": {"typename": "vfio-pci"}, "execute":

 {   'return': [   {   'description': 'on/off',
                       'name': 'x-igd-opregion',
                       'type': 'bool'},
+                  {'name': 'legacy-addr', 'type': 'str'},
                   {'name': 'x-pci-vendor-id', 'type': 'uint32'},
+                  {'name': 'type', 'type': 'string'},
                   {'name': 'x-pci-sub-device-id', 'type': 'uint32'},
                   {'name': 'rombar', 'type': 'uint32'},
                   {   'description': 'on/off',
@@ -19,10 +21,13 @@
                                      'host device, example: 04:10.0',
                       'name': 'host',
                       'type': 'str'},
+                  {'name': 'realized', 'type': 'bool'},
                   {'name': 'x-no-kvm-msix', 'type': 'bool'},
                   {   'description': 'on/off',
                       'name': 'command_serr_enable',
                       'type': 'bool'},
+                  {'name': 'parent_bus', 'type': 'link<bus>'},
+                  {'name': 'hotplugged', 'type': 'bool'},
                   {'name': 'x-pci-sub-vendor-id', 'type': 'uint32'},
                   {'name': 'x-pci-device-id', 'type': 'uint32'},
                   {   'description': 'on/off',
@@ -45,6 +50,7 @@
                       'type': 'bool'},
                   {'name': 'x-no-mmap', 'type': 'bool'},
                   {'name': 'bootindex', 'type': 'int32'},
+                  {'name': 'hotpluggable', 'type': 'bool'},
                   {'name': 'sysfsdev', 'type': 'str'},
                   {'description': 'on/off', 'name': 'x-vga', 'type': 'bool'},
                   {'name': 'romfile', 'type': 'str'}]}

The difference is "uninteresting" props and one suspicious "legacy-addr"
(which I have no idea what is that but it is offtopic here).

>>> We need to instantiate because QOM properties are dynamic: they aren't
>>> specified by data (which qom-list-properties could simply read), they
>>> are created by (instantiation) code (which qom-list-properties has to
>>> run).
>> Correct.
>>> Properties created only after instantiation (by realize, perhaps) aren't
>>> visible in qom-list-properties.  Do such properties exist?
>> No idea but if they do, then this issue already exists in
>> device-list-properties.
>>> Properties created only in non-default configuration aren't visible
>>> either.  Such properties have to exist, or else dynamic property
>>> creation would be idiotic.
> Thus, qom-list-properties design limitation: the result need not reflect
> properties of instantiated objects.  It usually does, as most QOM
> properties behave as if they were static.  But when it doesn't, what
> then?  How are users of qom-list-properties supposed to deal with such
> inaccurate / incorrect information?  Do they just have to know which
> properties aren't visible in qom-list-properties, and which properties
> are, but cannot be trusted?
> I posit that right now *nobody* knows.
> Would such a command be useful anyway?

qom-list-properties? yes, we want it to let libvirt a way to know if a
machine supports certain properties, if a machine was derived from a
device, then we would not need it.

>>> Likewise for properties created differently (say with a different type)
>>> in non-default configuration.  We can hope that no such beasts exist.
>>> Since properties get created by code, and code can do anything, we're
>>> reduced to hope.  Data is so much easier to reason about than code.
>>> Three building blocks: instantiate, qom-list, destroy.  Do we want the
>>> building blocks, or do we want their combination qom-list-properties?
>> Building blocks as QEMU internal helpers to split my
>> qmp_qom_list_properties() into? These are not going to be huge and
>> "destroy" is literally object_unref(obj) which does not seem very useful.
>> Or I missed the point here?
> My question is whether the QMP interface should provide the building
> blocks, or only compositions.

instantiate but not realize? Not sure we have users for that.

>>>> ---
>>>> I am missing the point of make_device_property_info().
>>>> qmp_device_list_properties() creates the instance which copies everything
>>>> to QOM properties hashtable and commenting out the do{}while() in
>>>> make_device_property_info() does not seem to change a thing, what case
>>>> am I missing here?
>>> git-blame points to Stefan.  Stefan, can you help?


Reply via email to