scripts: Add qom-tree script as modern equivalent of info qtree
diff mbox

Message ID 1391621709-15620-1-git-send-email-afaerber@suse.de
State New
Headers show

Commit Message

Andreas Färber Feb. 5, 2014, 5:35 p.m. UTC
Functionally it is a recursive qom-list with qom-get per non-child<>
property. Some failures needed to be handled, such as trying to read a
pointer property, which is not representable in QMP. Those print a
literal "<EXCEPTION>".

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 scripts/qmp/qom-tree | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100755 scripts/qmp/qom-tree

Comments

Paolo Bonzini Feb. 5, 2014, 5:48 p.m. UTC | #1
Il 05/02/2014 18:35, Andreas Färber ha scritto:
> Functionally it is a recursive qom-list with qom-get per non-child<>
> property. Some failures needed to be handled, such as trying to read a
> pointer property, which is not representable in QMP. Those print a
> literal "<EXCEPTION>".
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>

I don't think it's a modern equivalent of anything.  The two are just 
different.

"info qtree" may be focused the old concept of buses, but those buses 
aren't going anywhere anytime soon.  "info qtree" may also be focused on 
the old concept of qdev properties (now "static" properties), but that's 
not something that cannot be fixed.

So, even though I think this script is a very welcome addition, I don't 
think it helps settling the question of what to do with "info qtree". 
IMO there's no good reason to exclude busless devices from "info qtree", 
and it's a bug (of course less severe than crashing, but still a bug) 
that the busless nand device doesn't appear there.

We can apply all three patches:

* qdev_try_create for busless devices (or alternatively abort when 
creating the device, qom-test will catch that)

* add qom-tree

* nand: Don't use qdev_create() in nand_init()

and still improve "info qtree" on top.

Paolo

> ---
>  scripts/qmp/qom-tree | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100755 scripts/qmp/qom-tree
>
> diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
> new file mode 100755
> index 0000000..5fd506a
> --- /dev/null
> +++ b/scripts/qmp/qom-tree
> @@ -0,0 +1,70 @@
> +#!/usr/bin/python
> +##
> +# QEMU Object Model test tools
> +#
> +# Copyright IBM, Corp. 2011
> +# Copyright (c) 2013 SUSE LINUX Products GmbH
> +#
> +# Authors:
> +#  Anthony Liguori   <aliguori@us.ibm.com>
> +#  Andreas Faerber   <afaerber@suse.de>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.  See
> +# the COPYING file in the top-level directory.
> +##
> +
> +import sys
> +import os
> +from qmp import QEMUMonitorProtocol
> +
> +cmd, args = sys.argv[0], sys.argv[1:]
> +socket_path = None
> +path = None
> +prop = None
> +
> +def usage():
> +    return '''environment variables:
> +    QMP_SOCKET=<path | addr:port>
> +usage:
> +    %s [-h] [-s <QMP socket path | addr:port>] [<path>]
> +''' % cmd
> +
> +def usage_error(error_msg = "unspecified error"):
> +    sys.stderr.write('%s\nERROR: %s\n' % (usage(), error_msg))
> +    exit(1)
> +
> +if len(args) > 0:
> +    if args[0] == "-h":
> +        print usage()
> +        exit(0);
> +    elif args[0] == "-s":
> +        try:
> +            socket_path = args[1]
> +        except:
> +            usage_error("missing argument: QMP socket path or address");
> +        args = args[2:]
> +
> +if not socket_path:
> +    if os.environ.has_key('QMP_SOCKET'):
> +        socket_path = os.environ['QMP_SOCKET']
> +    else:
> +        usage_error("no QMP socket path or address given");
> +
> +srv = QEMUMonitorProtocol(socket_path)
> +srv.connect()
> +
> +def list_node(path):
> +    print '%s' % path
> +    items = srv.command('qom-list', path=path)
> +    for item in items:
> +        if not item['type'].startswith('child<'):
> +            try:
> +                print '  %s: %s (%s)' % (item['name'], srv.command('qom-get', path=path, property=item['name']), item['type'])
> +            except:
> +                print '  %s: <EXCEPTION> (%s)' % (item['name'], item['type'])
> +    print ''
> +    for item in items:
> +        if item['type'].startswith('child<'):
> +            list_node(path + '/' + item['name'])
> +
> +list_node('/machine')
>
Andreas Färber Feb. 5, 2014, 5:51 p.m. UTC | #2
Am 05.02.2014 18:48, schrieb Paolo Bonzini:
> Il 05/02/2014 18:35, Andreas Färber ha scritto:
>> Functionally it is a recursive qom-list with qom-get per non-child<>
>> property. Some failures needed to be handled, such as trying to read a
>> pointer property, which is not representable in QMP. Those print a
>> literal "<EXCEPTION>".
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> I don't think it's a modern equivalent of anything.  The two are just
> different.
> 
> "info qtree" may be focused the old concept of buses, but those buses
> aren't going anywhere anytime soon.  "info qtree" may also be focused on
> the old concept of qdev properties (now "static" properties), but that's
> not something that cannot be fixed.
> 
> So, even though I think this script is a very welcome addition, I don't
> think it helps settling the question of what to do with "info qtree".
> IMO there's no good reason to exclude busless devices from "info qtree",
> and it's a bug (of course less severe than crashing, but still a bug)
> that the busless nand device doesn't appear there.

Don't you see that that is unfixable? We may be able to replace info
qtree by an info qom-tree, which does the equivalent of this QMP-based
script, but qtree ues a completely different display hierarchy than QOM.

