Re: [Qemu-devel] [PATCH v3 2/2] query-command-line-options: query all the options in qemu-options.hx

[Amos Kong <akong@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 6972 bytes --]

On Tue, Mar 04, 2014 at 03:03:08PM -0700, Eric Blake wrote:

Hi Eric,

> On 03/03/2014 10:51 PM, Amos Kong wrote:
> > vm_config_groups[] only contains part of the options which have
> > argument, and all options which have no argument aren't added to
> > vm_config_groups[]. Current query-command-line-options only
> > checks options from vm_config_groups[], so some options will
> > be lost.
> > 
> > We have some macros in qemu-options.hx to generate a table that
> > contains all the options. This patch tries to query options from
> > the table.
> > 
> > Then we won't lose the legacy options that weren't added to
> > vm_config_groups[] (eg: -vnc, -smbios). The options that have
> > no argument will also be returned (eg: -enable-fips)
> > 
> > Some options that have argument have a NULL desc list, some
> > options don't have argument, and "parameters" is mandatory
> > in the past. So we add a new field "arguments" to present
> 
> Here you call it "arguments", but in the code you call it "argument".
> 
> > if the option takes unspecified arguments.
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  qapi-schema.json   |  8 ++++++--
> >  qemu-options.h     | 18 ++++++++++++++++++
> >  util/qemu-config.c | 35 +++++++++++++++++++++++++++++------
> >  vl.c               | 17 -----------------
> >  4 files changed, 53 insertions(+), 25 deletions(-)
> 
> Umm, did you test this?
> 
> $ printf %s\\n \
>   '{"execute":"qmp_capabilities"}' \
>   '{"execute":"query-command-line-options"}' \
>   '{"execute":"quit"}' \
>  | ./x86_64-softmmu/qemu-system-x86_64 -qmp stdio | grep fips
> $
 
