Re: [Qemu-devel] [PATCH 00/19] RFC: Reporting QEMU binary capabilities

[Anthony Liguori <anthony@codemonkey.ws>]

On 06/07/2010 09:42 AM, Daniel P. Berrange wrote:
> As everyone here agrees, having management apps parse -help output
> to determine the QEMU capabilities is not at all nice, because it
> is an ill-defined&  fragile data format. Looking more broadly these
> same issues apply to all the other command line options that accept
> a '?' flag for querying capabilities.
>
> We have a very nice structured data format we could be using for
> this: JSON. What's lacking is code to output all this information
> in the JSON format. This patch series can thus be summarized as
> 'JSON for everything'
>    

I'm slightly skeptical that JSON is the right format for this TBH.

> For reference, here is the full list of information libvirt currently
> queries from QEMU:
>
>    * Detection of command line flags (-help parsing)
>
>      -no-kqemu, -no-kvm, -enable-kvm, -no-reboot
>      -name, -uuid, -xen-domid, -domid, -drive
>      -vga, -std-vga, -pcidevice, -mem-path
>      -chardev, -balloon, -device, -rtc, -rtc-td-hack
>      -no-hpet, -no-kvm-pit-reinjection, -tdf
>      -fsdev -sdl
>    

Most of these things can be associated with a config option and/or a 
version.

>    * Detection of parameters (-help parsing)
>
>       -drive format=
>       -drive boot=
>       -drive serial=
>       -drive cache=
>       -cpu cores=, threads=, sockets=
>       -netdev vhost=
>    

Likewise.

>    * Detection of parameter values (-help parsing)
>
>       -drive cache=writethrough|writeback|none
>         vs
>       -drive cache=default|on|off
>    

Same here.

>    * Version number  (-help parsing)
>
>      -netdev  + 0.13.0
>
>      0.9.0  for VNC syntax
>
>      0.10.0 for vnet hdr
>
>    * Parse -M ?  output to get list of machine types + aliases
>    

Yeah, it would be a good to have a qmp command to list supported machines.

>    * Parse -device pci-assign,? to check for 'configfd' parameter
>
>    * Parse -cpu ?  to get list of named CPU types
>    

Would be good to have qmp for this too.

>    * Parse binary name (qemu-system-XXXX) to guess arch of target
>    

Same here.

> This isn't an 100% exhaustive list of things that we would like
> to be able to get access to though. It can likely cover all of
> the following:
>
>   * Version
>
>        QEMU major, minor, micro numbers. KVM version. Package
>        version
>
>   * Devices
>
>        List of device names. Parameter names. Parameter value
>        data types. Allowed strings for enums
>
>   * Arguments
>
>        List of command line arguments. Parameter names. Parameter
>        value data types. Allowed strings for enums
>    

I'm a bit sceptical that this is the right thing to offer.

Generally speaking, libvirt is not going to be capable of handling every 
possible combination of qemu features.  Instead, for 0.12 it will use 
this set of functionality, for 0.13 it might use a different set.

>   * Machine types
>
>        List of names + aliases
>        List of default devices in machine
>
>   * CPU types
>
>        List of names, associated feature flags, all allowed
>        feature flags
>
>   * Target info
>
>        Arch name, wordsize
>
>   * Monitor commands
>
>        List of all monitor commands. Parameter names. Parameter
>        value data types. Allowed strings for enums
>
>   * Block device backends
>
>        List of all block device backends. Parameter names.
>        Parameter value data types. Allowed strings for enums
>
>   * Netdev device backends
>
>        List of all netdev device backends. Parameter names.
>        Parameter value data types. Allowed strings for enums
>
>   * Chardev device backends
>
>        List of all chardev device backends. Parameter names.
>        Parameter value data types. Allowed strings for enums
>
>
> This patch series attempts to satisfy as much of this as is
> feasible with the current QEMU codebase structure. The series
> comprises four stages
>
>   * Some basic infrastructure. Support for JSON pretty printing.
>     Introduction of standardized helpers for converting enums
>     to/from strings. Introduction of an 'enum' data type for
>     QemuOpt to expose a formal list of valid values&  simplify
>     config handling / parsing. Compile time assertion checking
>
>   * Convert driver, netdev&  rtc config options to use the new
>     enum data type for all appropriate args
>
>   * Add new QMP monitor commands exposing data for machine
>     types, devices, cpu types, arch target, argv, config params,
>     and netdev backends.
>
>   * Add a new '-capabilities' command line arg as syntactic
>     sugar for the monitor commands that just report on static
>     info about the QEMU binary.
>
> The main problem encountered with this patch series is the
> split between argv and config parameters. The qemu-config.c
> file provides the information is a good data format, allowing
> programatic access to the list of parameters for each config
> option (eg, the 'cache', 'file', 'aio', etc bits for -drive).
> There are some issues with it though
>
>   - It lacks a huge amount of coverage wrt to the full argv
>     available, even simple things like the '-m' argument
>     don't exist.
>
>   - It is inconsistent in handling parameters. eg it  hands
>     off validation of netdev backends to other code, to allow
>     for handling of different parameters for tap vs user vs
>     socket backends.  For chardev backends though, it directly
>     includes a union of all possible parameters.
>
> Ideally the 'query-argv' command would not be required, but
> unless those problems with the qemu-config are resolved, it
> is the only way to access alot of the information. This is
> sub-optimal because although it lets apps easily determine
> whether an arg like '-smp' is present, it still leaves them
> parsing that args' help string to discover parameters.
>    