Andreas

> 
> We can apply all three patches:
> 
> * qdev_try_create for busless devices (or alternatively abort when
> creating the device, qom-test will catch that)
> 
> * add qom-tree
> 
> * nand: Don't use qdev_create() in nand_init()
> 
> and still improve "info qtree" on top.
> 
> Paolo
Paolo Bonzini Feb. 5, 2014, 5:55 p.m. UTC | #3
Il 05/02/2014 18:51, Andreas Färber ha scritto:
>> > So, even though I think this script is a very welcome addition, I don't
>> > think it helps settling the question of what to do with "info qtree".
>> > IMO there's no good reason to exclude busless devices from "info qtree",
>> > and it's a bug (of course less severe than crashing, but still a bug)
>> > that the busless nand device doesn't appear there.
> Don't you see that that is unfixable? We may be able to replace info
> qtree by an info qom-tree, which does the equivalent of this QMP-based
> script, but qtree ues a completely different display hierarchy than QOM.

Yes, that's why it's useful. :)

Busless devices can still be listed, either under their parent or as 
siblings of the system bus.

Paolo
Andreas Färber Feb. 5, 2014, 5:56 p.m. UTC | #4
Am 05.02.2014 18:51, schrieb Andreas Färber:
> Am 05.02.2014 18:48, schrieb Paolo Bonzini:
>> Il 05/02/2014 18:35, Andreas Färber ha scritto:
>>> Functionally it is a recursive qom-list with qom-get per non-child<>
>>> property. Some failures needed to be handled, such as trying to read a
>>> pointer property, which is not representable in QMP. Those print a
>>> literal "<EXCEPTION>".
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>
>> I don't think it's a modern equivalent of anything.  The two are just
>> different.

It is quite obviously the equivalent in that it dumps the values of all
properties like info qtree does for static properties only of
qbus-attached devices only.

Andreas
Paolo Bonzini Feb. 5, 2014, 5:58 p.m. UTC | #5
Il 05/02/2014 18:56, Andreas Färber ha scritto:
>>> >> I don't think it's a modern equivalent of anything.  The two are just
>>> >> different.
> It is quite obviously the equivalent in that it dumps the values of all
> properties like info qtree does for static properties only of
> qbus-attached devices only.

But it hardly shows the bus relationship (except with parent_bus 
properties), which is quite useful to see in a visual manner.

Paolo
Andreas Färber Feb. 5, 2014, 6:01 p.m. UTC | #6
Am 05.02.2014 18:55, schrieb Paolo Bonzini:
> Il 05/02/2014 18:51, Andreas Färber ha scritto:
>>> > So, even though I think this script is a very welcome addition, I
>>> don't
>>> > think it helps settling the question of what to do with "info qtree".
>>> > IMO there's no good reason to exclude busless devices from "info
>>> qtree",
>>> > and it's a bug (of course less severe than crashing, but still a bug)
>>> > that the busless nand device doesn't appear there.
>> Don't you see that that is unfixable? We may be able to replace info
>> qtree by an info qom-tree, which does the equivalent of this QMP-based
>> script, but qtree ues a completely different display hierarchy than QOM.
> 
> Yes, that's why it's useful. :)
> 
> Busless devices can still be listed, either under their parent or as
> siblings of the system bus.

info qtree has been inconclusive for - what? - two years now and no one
has bothered to fix it. If you or Markus care about it, post a patch. :)

The code uses qdev/qbus functions to list those devices so I don't see
an easy way of filtering those devices that qdev/qbus missed and
printing them using the same walking functions. Therefore my saying that
we would need to walk the QOM hierarchy instead, which is
output-incompatible with info qtree and thus a different command.

Not to mention that it will not work for objects that are not devices.

Andreas
Andreas Färber Feb. 5, 2014, 6:08 p.m. UTC | #7
Am 05.02.2014 18:58, schrieb Paolo Bonzini:
> Il 05/02/2014 18:56, Andreas Färber ha scritto:
>>>> >> I don't think it's a modern equivalent of anything.  The two are
>>>> just
>>>> >> different.
>> It is quite obviously the equivalent in that it dumps the values of all
>> properties like info qtree does for static properties only of
>> qbus-attached devices only.
> 
> But it hardly shows the bus relationship (except with parent_bus
> properties), which is quite useful to see in a visual manner.

http://wiki.qemu.org/Features/QOM#TODO

Anthony wants buses to go away completely. So that seems a legacy
concept to me even though they do not seem to be gone tomorrow.

Andreas
Eric Blake Feb. 5, 2014, 6:24 p.m. UTC | #8
On 02/05/2014 10:35 AM, Andreas Färber wrote:
> Functionally it is a recursive qom-list with qom-get per non-child<>
> property. Some failures needed to be handled, such as trying to read a
> pointer property, which is not representable in QMP. Those print a
> literal "<EXCEPTION>".
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  scripts/qmp/qom-tree | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>  create mode 100755 scripts/qmp/qom-tree
> 
> diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
> new file mode 100755
> index 0000000..5fd506a
> --- /dev/null
> +++ b/scripts/qmp/qom-tree
> @@ -0,0 +1,70 @@
> +#!/usr/bin/python
> +##
> +# QEMU Object Model test tools
> +#
> +# Copyright IBM, Corp. 2011
> +# Copyright (c) 2013 SUSE LINUX Products GmbH

It's now 2014, if that affects your copyright.

> +#
> +# Authors:
> +#  Anthony Liguori   <aliguori@us.ibm.com>

