qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] Confused by QOM: /machine/unattached/device[5]/dr-connector[255]/fdt
Date: Wed, 9 Sep 2015 18:04:10 +0200	[thread overview]
Message-ID: <55F0587A.1070301@suse.de> (raw)
In-Reply-To: <87d1xr62ji.fsf@blackfin.pond.sub.org>

Am 09.09.2015 um 17:22 schrieb Markus Armbruster:
> Andreas Färber <afaerber@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": "/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": "/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 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.

> 
> Moving on to my next confusion: qom-get.
> 
>     QMP> { "execute": "qom-get", "arguments": { "path": "/machine/unattached/device[5]/dr-connector[255]", "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/

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.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

  reply	other threads:[~2015-09-09 16:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-09 14:38 [Qemu-devel] Confused by QOM: /machine/unattached/device[5]/dr-connector[255]/fdt Markus Armbruster
2015-09-09 14:46 ` Andreas Färber
2015-09-09 15:22   ` Markus Armbruster
2015-09-09 16:04     ` Andreas Färber [this message]
2015-09-09 16:16       ` Paolo Bonzini
2015-09-09 16:30         ` Andreas Färber
2015-09-13  2:04         ` Eric Blake
2015-09-10  8:55       ` 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=55F0587A.1070301@suse.de \
    --to=afaerber@suse.de \
    --cc=armbru@redhat.com \
    --cc=lcapitulino@redhat.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).