A query-argv is a really bad idea IMHO.

Regards,

Anthony Liguori

> This series is lacking a 'query-blockdev' and 'query-chardev'
> commands to report on availability of backends for -blockdev
> and '-chardev' commands.
>
> Finally the existing 'query-commands' output is lacking any
> information about the parameters associated with each command.
> This needs to be addressed somehow.
>
> In summary though, IMHO, this patch series provides a good
> base for providing information about static QEMU binary
> capabilities to applications. It should ensure apps never
> again need to go anywhere near -help, -M ?, -cpu ?, -device ?,
> -soundhw ? and other such ill defined output formats.
>
> NB, I know this patch series will probably clash horribly
> with other patch series recently posted for review, in
> particular Jes' cleanup of vl.c   I will rebase as required
> if any of those get merged.
>
> This series is currently based on GIT master as of:
>
>    commit fd1dc858370d9a9ac7ea2512812c3a152ee6484b
>    Author: Edgar E. Iglesias<edgar.iglesias@gmail.com>
>    Date:   Mon Jun 7 11:54:27 2010 +0200
>
> Regards,
> Daniel
>
>   Makefile.objs                |    2
>   block.c                      |   32 +-
>   block.h                      |   55 ++++
>   cpus.c                       |   78 ++++++
>   cpus.h                       |    1
>   hw/boards.h                  |    2
>   hw/mc146818rtc.c             |    8
>   hw/qdev.c                    |  148 +++++++++++++
>   hw/qdev.h                    |    2
>   monitor.c                    |  167 ++++++++++++++
>   monitor.h                    |    5
>   net.c                        |  173 ++++++++++-----
>   net.h                        |    2
>   qemu-config.c                |  228 +++++++++++++++++++-
>   qemu-config.h                |    2
>   qemu-enum.c                  |   44 +++
>   qemu-enum.h                  |   45 ++++
>   qemu-option.c                |   35 +++
>   qemu-option.h                |   14 +
>   qemu-options.hx              |    9
>   qjson.c                      |   55 ++++
>   qjson.h                      |    1
>   sysemu.h                     |   43 +++
>   target-arm/cpu.h             |    7
>   target-arm/helper.c          |   21 +
>   target-cris/cpu.h            |    7
>   target-cris/translate.c      |   22 +
>   target-i386/cpu.h            |    7
>   target-i386/cpuid.c          |   79 +++++++
>   target-m68k/cpu.h            |    9
>   target-m68k/helper.c         |   23 ++
>   target-mips/cpu.h            |    7
>   target-mips/translate.c      |    4
>   target-mips/translate_init.c |   19 +
>   target-ppc/cpu.h             |    7
>   target-ppc/translate.c       |    4
>   target-ppc/translate_init.c  |   17 +
>   target-sh4/cpu.h             |    8
>   target-sh4/translate.c       |   23 ++
>   target-sparc/cpu.h           |    9
>   target-sparc/helper.c        |   60 +++++
>   verify.h                     |  164 ++++++++++++++
>   vl.c                         |  482 +++++++++++++++++++++++++++++--------------
>   43 files changed, 1869 insertions(+), 261 deletions(-)
>
>
>
>