Old address.
Andreas Färber Feb. 5, 2014, 6:39 p.m. UTC | #9
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 05.02.2014 19:24, schrieb Eric Blake:
> On 02/05/2014 10:35 AM, Andreas Färber wrote:
>> Functionally it is a recursive qom-list with qom-get per
>> non-child<> property. Some failures needed to be handled, such as
>> trying to read a pointer property, which is not representable in
>> QMP. Those print a literal "<EXCEPTION>".
>> 
>> Signed-off-by: Andreas Färber <afaerber@suse.de> --- 
>> scripts/qmp/qom-tree | 70
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file
>> changed, 70 insertions(+) create mode 100755
>> scripts/qmp/qom-tree
>> 
>> diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree new file
>> mode 100755 index 0000000..5fd506a --- /dev/null +++
>> b/scripts/qmp/qom-tree @@ -0,0 +1,70 @@ +#!/usr/bin/python +## +#
>> QEMU Object Model test tools +# +# Copyright IBM, Corp. 2011 +#
>> Copyright (c) 2013 SUSE LINUX Products GmbH
> 
> It's now 2014, if that affects your copyright.

Thanks, it does date to mid-2013 and I didn't apply functional changes
today:

http://lists.nongnu.org/archive/html/qemu-devel/2013-07/msg05146.html

>> +# +# Authors: +#  Anthony Liguori   <aliguori@us.ibm.com>
> 
> Old address.

It's derived from qom-list and qom-get scripts in the same
subdirectory, which still have that address. Not sure if I should
change such an author entry? Maybe it would be safest to drop email
addresses altogether and just put the names and companies?

FWIW minus .mailmap there's 118 git-grep hits for aliguori@us.ibm.com
in our tree.

Cheers,
Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJS8oVLAAoJEPou0S0+fgE/FjQP/ig86tePat1OqsQEkGaW36SF
Wicq1x9cZwi26fzDjqO0T1t5n4qmNm9rbP0Pawu9hJGmQjeNCay9GxhsC1DMfTu7
0oLaZtI4f6lYTHfe6z3J/kgOUB9LRASLcX+cXGrtKQXLjn6c+CvsjQ+am864/q7i
SPIRaEC0ybLalBkyGQfNsivMjMiDvEvtmQF41z2gDNZggmwIN+BX/eLAs/zcNw3f
3lhRg4mwGGRW3RHtSM6b6XAs+XZACffGSg1l5VQNBXfPR84Vv6krppyuFksuKxNk
KzDGoPqMZwFhX0vk8n5eKbz0ovNJNZwkwK5HiM7JaGm5f0d5jCpoF2QYKJXQ+76h
rmfvLqGMgWgRgDcRSYn2bmdScGoyDNdkNZbs42FEZApD3PG+6/gj7WzcUoTGwe2x
ncahtBX+F+mGi0B3woy8Y9SRtEJD1Es8i3miOI1Xc4OsAyp0EhOT5LTopjWicJRe
XUqWnSA9A3UhDyka7I9mm3J8lmHEU2f+5RZghSDASNgwKKIc6UK8eIC62iwGnh7W
U3AIN+RpC0cKJyf+4EjbY3v+CyBbm9/trSpCAp9i6egtQf173J7oHCY/HqZIcE33
+6Do3pojadJxPAtCH0CY+MzKJfK9h9YjSChoVfLnYoAu9zzeVoGWZg3N1C/3zzMQ
TdeoqrcYDaQbgp1nK4kJ
=y+Zz
-----END PGP SIGNATURE-----
Paolo Bonzini Feb. 7, 2014, 7:13 a.m. UTC | #10
Il 05/02/2014 19:01, Andreas Färber ha scritto:
> Am 05.02.2014 18:55, schrieb Paolo Bonzini:
>> Il 05/02/2014 18:51, Andreas Färber ha scritto:
>>>>> So, even though I think this script is a very welcome addition, I
>>>> don't
>>>>> think it helps settling the question of what to do with "info qtree".
>>>>> IMO there's no good reason to exclude busless devices from "info
>>>> qtree",
>>>>> and it's a bug (of course less severe than crashing, but still a bug)
>>>>> that the busless nand device doesn't appear there.
>>> Don't you see that that is unfixable? We may be able to replace info
>>> qtree by an info qom-tree, which does the equivalent of this QMP-based
>>> script, but qtree ues a completely different display hierarchy than QOM.
>>
>> Yes, that's why it's useful. :)
>>
>> Busless devices can still be listed, either under their parent or as
>> siblings of the system bus.
>
> info qtree has been inconclusive for - what? - two years now and no one
> has bothered to fix it. If you or Markus care about it, post a patch. :)

Is "inconclusive" the right word?  Or is it just that it works for the 
cases that people care about?  There are exactly two cases of busless 
devices in the tree: NAND after Peter's patch, and CPUs.  Wait, on x86 
CPUs do have a bus!

No matter how much I like QOM (I do), I would rather say that the all 
QOM grand plan has been "inconclusive".  99% in-tree uses of QOM are 
just a glorified qdev, buses and all.  You shouldn't be surprised if 
people still care about the "legacy" qdev tree.

> The code uses qdev/qbus functions to list those devices so I don't see
> an easy way of filtering those devices that qdev/qbus missed and
> printing them using the same walking functions.

