TL;DR: Andreas, there's one question specifically for you, search for
"QOM:".
Andreas Färber <afaer...@suse.de> writes:
> Am 09.09.2015 um 17:22 schrieb Markus Armbruster:
>> Andreas Färber <afaer...@suse.de> writes:
>>> Am 09.09.2015 um 16:38 schrieb Markus Armbruster:
>>>> I ran into this:
>>>>
>>>> $ qemu-system-ppc64 -nodefaults -S -display none -monitor stdio
>>>> -machine pseries-2.4
>>>> QEMU 2.4.50 monitor - type 'help' for more information
>>>> (qemu) qom-list /machine/unattached/device[5]/dr-connector[255]
>>>> fdt (struct)
>>>> entity-sense (uint32)
>>>> name (string)
>>>> connector_type (uint32)
>>>> index (uint32)
>>>> id (uint32)
>>>> allocation-state (uint32)
>>>> indicator-state (uint32)
>>>> isolation-state (uint32)
>>>> parent_bus (link<bus>)
>>>> hotplugged (bool)
>>>> hotpluggable (bool)
>>>> realized (bool)
>>>> type (string)
>>>> (qemu) qom-list /machine/unattached/device[5]/dr-connector[255]/fdt
>>>> Device '/machine/unattached/device[5]/dr-connector[255]/fdt' not found
>>>>
>>>> According to the first qom-list, .../fdt exists. According to the
>>>> second, it doesn't.
>>>
>>> Actually this is fully expected: qom-list operates on QOM objects. The
>>> fdt property returns a struct, which is considered a value QOM-wise, so
>>> to read it you need to use qom-get, not qom-list.
>>>
>>> Now, it may well be that visiting a struct does not work as expected, I
>>> remember we ran into issues there, that held up the qom-tree stuff...
>>
>> Okay, switching to QMP, because there's no qom-get in HMP (is that
>> intentional?).
>>
>> QMP> { "execute": "qom-list", "arguments": { "path":
>> QMP> "/machine/unattached/device[5]/dr-connector[255]" } }
>> {"return": [{"name": "fdt", "type": "struct"}, {"name":
>> "entity-sense", "type": "uint32"}, {"name": "name", "type":
>> "string"}, {"name": "connector_type", "type": "uint32"}, {"name":
>> "index", "type": "uint32"}, {"name": "id", "type": "uint32"},
>> {"name": "allocation-state", "type": "uint32"}, {"name":
>> "indicator-state", "type": "uint32"}, {"name": "isolation-state",
>> "type": "uint32"}, {"name": "parent_bus", "type": "link<bus>"},
>> {"name": "hotplugged", "type": "bool"}, {"name": "hotpluggable",
>> "type": "bool"}, {"name": "realized", "type": "bool"}, {"name":
>> "type", "type": "string"}]}
>> QMP> { "execute": "qom-list", "arguments": { "path":
>> QMP> "/machine/unattached/device[5]/dr-connector[255]/fdt" } }
>> {"error": {"class": "DeviceNotFound", "desc": "Device
>> '/machine/unattached/device[5]/dr-connector[255]/fdt' not found"}}
>>
>> Should qom-list really fail DeviceNotFound? I find it rather confusing.
>> For what it's worth, attempting to read a directory fails with EISDIR,
>> not ENOENT.
>
> Listing a non-existing directory on my system results in:
>
> ls: cannot access foo: No such file or directory
As Paolo remarked, we're listing an existing non-directory here, which
fails ENOTDIR. I used the wrong example (file-only op on directory
instead of directory-only op on file).
> As for the DeviceNotFound, I merely implemented the HMP glue, so you
> should rather direct such questions at Eric or Luiz (or Anthony).
> I believe that an ObjectNotFound is out of the question, as any new code
> would just use the Generic class.
Yes.
My (badly worded) question was about the misleading error *message*. I
don't care for the error *class*.
>>
>> Moving on to my next confusion: qom-get.
>>
>> QMP> { "execute": "qom-get", "arguments": { "path":
>> QMP> "/machine/unattached/device[5]/dr-connector[255]",
>> QMP> "property": "fdt" } }
>> {"return": {}}
>>
>> The return value {} is weird. Let's see where it comes from.
>>
>> qmp_qom_get() first calls object_resolve_path() on the path, then
>> object_property_get_qobject() on the property. For our test case,
>> object_resolve_path() succeeds. Now have a closer look at
>> object_property_get_qobject()'s behavior:
>>
>> QObject *object_property_get_qobject(Object *obj, const char *name,
>> Error **errp)
>> {
>> QObject *ret = NULL;
>> Error *local_err = NULL;
>> QmpOutputVisitor *mo;
>>
>> mo = qmp_output_visitor_new();
>> object_property_get(obj, qmp_output_get_visitor(mo), name,
>> &local_err);
>>
>> This call succeeds, and we enter the conditional.
>>
>> if (!local_err) {
>> ret = qmp_output_get_qobject(mo);
>>
>> This call returns NULL.
>>
>> }
>> error_propagate(errp, local_err);
>>
>> This sets *errp = NULL.
>>
>> qmp_output_visitor_cleanup(mo);
>> return ret;
>> }
>>
>> Function returns NULL without setting an error. Its function comment:
>>
>> /*
>> * object_property_get_qobject:
>> * @obj: the object
>> * @name: the name of the property
>> * @errp: returns an error if this function fails
>> *
>> * Returns: the value of the property, converted to QObject, or NULL if
>> * an error occurs.
>> */
>>
>> Is returning NULL without setting an error okay?
>>
>> Should it return qnull() instead? Then the QMP return value would be
>> JSON null.
>>
>
> That smells like the StringOutputVisitor problem that was holding up HMP
> qom-get:
>
> http://patchwork.ozlabs.org/patch/449596/
Interesting.
There are two software layers to consider, though:
1. QOM: What's the contract for object_property_add()'s argument @get()?
In particular, under what circumstances may it return without doing
anything (prop_get_fdt() does for me), and what does that mean?
This is where I could use guidance from core QOM maintainers.
2. Visitors: dealing with null
Not maintained by you. Of course, your advice is as welcome as
anyone's.
Besides the patch you quoted, there's also a suspicious FIXME in
qmp_output_first().
> IIRC I needed to add a test case - not sure if I did, and busy now.
>
> Searching for that subject should find you the qom-get patch as well.
Thanks.
