Message ID | 1391621709-15620-1-git-send-email-afaerber@suse.de |
---|---|
State | New |
Headers | show |
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') >
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
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
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
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
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
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
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.
-----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-----
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
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
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
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
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
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.
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
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
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
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
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
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
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. [...]
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')
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