Make a hash table, walk sysbus and enter devices that have a bus in the 
hash table. Walk /machine and if a device is not in the hash table 
(doesn't have a bus) add it to a list keyed by the QOM parent.  Then 
walk sysbus again, print each device, and after printing a device also 
print the busless part of the QOM subtree rooted at that device.  A bit 
of a hack perhaps, but I suspect it would work for the cases that people 
care about.

> Therefore my saying that
> we would need to walk the QOM hierarchy instead, which is
> output-incompatible with info qtree and thus a different command.

Yes, it is a different command.  Not arguing about that.

> Not to mention that it will not work for objects that are not devices.

Yeah, for the handful of classes that are not in the device hierarchy... ;>

Paolo
Paolo Bonzini Feb. 7, 2014, 10:41 a.m. UTC | #11
Il 05/02/2014 19:08, Andreas Färber ha scritto:
> http://wiki.qemu.org/Features/QOM#TODO
>
> Anthony wants buses to go away completely. So that seems a legacy
> concept to me even though they do not seem to be gone tomorrow.

The way I read that, buses would be replaced by controller devices.

This is unrelated to showing the devices according to the bus topology 
(which still exists in either approach), which is what "info qtree" 
device.  I guess if controller devices existed, you'd add interfaces 
like BusProvider and BusChildEnumerator, or equivalently some "magic" 
properties to do the same, and use that in "info qtree".

Paolo
Andreas Färber Feb. 7, 2014, 11 a.m. UTC | #12
Am 07.02.2014 11:41, schrieb Paolo Bonzini:
> Il 05/02/2014 19:08, Andreas Färber ha scritto:
>> http://wiki.qemu.org/Features/QOM#TODO
>>
>> Anthony wants buses to go away completely. So that seems a legacy
>> concept to me even though they do not seem to be gone tomorrow.
> 
> The way I read that, buses would be replaced by controller devices.
> 
> This is unrelated to showing the devices according to the bus topology
> (which still exists in either approach), which is what "info qtree"
> device.  I guess if controller devices existed, you'd add interfaces
> like BusProvider and BusChildEnumerator, or equivalently some "magic"
> properties to do the same, and use that in "info qtree".

Actually my point was that in Anthony's model you get devices on their
controller device simply by looking at link<foo-device> properties on
the foo controller device. No special bus code is required any more!

Apart from Anthony or someone having to do the actual conversion at some
point, the big unanswered question is how do we share code between PCI
bus hosts - for PCI host bridges we can place code in the QOM base type
(think realizing PCIBus with Bandan's series (which I have been refining
the last few days)), but for PCI-PCI bridges where there's two that's
gonna be slightly more difficult sharing-wise.

Andreas
Andreas Färber Feb. 7, 2014, 11:09 a.m. UTC | #13
Am 07.02.2014 08:13, schrieb Paolo Bonzini:
> Il 05/02/2014 19:01, Andreas Färber ha scritto:
>> Am 05.02.2014 18:55, schrieb Paolo Bonzini:
>>> Il 05/02/2014 18:51, Andreas Färber ha scritto:
>>>>>> So, even though I think this script is a very welcome addition, I
>>>>> don't
>>>>>> think it helps settling the question of what to do with "info qtree".
>>>>>> IMO there's no good reason to exclude busless devices from "info
>>>>> qtree",
>>>>>> and it's a bug (of course less severe than crashing, but still a bug)
>>>>>> that the busless nand device doesn't appear there.
>>>> Don't you see that that is unfixable? We may be able to replace info
>>>> qtree by an info qom-tree, which does the equivalent of this QMP-based
>>>> script, but qtree ues a completely different display hierarchy than
>>>> QOM.
>>>
>>> Yes, that's why it's useful. :)
>>>
>>> Busless devices can still be listed, either under their parent or as
>>> siblings of the system bus.
>>
>> info qtree has been inconclusive for - what? - two years now and no one
>> has bothered to fix it. If you or Markus care about it, post a patch. :)
> 
> Is "inconclusive" the right word?  Or is it just that it works for the
> cases that people care about?  There are exactly two cases of busless
> devices in the tree: NAND after Peter's patch, and CPUs.  Wait, on x86
> CPUs do have a bus!
> 
> No matter how much I like QOM (I do), I would rather say that the all
> QOM grand plan has been "inconclusive".  99% in-tree uses of QOM are
> just a glorified qdev, buses and all.  You shouldn't be surprised if
> people still care about the "legacy" qdev tree.

I am not offended about people caring about legacy devices. I am
offended that people are trying to revert good QOM changes so that they
match their expectations from legacy concepts. I have stated at two KVM
Forums already that qdev is dead. What else do I have to do? Rename
qdev.c to device.c? That's pretty easy to do if it solves these qdev
discussions...