{"return": [{"parameters": [{"name": "timestamp", "type": "boolean"}], "option": "msg"}... {"parameters": [], "option": "enable-fips", "argument": false}, ...

the output of query-command-line-options is one-line, it contains all
the options. I can find 'englab-fips' in the output.

> I was expecting -enable-fips to appear somewhere in the output.
> Something's not right, but I'm not going to figure out what.  Here's
> hoping v4 actually gets it working.
> 
> > 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 05ced9d..0bd8e12 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3944,12 +3944,16 @@
> >  #
> >  # @option: option name
> >  #
> > -# @parameters: an array of @CommandLineParameterInfo
> > +# @parameters: array of @CommandLineParameterInfo, possibly empty
> > +# @argument: @optional present if the @parameters array is empty. If
> > +#            true, then the option takes unspecified arguments, if
> > +#            false, then the option is merely a boolean flag (since 2.0)
> 
> For that matter, even this wasn't true.  In my testing, I see the same
> thing as pre-patch for the -smbios option:
> 
> {"parameters": [], "option": "smbios"}
> 
> but the docs imply that I should now see:
> 
> {"parameters": [], "option": "smbios", "argument": true}

I really got : {"parameters": [], "option": "smbios", "argument": true}

(I was testing with latest qemu upstream + my patches, attached the
output file)

> > +++ b/qemu-options.h
> > @@ -25,6 +25,8 @@
> >   * THE SOFTWARE.
> >   */
> >  
> > +#include "sysemu/arch_init.h"
> > +
> >  #ifndef _QEMU_OPTIONS_H_
> >  #define _QEMU_OPTIONS_H_
> >  
> > @@ -33,4 +35,20 @@ enum {
> >  #include "qemu-options-wrapper.h"
> >  };
> >  
> > +#define HAS_ARG 0x0001
> 
> Defining this non-namespace-friendly macro in a header seems risky.  Can
> you localize its use, by using it only in the .c file that needs it,
> and/or #undef it when done using it?

I will define it in vl.c & qemu-config.c
 
> > +
> > +typedef struct QEMUOption {
> > +    const char *name;
> > +    int flags;
> > +    int index;
> > +    uint32_t arch_mask;
> > +} QEMUOption;

Keep this in qemu-options.h

> > +static const QEMUOption qemu_options[] = {
> > +    { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
> > +#define QEMU_OPTIONS_GENERATE_OPTIONS
> > +#include "qemu-options-wrapper.h"
> > +    { NULL },
> > +};
>
> Sticking a static array in a header is even worse than the v2 - now
> every .c file that includes this .h has its own copy of the variable.
> You really want the .h to just declare the variable as extern, then have
> a single .c file actually implement it.

I will implement it in qemu-config.c when I post V4, thanks
 
> > +    for (i = 0; qemu_options[i].name; i++) {
> > +        if (!has_option || !strcmp(option, qemu_options[i].name)) {
> >              info = g_malloc0(sizeof(*info));
> 
> defaults info->has_argument to false and info->argument to false...
> 
> > -            info->option = g_strdup(vm_config_groups[i]->name);
> > -            if (!strcmp("drive", vm_config_groups[i]->name)) {
> > +            info->option = g_strdup(qemu_options[i].name);
> > +
> > +            int idx = get_group_index(qemu_options[i].name);
> > +
> > +            if (qemu_options[i].flags) {

                     if flags == HAS_ARG == 0x1 ---> True

> > +                info->argument = true;

                    +#  If true, then the option takes unspecified arguments,

> > +            }

                 else { /// default case

                      +# if false, then the option is merely a boolean flag
                 }
> 
> ...changes info->argument to true for options that take unspecified
> arguments (such as -smbios), but with no effect to output unless...
> 
> > +
> > +            if (!strcmp("drive", qemu_options[i].name)) {
> >                  info->parameters = get_drive_infolist();
> > -            } else {
> > +            } else if (idx >= 0) {
> >                  info->parameters =
> > -                    get_param_infolist(vm_config_groups[i]->desc);
> > +                    get_param_infolist(vm_config_groups[idx]->desc);
> > +            }
> > +
> > +            if (!info->parameters) {
> > +                info->has_argument = true;

  //  # @argument: @optional present if the @parameters array is empty.

  If info->parameters is NULL, the array is empty, then @argument presents.

> >              }

                 else {
                     @argument won't present 
                     option has argument and array isn't empty
                 }

> 
> ...this line gets executed.  I guess info->parameters is false for both
> boolean options (where info->argument remains at its default of false)
> and for unspecified arguments (where info->argument was set to true
> above), while omitting the argument output for options that take named
> options?  But while it looks okay in theory, the implementation was
> still broken based on my testing, so I'm not sure what went wrong.

I can only confirm the issue of macro/table definition. Can you help
to re-check if something is wrong in your environment?

-- 
			Amos.

[-- Attachment #2: query-command-line-options.output.txt --]
[-- Type: text/plain, Size: 16513 bytes --]

{"QMP": {"version": {"qemu": {"micro": 50, "minor": 7, "major": 1}, "package": ""}, "capabilities": []}}
{"return": {}}
{"return": [{"parameters": [{"name": "timestamp", "type": "boolean"}], "option": "msg"}, {"parameters": [], "option": "object", "argument": true}, {"parameters": [], "option": "tdf", "argument": false}, {"parameters": [], "option": "no-kvm-irqchip", "argument": false}, {"parameters": [], "option": "no-kvm-pit", "argument": false}, {"parameters": [], "option": "no-kvm-pit-reinjection", "argument": false}, {"parameters": [], "option": "no-kvm", "argument": false}, {"parameters": [], "option": "enable-fips", "argument": false}, {"parameters": [], "option": "qtest-log", "argument": true}, {"parameters": [], "option": "qtest", "argument": true}, {"parameters": [{"name": "file", "type": "string"}, {"name": "events", "type": "string"}], "option": "trace"}, {"parameters": [], "option": "no-user-config", "argument": false}, {"parameters": [], "option": "nodefconfig", "argument": false}, {"parameters": [], "option": "writeconfig", "argument": true}, {"parameters": [], "option": "readconfig", "argument": true}, {"parameters": [{"name": "enable", "type": "boolean"}], "option": "sandbox"}, {"parameters": [], "option": "old-param", "argument": false}, {"parameters": [], "option": "semihosting", "argument": false}, {"parameters": [], "option": "prom-env", "argument": true}, {"parameters": [], "option": "runas", "argument": true}, {"parameters": [], "option": "chroot", "argument": true}, {"parameters": [], "option": "nodefaults", "argument": false}, {"parameters": [], "option": "incoming", "argument": true}, {"parameters": [], "option": "tb-size", "argument": true}, {"parameters": [], "option": "show-cursor", "argument": false}, {"parameters": [], "option": "virtioconsole", "argument": true}, {"parameters": [], "option": "echr", "argument": true}, {"parameters": [], "option": "watchdog-action", "argument": true}, {"parameters": [], "option": "watchdog", "argument": true}, {"parameters": [], "option": "icount", "argument": true}, {"parameters": [{"name": "driftfix", "type": "string"}, {"name": "clock", "type": "string"}, {"name": "base", "type": "string"}], "option": "rtc"}, {"parameters": [], "option": "startdate", "argument": true}, {"parameters": [], "option": "localtime", "argument": false}, {"parameters": [], "option": "clock", "argument": true}, {"parameters": [{"name": "romfile", "type": "string"}, {"name": "bootindex", "type": "number"}], "option": "option-rom"}, {"parameters": [], "option": "daemonize", "argument": false}, {"parameters": [], "option": "loadvm", "argument": true}, {"parameters": [], "option": "no-shutdown", "argument": false}, {"parameters": [], "option": "no-reboot", "argument": false}, {"parameters": [], "option": "xen-attach", "argument": false}, {"parameters": [], "option": "xen-create", "argument": false}, {"parameters": [], "option": "xen-domid", "argument": true}, {"parameters": [], "option": "enable-kvm", "argument": false}, {"parameters": [], "option": "bios", "argument": true}, {"parameters": [], "option": "L", "argument": true}, {"parameters": [], "option": "D", "argument": true}, {"parameters": [], "option": "d", "argument": true}, {"parameters": [], "option": "s", "argument": false}, {"parameters": [], "option": "gdb", "argument": true}, {"parameters": [{"name": "mlock", "type": "boolean"}], "option": "realtime"}, {"parameters": [], "option": "S", "argument": false}, {"parameters": [], "option": "singlestep", "argument": false}, {"parameters": [], "option": "pidfile", "argument": true}, {"parameters": [], "option": "debugcon", "argument": true}, {"parameters": [{"name": "pretty", "type": "boolean"}, {"name": "default", "type": "boolean"}, {"name": "chardev", "type": "string"}, {"name": "mode", "type": "string"}], "option": "mon"}, {"parameters": [], "option": "qmp", "argument": true}, {"parameters": [], "option": "monitor", "argument": true}, {"parameters": [], "option": "parallel", "argument": true}, {"parameters": [], "option": "serial", "argument": true}, {"parameters": [], "option": "dtb", "argument": true}, {"parameters": [], "option": "initrd", "argument": true}, {"parameters": [], "option": "append", "argument": true}, {"parameters": [], "option": "kernel", "argument": true}, {"parameters": [], "option": "bt", "argument": true}, {"parameters": [{"name": "initiator-name", "help": "Initiator iqn name to use when connecting", "type": "string"}, {"name": "header-digest", "help": "HeaderDigest setting. {CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}", "type": "string"}, {"name": "password", "help": "password for CHAP authentication to target", "type": "string"}, {"name": "user", "help": "username for CHAP authentication to target", "type": "string"}], "option": "iscsi"}, {"parameters": [{"name": "chardev", "type": "string"}, {"name": "size", "type": "size"}, {"name": "debug", "type": "number"}, {"name": "name", "type": "string"}, {"name": "signal", "type": "boolean"}, {"name": "mux", "type": "boolean"}, {"name": "rows", "type": "number"}, {"name": "cols", "type": "number"}, {"name": "height", "type": "number"}, {"name": "width", "type": "number"}, {"name": "telnet", "type": "boolean"}, {"name": "delay", "type": "boolean"}, {"name": "server", "type": "boolean"}, {"name": "wait", "type": "boolean"}, {"name": "ipv6", "type": "boolean"}, {"name": "ipv4", "type": "boolean"}, {"name": "to", "type": "number"}, {"name": "localport", "type": "string"}, {"name": "localaddr", "type": "string"}, {"name": "port", "type": "string"}, {"name": "host", "type": "string"}, {"name": "path", "type": "string"}, {"name": "backend", "type": "string"}], "option": "chardev"}, {"parameters": [], "option": "netdev", "argument": true}, {"parameters": [], "option": "net", "argument": true}, {"parameters": [], "option": "smb", "argument": true}, {"parameters": [], "option": "redir", "argument": true}, {"parameters": [], "option": "bootp", "argument": true}, {"parameters": [], "option": "tftp", "argument": true}, {"parameters": [], "option": "smbios", "argument": true}, {"parameters": [], "option": "acpitable", "argument": true}, {"parameters": [], "option": "no-hpet", "argument": false}, {"parameters": [], "option": "no-acpi", "argument": false}, {"parameters": [], "option": "no-fd-bootchk", "argument": false}, {"parameters": [], "option": "rtc-td-hack", "argument": false}, {"parameters": [], "option": "win2k-hack", "argument": false}, {"parameters": [], "option": "vnc", "argument": true}, {"parameters": [], "option": "g", "argument": true}, {"parameters": [], "option": "full-screen", "argument": false}, {"parameters": [], "option": "vga", "argument": true}, {"parameters": [], "option": "rotate", "argument": true}, {"parameters": [], "option": "portrait", "argument": false}, {"parameters": [{"name": "seamless-migration", "type": "boolean"}, {"name": "playback-compression", "type": "boolean"}, {"name": "agent-mouse", "type": "boolean"}, {"name": "streaming-video", "type": "string"}, {"name": "zlib-glz-wan-compression", "type": "string"}, {"name": "jpeg-wan-compression", "type": "string"}, {"name": "image-compression", "type": "string"}, {"name": "plaintext-channel", "type": "string"}, {"name": "tls-channel", "type": "string"}, {"name": "tls-ciphers", "type": "string"}, {"name": "x509-dh-key-file", "type": "string"}, {"name": "x509-cacert-file", "type": "string"}, {"name": "x509-cert-file", "type": "string"}, {"name": "x509-key-password", "type": "string"}, {"name": "x509-key-file", "type": "string"}, {"name": "x509-dir", "type": "string"}, {"name": "sasl", "type": "boolean"}, {"name": "disable-agent-file-xfer", "type": "boolean"}, {"name": "disable-copy-paste", "type": "boolean"}, {"name": "disable-ticketing", "type": "boolean"}, {"name": "password", "type": "string"}, {"name": "ipv6", "type": "boolean"}, {"name": "ipv4", "type": "boolean"}, {"name": "addr", "type": "string"}, {"name": "tls-port", "type": "number"}, {"name": "port", "type": "number"}], "option": "spice"}, {"parameters": [], "option": "sdl", "argument": false}, {"parameters": [], "option": "no-quit", "argument": false}, {"parameters": [], "option": "ctrl-grab", "argument": false}, {"parameters": [], "option": "alt-grab", "argument": false}, {"parameters": [], "option": "no-frame", "argument": false}, {"parameters": [], "option": "curses", "argument": false}, {"parameters": [], "option": "nographic", "argument": false}, {"parameters": [], "option": "display", "argument": true}, {"parameters": [], "option": "usbdevice", "argument": true}, {"parameters": [], "option": "usb", "argument": false}, {"parameters": [], "option": "virtfs_synth", "argument": false}, {"parameters": [{"name": "sock_fd", "type": "number"}, {"name": "socket", "type": "string"}, {"name": "readonly", "type": "boolean"}, {"name": "writeout", "type": "string"}, {"name": "security_model", "type": "string"}, {"name": "mount_tag", "type": "string"}, {"name": "path", "type": "string"}, {"name": "fsdriver", "type": "string"}], "option": "virtfs"}, {"parameters": [{"name": "sock_fd", "type": "number"}, {"name": "socket", "type": "string"}, {"name": "readonly", "type": "boolean"}, {"name": "writeout", "type": "string"}, {"name": "security_model", "type": "string"}, {"name": "path", "type": "string"}, {"name": "fsdriver", "type": "string"}], "option": "fsdev"}, {"parameters": [], "option": "hdachs", "argument": true}, {"parameters": [], "option": "snapshot", "argument": false}, {"parameters": [], "option": "pflash", "argument": true}, {"parameters": [], "option": "sd", "argument": true}, {"parameters": [], "option": "mtdblock", "argument": true}, {"parameters": [{"name": "copy-on-read", "help": "copy read data from backing file into image file", "type": "boolean"}, {"name": "werror", "help": "write error action", "type": "string"}, {"name": "rerror", "help": "read error action", "type": "string"}, {"name": "read-only", "help": "open drive file as read-only", "type": "boolean"}, {"name": "file", "help": "file name", "type": "string"}, {"name": "addr", "help": "pci address (virtio only)", "type": "string"}, {"name": "boot", "help": "(deprecated, ignored)", "type": "boolean"}, {"name": "trans", "help": "chs translation (auto, lba, none)", "type": "string"}, {"name": "secs", "help": "number of sectors (ide disk geometry)", "type": "number"}, {"name": "heads", "help": "number of heads (ide disk geometry)", "type": "number"}, {"name": "cyls", "help": "number of cylinders (ide disk geometry)", "type": "number"}, {"name": "if", "help": "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", "type": "string"}, {"name": "media", "help": "media type (disk, cdrom)", "type": "string"}, {"name": "index", "help": "index number", "type": "number"}, {"name": "unit", "help": "unit number (i.e. lun for scsi)", "type": "number"}, {"name": "bus", "help": "bus number", "type": "number"}, {"name": "throttling.iops-size", "help": "when limiting by iops max size of an I/O in bytes", "type": "number"}, {"name": "throttling.bps-write-max", "help": "total bytes write burst", "type": "number"}, {"name": "throttling.bps-read-max", "help": "total bytes read burst", "type": "number"}, {"name": "throttling.bps-total-max", "help": "total bytes burst", "type": "number"}, {"name": "throttling.iops-write-max", "help": "I/O operations write burst", "type": "number"}, {"name": "throttling.iops-read-max", "help": "I/O operations read burst", "type": "number"}, {"name": "throttling.iops-total-max", "help": "I/O operations burst", "type": "number"}, {"name": "throttling.bps-write", "help": "limit write bytes per second", "type": "number"}, {"name": "throttling.bps-read", "help": "limit read bytes per second", "type": "number"}, {"name": "throttling.bps-total", "help": "limit total bytes per second", "type": "number"}, {"name": "throttling.iops-write", "help": "limit write operations per second", "type": "number"}, {"name": "throttling.iops-read", "help": "limit read operations per second", "type": "number"}, {"name": "throttling.iops-total", "help": "limit total I/O operations per second", "type": "number"}, {"name": "werror", "help": "write error action", "type": "string"}, {"name": "serial", "help": "disk serial number", "type": "string"}, {"name": "format", "help": "disk format (raw, qcow2, ...)", "type": "string"}, {"name": "aio", "help": "host AIO implementation (threads, native)", "type": "string"}, {"name": "cache.no-flush", "help": "ignore any flush requests for the device", "type": "boolean"}, {"name": "cache.direct", "help": "enables use of O_DIRECT (bypass the host page cache)", "type": "boolean"}, {"name": "cache.writeback", "help": "enables writeback mode for any caches", "type": "boolean"}, {"name": "discard", "help": "discard operation (ignore/off, unmap/on)", "type": "string"}, {"name": "snapshot", "help": "enable/disable snapshot mode", "type": "boolean"}], "option": "drive"}, {"parameters": [], "option": "cdrom", "argument": true}, {"parameters": [], "option": "hdd", "argument": true}, {"parameters": [], "option": "hdc", "argument": true}, {"parameters": [], "option": "hdb", "argument": true}, {"parameters": [], "option": "hda", "argument": true}, {"parameters": [], "option": "fdb", "argument": true}, {"parameters": [], "option": "fda", "argument": true}, {"parameters": [], "option": "uuid", "argument": true}, {"parameters": [], "option": "name", "argument": true}, {"parameters": [], "option": "device", "argument": true}, {"parameters": [], "option": "balloon", "argument": true}, {"parameters": [], "option": "soundhw", "argument": true}, {"parameters": [], "option": "audio-help", "argument": false}, {"parameters": [], "option": "k", "argument": true}, {"parameters": [], "option": "mem-prealloc", "argument": false}, {"parameters": [], "option": "mem-path", "argument": true}, {"parameters": [], "option": "m", "argument": true}, {"parameters": [], "option": "boot", "argument": true}, {"parameters": [{"name": "value", "type": "string"}, {"name": "property", "type": "string"}, {"name": "driver", "type": "string"}], "option": "global"}, {"parameters": [], "option": "set", "argument": true}, {"parameters": [{"name": "opaque", "help": "free-form string used to describe fd", "type": "string"}, {"name": "set", "help": "ID of the fd set to add fd to", "type": "number"}, {"name": "fd", "help": "file descriptor of which a duplicate is added to fd set", "type": "number"}], "option": "add-fd"}, {"parameters": [], "option": "numa", "argument": true}, {"parameters": [], "option": "smp", "argument": true}, {"parameters": [], "option": "cpu", "argument": true}, {"parameters": [], "option": "M", "argument": true}, {"parameters": [{"name": "firmware", "help": "firmware image", "type": "string"}, {"name": "usb", "help": "Set on/off to enable/disable usb", "type": "boolean"}, {"name": "mem-merge", "help": "enable/disable memory merge support", "type": "boolean"}, {"name": "dump-guest-core", "help": "Include guest memory in  a core dump", "type": "boolean"}, {"name": "dt_compatible", "help": "Overrides the \"compatible\" property of the dt root node", "type": "string"}, {"name": "phandle_start", "help": "The first phandle ID we may generate dynamically", "type": "number"}, {"name": "dumpdtb", "help": "Dump current dtb to a file and quit", "type": "string"}, {"name": "dtb", "help": "Linux kernel device tree file", "type": "string"}, {"name": "append", "help": "Linux kernel command line", "type": "string"}, {"name": "initrd", "help": "Linux initial ramdisk file", "type": "string"}, {"name": "kernel", "help": "Linux kernel image file", "type": "string"}, {"name": "kvm_shadow_mem", "help": "KVM shadow MMU size", "type": "size"}, {"name": "kernel_irqchip", "help": "use KVM in-kernel irqchip", "type": "boolean"}, {"name": "accel", "help": "accelerator list", "type": "string"}, {"name": "type", "help": "emulated machine", "type": "string"}], "option": "machine"}, {"parameters": [], "option": "version", "argument": false}, {"parameters": [], "option": "help", "argument": false}, {"parameters": [], "option": "h", "argument": false}]}
{"return": {}}
{"timestamp": {"seconds": 1394001181, "microseconds": 494773}, "event": "SHUTDOWN"}
{"timestamp": {"seconds": 1394001181, "microseconds": 495125}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
{"timestamp": {"seconds": 1394001181, "microseconds": 495275}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}