On Fri, May 9, 2014 at 12:21 AM, Andreas Färber <afaer...@suse.de> wrote: > Hello, > > The main patch of this series is an HMP command "info qom-composition",
qom-composition is a mouthful. Can it be shortened? Although I suggest alternative that obsoletes it anyways further down. > which displays the machine composition tree. This names all devices, > including those missing in "info qtree" for lack of bus. > > To make it more like "info qtree" bus-wise, we could extend it to display > link<> properties as well. > > Properties of devices can be listed with "qom-list", like in QMP. > So this has some ease-of-use challenges. "info qom-comp" doesnt give you copy-pastable full canonical paths, yet you need to have a full path to get the options of a particular device. Some possibilities to make this more usable might include: 1: Full canonical paths in your output rather than indentation. Probably costs readability. 2: More flexibility in the information provided by qom-comp. 3: Tab autocomplete of args to functions like qom-list (probably rather hard) > Based on a suggestion from Paolo, I went on to implement "qom-get" and > "qom-set" for reading and setting them, not basing on their QMP counterparts > but reimplementing them to circumvent the complicated QObject -> string > conversion Hani tried [1]. > > As I replied there, I think we can have both views - with and without > properties. My question is, should they be differently named commands? > Or an argument to the command? Elaborating idea #2 a bit, I think qom-composition as it stands, is a little bit "all or nothing". Should it perhaps accept an argument (much like qom-list) and then it shows the composition tree starting from that node? Once you add a with-properties mode the output for each node, the output is going to be similar to qom-list for each subnode. To that end can all the problems be solved with only a single qom-list command where: 1: We add a recursive mode to qom-list where instead of showing a child link prop we show qom-list output of that child (indentation++). 2: We allow filtering property types for qom-list output (all, none, only links etc). This allows you to do tree views or either entire machine. or a subsystem (or just a single device), with or without property information solving all display-of-information desires. Depending on how sophisticated idea #2 is implemented you could also do you qom-composition-with-just-links output. > Should it go into qdev-monitor.c alongside > the old qdev HMP commands or into hmp.c? Is it okay to not base HMP commands > on their QMP counterparts, or is there any other visitor-based solution? > > Built on top of my pending qom-tree script [2] but shouldn't depend on it. > I still consider that useful, but not resending it since it didn't change. > > Regards, > Andreas > > [1] http://patchwork.ozlabs.org/patch/343136/ > [2] http://patchwork.ozlabs.org/patch/317224/ > > $ ./x86_64-softmmu/qemu-system-x86_64 -S -display none -monitor stdio > QEMU 2.0.50 monitor - type 'help' for more information > (qemu) info qom-composition > /machine (pc-i440fx-2.1-machine) > /peripheral-anon (container) > /peripheral (container) > /unattached (container) > /sysbus (System) > /device[0] (qemu64-x86_64-cpu) > /apic (apic) > /device[1] (kvmvapic) > /device[2] (i440FX) > /device[3] (PIIX3) > /isa.0 (ISA) > /device[4] (isa-i8259) > /device[5] (isa-i8259) > /device[6] (cirrus-vga) > /device[7] (hpet) > /device[8] (mc146818rtc) > /device[9] (isa-pit) > /device[10] (isa-pcspk) > /device[11] (isa-serial) > /device[12] (isa-parallel) > /device[13] (i8042) > /device[14] (vmport) > /device[15] (vmmouse) > /device[16] (port92) > /device[17] (isa-fdc) > /device[18] (e1000) > /device[19] (piix3-ide) > /ide.0 (IDE) > /ide.1 (IDE) > /device[20] (ide-cd) > /device[21] (PIIX4_PM) > /i2c (i2c-bus) Quite a tangential problem (and way out of scope of this series), but should I2C devices be parented to their bus? I know TYPE_BUS is evil, but TYPE_I2C_BUS probably need to stay even after full de-BUSification (probably just reparented to TYPE_DEVICE). Then all the actual I2C devices are children of that i2c-bus DEVICE. I2C bus itself implements non-trivial functionality (much unlike sysbus). > /device[22] (smbus-eeprom) > /device[23] (smbus-eeprom) > /device[24] (smbus-eeprom) > /device[25] (smbus-eeprom) > /device[26] (smbus-eeprom) > /device[27] (smbus-eeprom) > /device[28] (smbus-eeprom) > /device[29] (smbus-eeprom) > /icc-bridge (icc-bridge) > /icc (icc-bus) > /fw_cfg (fw_cfg) > /i440fx (i440FX-pcihost) > /pci.0 (PCI) > /ioapic (ioapic) > (qemu) qom-list > / > (qemu) qom-list / > backend (child<container>) > machine (child<pc-i440fx-2.1-machine>) > type (string) > (qemu) qom-list /machine > i440fx (child<i440FX-pcihost>) > fw_cfg (child<fw_cfg>) > icc-bridge (child<icc-bridge>) > unattached (child<container>) > peripheral (child<container>) > peripheral-anon (child<container>) > type (string) > (qemu) qom-get /machine type > "pc-i440fx-2.1-machine" > (qemu) qom-get /machine/unassigned/device[0] realized > Device '/machine/unassigned/device[0]' not found > (qemu) qom-get /machine/unattached/device[0] realized > true > (qemu) qom-set /machine/unattached/device[0] realized true > (qemu) qom-set /machine/unattached/device[0] realized false > (qemu) > > Cc: Hani Benhabiles <h...@linux.com> > Cc: Luiz Capitulino <lcapitul...@redhat.com> > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: Markus Armbruster <arm...@redhat.com> > Cc: Eric Blake <ebl...@redhat.com> > Cc: Anthony Liguori <anth...@codemonkey.ws> > Cc: Peter Crosthwaite <peter.crosthwa...@xilinx.com> > > Andreas Färber (4): > qom: Implement info qom-composition HMP command > qom: Implement qom-list HMP command > qom: Implement qom-get HMP command > qom: Implement qom-set HMP command These two seem a little out of scope. They don't really add anything to the "lets get rid of qtree" problem. They perhaps just stand in their own right. Regards, Peter > > hmp-commands.hx | 39 +++++++++++++++++++++++++++++ > hmp.c | 67 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > hmp.h | 3 +++ > include/monitor/qdev.h | 1 + > monitor.c | 7 ++++++ > qdev-monitor.c | 35 ++++++++++++++++++++++++++ > 6 files changed, 152 insertions(+) > > -- > 1.8.4.5 > >