>> The code uses qdev/qbus functions to list those devices so I don't see
>> an easy way of filtering those devices that qdev/qbus missed and
>> printing them using the same walking functions.
> 
> Make a hash table, walk sysbus and enter devices that have a bus in the
> hash table. Walk /machine and if a device is not in the hash table
> (doesn't have a bus) add it to a list keyed by the QOM parent.  Then
> walk sysbus again, print each device, and after printing a device also
> print the busless part of the QOM subtree rooted at that device.  A bit
> of a hack perhaps, but I suspect it would work for the cases that people
> care about.

Don't instruct *me*, post a patch. It does sound like a hack to me. Not
even SysBusDevices are shown in the proper composition in info qtree.

What I have been drafting is an info qom-composition command, which may
show you have the composition differs if you're unwilling to use
qom-list or qom-tree to see for yourself. We could still highlight buses
in that model, if desired. But I'd rather decouple it from random
properties in the new command.

Andreas

>> Therefore my saying that
>> we would need to walk the QOM hierarchy instead, which is
>> output-incompatible with info qtree and thus a different command.
> 
> Yes, it is a different command.  Not arguing about that.
> 
>> Not to mention that it will not work for objects that are not devices.
> 
> Yeah, for the handful of classes that are not in the device hierarchy... ;>
> 
> Paolo
Paolo Bonzini Feb. 7, 2014, 11:10 a.m. UTC | #14
Il 07/02/2014 12:00, Andreas Färber ha scritto:
> Am 07.02.2014 11:41, schrieb Paolo Bonzini:
>> Il 05/02/2014 19:08, Andreas Färber ha scritto:
>>> http://wiki.qemu.org/Features/QOM#TODO
>>>
>>> Anthony wants buses to go away completely. So that seems a legacy
>>> concept to me even though they do not seem to be gone tomorrow.
>>
>> The way I read that, buses would be replaced by controller devices.
>>
>> This is unrelated to showing the devices according to the bus topology
>> (which still exists in either approach), which is what "info qtree"
>> device.  I guess if controller devices existed, you'd add interfaces
>> like BusProvider and BusChildEnumerator, or equivalently some "magic"
>> properties to do the same, and use that in "info qtree".
>
> Actually my point was that in Anthony's model you get devices on their
> controller device simply by looking at link<foo-device> properties on
> the foo controller device. No special bus code is required any more!

Kind of...  There may still be other links (e.g. to the hotplug handler 
in Igor's series), and you may still want to organize your dump so that 
the bus hierarchy is emphasized.  You cannot unconditionally "expand" 
all links, since they can be recursive.

Paolo
Paolo Bonzini Feb. 7, 2014, 11:21 a.m. UTC | #15
Il 07/02/2014 12:09, Andreas Färber ha scritto:
>> No matter how much I like QOM (I do), I would rather say that the all
>> QOM grand plan has been "inconclusive".  99% in-tree uses of QOM are
>> just a glorified qdev, buses and all.  You shouldn't be surprised if
>> people still care about the "legacy" qdev tree.
>
> I am not offended about people caring about legacy devices. I am
> offended that people are trying to revert good QOM changes so that they
> match their expectations from legacy concepts.

I object to calling any change good if it causes a segfault anywhere 
else in the code, even if it may be good from a QOM-only point of view. 
  After a month the change hasn't been reverted yet, so I don't think 
people are trying too hard.  But reverting is always one of the possible 
solutions.  Reapplying a reverted patch is easy.

> I have stated at two KVM Forums already that qdev is dead.

Define "qdev", please.

> qdev.c to device.c? That's pretty easy to do if it solves these qdev
> discussions...

Let's stop talking about theory and let's look at the actual ccode, please.
Andreas Färber Feb. 7, 2014, 12:44 p.m. UTC | #16
Am 07.02.2014 12:21, schrieb Paolo Bonzini:
> Let's stop talking about theory and let's look at the actual ccode, please.

I have posted actual patches, you haven't. I even asked you to myself.

And there's no point continuing discussions about reverts here when
there are actual solutions - by me - on the list. How many more patches
are you asking *me* to supply? I did look at the code, of the nand and
qdev_create() and info qtree code.

Andreas
Andreas Färber Feb. 7, 2014, 12:49 p.m. UTC | #17
Am 07.02.2014 12:21, schrieb Paolo Bonzini:
> Il 07/02/2014 12:09, Andreas Färber ha scritto:
>>> No matter how much I like QOM (I do), I would rather say that the all
>>> QOM grand plan has been "inconclusive".  99% in-tree uses of QOM are
>>> just a glorified qdev, buses and all.  You shouldn't be surprised if
>>> people still care about the "legacy" qdev tree.
>>
>> I am not offended about people caring about legacy devices. I am
>> offended that people are trying to revert good QOM changes so that they
>> match their expectations from legacy concepts.
[...]
>> I have stated at two KVM Forums already that qdev is dead.
> 
> Define "qdev", please.

See my slides. :)

qdev is a framework from Paul Brook with a bus-tree-oriented graph.
QOM by comparison uses an arbitrary composition tree of child<> nodes.

qdev as such no longer exists since late 2011 / early 2012.

Andreas
Paolo Bonzini Feb. 7, 2014, 1:06 p.m. UTC | #18
Il 07/02/2014 13:44, Andreas Färber ha scritto:
> Am 07.02.2014 12:21, schrieb Paolo Bonzini:
>> Let's stop talking about theory and let's look at the actual ccode, please.
>
> I have posted actual patches, you haven't.

I have reviewed those, and said that we can apply all three.  It's 
certainly better than reverting.  That doesn't mean that keeping broken 
code would have been better than reverting.  And let me repeat that 
*reverting a broken patch should always be one of the alternatives*.

But my request to "look at the actual code" was not related to 
contribution of patches.  It referred to _all_ of QEMU device hierarchy. 
Your assertion that "qdev is dead" seems quite an exaggeration; having 
contributed quite a few patches to "kill" qdev-specific mechanisms in 
favor of generic ones, it seems very much alive to me.

Let's look at qdev.  Ask ourselves what useful functionality of Device 
has nothing to do with devices.  Ask ourselves where it clashes with the 
design of Object, and which of the two is better.  Make a design that is 
consistent with what we need, not with a generic 2-year old vision that 
sometimes borders on dogma.  Then we can write code.

This is all totally unrelated from which interesting relationships are 
useful to extract and visualize from the QOM tree---and my point there 
is that both parent-child ("qom-tree") and controller-controlled ("info 
qtree") are useful relationships.

Paolo
Paolo Bonzini Feb. 7, 2014, 1:16 p.m. UTC | #19
Il 07/02/2014 13:49, Andreas Färber ha scritto:
> qdev is a framework from Paul Brook with a bus-tree-oriented graph.
> QOM by comparison uses an arbitrary composition tree of child<> nodes.
>
> qdev as such no longer exists since late 2011 / early 2012.

Let me be blunt: to me, this is a huge, huge delusion.  And I guess this 
difference in views is why we keep talking past each other.

First, having to kill qdev is a dogma at best.  QOM allows more 
expressive relationships than just bus-children, but real hardware does 
have buses.  A bus is not just a n:1 relationship between a controller 
and a controlled device.  Buses, not composition, describe the path that 
signals actually follow while the system runs.  Composition lets you 
understand how a system is built.  Buses lets you run it.

Second, if anyone killed qdev, I didn't notice.  At best, qdev is now 
implemented on top of QOM.  And in fact, the "fake" buses that qdev 
introduced are all still there, so whoever "killed" qdev left quite a 
few traces behind him.

Paolo
Andreas Färber Feb. 7, 2014, 1:48 p.m. UTC | #20
Am 07.02.2014 14:06, schrieb Paolo Bonzini:
> Il 07/02/2014 13:44, Andreas Färber ha scritto:
>> Am 07.02.2014 12:21, schrieb Paolo Bonzini:
>>> Let's stop talking about theory and let's look at the actual ccode,
>>> please.
>>
>> I have posted actual patches, you haven't.
> 
> I have reviewed those, and said that we can apply all three.  It's
> certainly better than reverting.  That doesn't mean that keeping broken
> code would have been better than reverting.

Right, my preferred way would've been a) Peter C. noticing in the first
place or b) Peter C. supplying the one-line NAND fix that I had to write
myself after no one else managed to come up with a real fix. :(

>  And let me repeat that
> *reverting a broken patch should always be one of the alternatives*.

Yes. But it has never been *mandatory* to revert things first before
fixing them. We usually apply incremental fixes referencing the commit
hash that broke things.

> But my request to "look at the actual code" was not related to
> contribution of patches.  It referred to _all_ of QEMU device hierarchy.
> Your assertion that "qdev is dead" seems quite an exaggeration; having
> contributed quite a few patches to "kill" qdev-specific mechanisms in
> favor of generic ones, it seems very much alive to me.

Negative, all the device registration, structs, etc. of qdev apart from
the actual devices' *State is gone, no longer exists. See Anthony's
large commits in Git history if you don't believe me. You do know better
than what you are saying here. I certainly agree that there were a
number of oversights and bugs the two of us and others have been fixing.
So you could argue that QOM was applied to fast with insufficient
testing and thinking-through - but also in this case I will rather fix
the fallout than reverting QOM. ;)

So yes, we still have buses, but yet qdev is a thing of the past. That's
why I have always asked to use device_ rather than qdev_ in new qdev.c
code. Scott openly said he didn't realize we had switched to QOM because
as a device author he was still using qdev_create(), qdev_init() and
other APIs with qdev in their name - that's the battle I'm fighting:
getting qdev out of people's heads and stopping to think the old way
just because it's been like that for a long time.

However, based on Anthony's vision/dogma/whatever I do not consider it
worth to rename qdev_set_parent_bus() to device_set_parent_bus() iff
Anthony wants to get rid of buses. And in that case i wouldn't want to
replace qdev_create() with object_new() + qdev_set_parent_bus() since
that's replacing Satan with Lucifer. ;)

> Let's look at qdev.  Ask ourselves what useful functionality of Device
> has nothing to do with devices.  Ask ourselves where it clashes with the
> design of Object, and which of the two is better.  Make a design that is
> consistent with what we need, not with a generic 2-year old vision that
> sometimes borders on dogma.  Then we can write code.
> 
> This is all totally unrelated from which interesting relationships are
> useful to extract and visualize from the QOM tree---and my point there
> is that both parent-child ("qom-tree") and controller-controlled ("info
> qtree") are useful relationships.

I'm not arguing against that. I have rather been telling you that trying
to *shoehorn* QOM objects into "info qtree" cannot work well.

The claim has been that info qtree *should* include all devices (nand in
particular) and that claim I strongly dispute. And I reject you giving
me instructions for how I should implement "info qtree" for you in your
opinion.

I intend to post patches offering new, alternative displays of the
composition tree (info qom-composition) since apparently there is a new
demand for HMP (I was actually under the impression HMP was deprecated
by QMP ever since we started added QMP-only commands like qom-* and
cpu-add, so this script would've been all to make use of the existing
QMP interface to get property values printed!) and I have been
considering a cpu-add like HMP wrapper around qom-list for property
discovery. But I do not see why I should turn "info qtree" into
something that's incompatible with its very idea and ignores the new
composition. Container objects and SoC objects are examples beyond
non-x86 CPU and NAND that do not fit into "info qtree". We don't
strictly needed buses for new devices any more, we'd then just need code
for -device and device_add to do the Right Thing. And this is affecting
actual patches such as s390x vCPU hot-plug and memory hot-add where we
have been discussing whether and in which form we should introduce an
interim bus to please legacy code and/or how we can make that work
longer-term with the QOM composition model.

Please define what you consider the interesting part of info qtree wrt
controller-controlled you would like to see. My problem with info qtree
is that it's trying to show the whole world in terms of recursive buses,
which is no longer applicable to QOM. Do you want to see all properties?
Then this script here does that. Do you want to see PCI free/occupied
slots on the PHB? Then a composition tree can show that. Do you want to
see which buses are available for bus=? Then a flat list of buses - with
or without children - would do, no bus tree needed, or the composition
tree shows them as children where they could be highlighted as proposed.

Andreas
Paolo Bonzini Feb. 7, 2014, 2:18 p.m. UTC | #21
Il 07/02/2014 14:48, Andreas Färber ha scritto:
>>  And let me repeat that
>> *reverting a broken patch should always be one of the alternatives*.
>
> Yes. But it has never been *mandatory* to revert things first before
> fixing them. We usually apply incremental fixes referencing the commit
> hash that broke things.

Ok, so far so good.  Though I think remembering people about the 
Damocles sword of reverting the patch is fair after 1 month.

> Negative, all the device registration, structs, etc. of qdev apart from
> the actual devices' *State is gone, no longer exists. See Anthony's
> large commits in Git history if you don't believe me. You do know better
> than what you are saying here.

I guess there's a difference in terminology.

You're saying qdev is dead.  I'm saying it is now implemented on top of 
QOM.  All the "meta" stuff in qdev is dead indeed.  But the _concepts_ 
are alive and still being used.  Part of it is because no one bothered 
to fix that, but part of it is because there was nothing to fix. :)

> I certainly agree that there were a
> number of oversights and bugs the two of us and others have been fixing.
> So you could argue that QOM was applied to fast with insufficient
> testing and thinking-through - but also in this case I will rather fix
> the fallout than reverting QOM. ;)

