4:03 p.m.
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(a)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
$
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}
+++ 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?
+
+typedef struct QEMUOption {
+ const char *name;
+ int flags;
+ int index;
+ uint32_t arch_mask;
+} QEMUOption;
+
+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.
+ 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) {
+ info->argument = true;
+ }
...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;
}
...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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org