Re: [Qemu-devel] [PATCH qom-next 0/4] qom: HMP commands to replace info qtree

[Peter Crosthwaite]

On Fri, May 9, 2014 at 12:21 AM, Andreas Färber <address@hidden> 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 <address@hidden>
> Cc: Luiz Capitulino <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Markus Armbruster <address@hidden>
> Cc: Eric Blake <address@hidden>
> Cc: Anthony Liguori <address@hidden>
> Cc: Peter Crosthwaite <address@hidden>
>
> 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
>
>