Sure.  At the same time, let's not treat QOM and the QOMified qdev as 
crystallized things.

So, Anthony's plan said "no more buses".  The question is also, will 
Anthony ultimately get rid of buses?  I honestly doubt that, and that's 
unrelated to the decrease of his involvement in the last few months.

This is what I referred to as "design by prophet".  It makes no sense to 
judge people's contributors on the basis of a 2-year old plan whose 
execution so far encountered more than a few hurdles.

Let people come up with their own design, and let their design and code 
speak.

FWIW, qdev also had dynamic properties in the beginning; it was Gerd who 
converted them to static.  Why shouldn't the same thing happen for QOM, 
if the code shows that it is an improvement?  Of course, these 
hypothetical QOM static properties need not (should not, in all 
likelihood) be a copycat imitation of what we have in Device.

> I'm not arguing against that. I have rather been telling you that trying
> to *shoehorn* QOM objects into "info qtree" cannot work well.
>
> The claim has been that info qtree *should* include all devices (nand in
> particular) and that claim I strongly dispute. And I reject you giving
> me instructions for how I should implement "info qtree" for you in your
> opinion.

I didn't say you *should* do that.  I said it's *possible* to do that. 
Is it good or bad?  Honestly, I cannot say because there are so few 
busless devices that the current "info qtree" is pretty good as it 
already is.  I don't care enough, for now.  Later, perhaps, I will.

> I intend to post patches offering new, alternative displays of the
> composition tree (info qom-composition) since apparently there is a new
> demand for HMP

Yes, that's useful!

I'm snipping the rest of the email because right now I cannot put enough 
thought into it.

Paolo
Markus Armbruster Feb. 7, 2014, 3 p.m. UTC | #22
Andreas Färber <afaerber@suse.de> writes:

> Am 07.02.2014 14:06, schrieb Paolo Bonzini:
>> Il 07/02/2014 13:44, Andreas Färber ha scritto:
>>> Am 07.02.2014 12:21, schrieb Paolo Bonzini:
>>>> Let's stop talking about theory and let's look at the actual ccode,
>>>> please.
>>>
>>> I have posted actual patches, you haven't.
>> 
>> I have reviewed those, and said that we can apply all three.  It's
>> certainly better than reverting.  That doesn't mean that keeping broken
>> code would have been better than reverting.
>
> Right, my preferred way would've been a) Peter C. noticing in the first
> place or b) Peter C. supplying the one-line NAND fix that I had to write
> myself after no one else managed to come up with a real fix. :(
>
>>  And let me repeat that
>> *reverting a broken patch should always be one of the alternatives*.
>
> Yes. But it has never been *mandatory* to revert things first before
> fixing them. We usually apply incremental fixes referencing the commit
> hash that broke things.

You're attacking a strawman here.

Nobody has demanded revert first, fix later.  The regression got
reported, possible fixes (including a revert) were discussed, Peter
volunteered a follow-up fix, we accepted his offer, and when he couldn't
complete the job in time, he recommended to fall back to revert for the
time being.

This is as standard operating procedure as it gets.  Except for the part
where you get all excited, I must say.

>> But my request to "look at the actual code" was not related to
>> contribution of patches.  It referred to _all_ of QEMU device hierarchy.
>> Your assertion that "qdev is dead" seems quite an exaggeration; having
>> contributed quite a few patches to "kill" qdev-specific mechanisms in
>> favor of generic ones, it seems very much alive to me.
>
> Negative, all the device registration, structs, etc. of qdev apart from
> the actual devices' *State is gone, no longer exists. See Anthony's
> large commits in Git history if you don't believe me. You do know better
> than what you are saying here. I certainly agree that there were a
> number of oversights and bugs the two of us and others have been fixing.

Much of qdev has been reimplemented on top of QOM.  It's not "gone" in
any sense of the word I'd understand.  Perhaps it'll be gone some day,
but we're not there, yet.

> So you could argue that QOM was applied to fast with insufficient
> testing and thinking-through - but also in this case I will rather fix
> the fallout than reverting QOM. ;)

"Reverting QOM" is an even bigger strawman.

> So yes, we still have buses, but yet qdev is a thing of the past. That's
> why I have always asked to use device_ rather than qdev_ in new qdev.c
> code. Scott openly said he didn't realize we had switched to QOM because
> as a device author he was still using qdev_create(), qdev_init() and
> other APIs with qdev in their name - that's the battle I'm fighting:
> getting qdev out of people's heads and stopping to think the old way
> just because it's been like that for a long time.

It's a good fight, but lashing out at people won't do squat for winning
it.

> However, based on Anthony's vision/dogma/whatever I do not consider it
> worth to rename qdev_set_parent_bus() to device_set_parent_bus() iff
> Anthony wants to get rid of buses. And in that case i wouldn't want to
> replace qdev_create() with object_new() + qdev_set_parent_bus() since
> that's replacing Satan with Lucifer. ;)
>
>> Let's look at qdev.  Ask ourselves what useful functionality of Device
>> has nothing to do with devices.  Ask ourselves where it clashes with the
>> design of Object, and which of the two is better.  Make a design that is
>> consistent with what we need, not with a generic 2-year old vision that
>> sometimes borders on dogma.  Then we can write code.
>> 
>> This is all totally unrelated from which interesting relationships are
>> useful to extract and visualize from the QOM tree---and my point there
>> is that both parent-child ("qom-tree") and controller-controlled ("info
>> qtree") are useful relationships.
>
> I'm not arguing against that. I have rather been telling you that trying
> to *shoehorn* QOM objects into "info qtree" cannot work well.
>
> The claim has been that info qtree *should* include all devices (nand in
> particular) and that claim I strongly dispute. And I reject you giving

Strawman#3.

Whether nand belongs into "info qtree" or not can be debated.  Whether
it's okay that "info qtree" crashes when a nand device is around cannot.

> me instructions for how I should implement "info qtree" for you in your
> opinion.

Please consider taking a break and cool down a bit.  That's what I do
when I catch myself writing such stuff.  Or my friends catch me.

> I intend to post patches offering new, alternative displays of the
> composition tree (info qom-composition) since apparently there is a new
> demand for HMP (I was actually under the impression HMP was deprecated
> by QMP ever since we started added QMP-only commands like qom-* and
> cpu-add, so this script would've been all to make use of the existing
> QMP interface to get property values printed!) and I have been
> considering a cpu-add like HMP wrapper around qom-list for property
> discovery.

A welcome contribution!

Demand for HMP isn't new, and HMP is deprecated only as stable interface
for tools.  We've told people to use QMP so many times that folks
getting the impression we want to deprecate HMP in general is quite
understandable.  It's incorrect all the same.

New functionality for HMP must be also available in QMP, unless it's
something that doesn't make sense in QMP.

HMP commands should be implemented so that the HMP code only deals with
the user interface.  The real work should be done by the same code that
does it for QMP.

[...]

Patch
diff mbox

diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
new file mode 100755
index 0000000..5fd506a
--- /dev/null
+++ b/scripts/qmp/qom-tree
@@ -0,0 +1,70 @@ 
+#!/usr/bin/python
+##
+# QEMU Object Model test tools
+#
+# Copyright IBM, Corp. 2011
+# Copyright (c) 2013 SUSE LINUX Products GmbH
+#
+# Authors:
+#  Anthony Liguori   <aliguori@us.ibm.com>
+#  Andreas Faerber   <afaerber@suse.de>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.  See
+# the COPYING file in the top-level directory.
+##
+
+import sys
+import os
+from qmp import QEMUMonitorProtocol
+
+cmd, args = sys.argv[0], sys.argv[1:]
+socket_path = None
+path = None
+prop = None
+
+def usage():
+    return '''environment variables:
+    QMP_SOCKET=<path | addr:port>
+usage:
+    %s [-h] [-s <QMP socket path | addr:port>] [<path>]
+''' % cmd
+
+def usage_error(error_msg = "unspecified error"):
+    sys.stderr.write('%s\nERROR: %s\n' % (usage(), error_msg))
+    exit(1)
+
+if len(args) > 0:
+    if args[0] == "-h":
+        print usage()
+        exit(0);
+    elif args[0] == "-s":
+        try:
+            socket_path = args[1]
+        except:
+            usage_error("missing argument: QMP socket path or address");
+        args = args[2:]
+
+if not socket_path:
+    if os.environ.has_key('QMP_SOCKET'):
+        socket_path = os.environ['QMP_SOCKET']
+    else:
+        usage_error("no QMP socket path or address given");
+
+srv = QEMUMonitorProtocol(socket_path)
+srv.connect()
+
+def list_node(path):
+    print '%s' % path
+    items = srv.command('qom-list', path=path)
+    for item in items:
+        if not item['type'].startswith('child<'):
+            try:
+                print '  %s: %s (%s)' % (item['name'], srv.command('qom-get', path=path, property=item['name']), item['type'])
+            except:
+                print '  %s: <EXCEPTION> (%s)' % (item['name'], item['type'])
+    print ''
+    for item in items:
+        if item['type'].startswith('child<'):
+            list_node(path + '/' + item['name'])
+
+list_node('/machine')