[PATCH] util/qemu-config: Fix "query-command-line-options" to provide the right values

[PATCH] util/qemu-config: Fix "query-command-line-options" to provide the right values

[Thomas Huth]

The "query-command-line-options" command uses a hand-crafted list
of options that should be returned for the "machine" parameter.
This is pretty much out of sync with reality, for example settings
like "kvm_shadow_mem" or "accel" are not parameters for the machine
anymore. Also, there is no distinction between the targets here, so
e.g. the s390x-specific values like "loadparm" in this list also
show up with the other targets like x86_64.

Let's fix this now by geting rid of the hand-crafted list and by
querying the properties of the machine class instead to assemble
the list.

Fixes: 0a7cf217d8 ("fix regression of qmp_query_command_line_options")
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 util/qemu-config.c | 141 ++++++++++++++++-----------------------------
 1 file changed, 50 insertions(+), 91 deletions(-)

diff --git a/util/qemu-config.c b/util/qemu-config.c
index 433488aa56..097c6c3939 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -8,6 +8,7 @@
 #include "qemu/error-report.h"
 #include "qemu/option.h"
 #include "qemu/config-file.h"
+#include "qom/object.h"
 
 static QemuOptsList *vm_config_groups[48];
 static QemuOptsList *drive_config_groups[5];
@@ -149,97 +150,55 @@ static CommandLineParameterInfoList *get_drive_infolist(void)
     return head;
 }
 
-/* restore machine options that are now machine's properties */
-static QemuOptsList machine_opts = {
-    .merge_lists = true,
-    .head = QTAILQ_HEAD_INITIALIZER(machine_opts.head),
-    .desc = {
-        {
-            .name = "type",
-            .type = QEMU_OPT_STRING,
-            .help = "emulated machine"
-        },{
-            .name = "accel",
-            .type = QEMU_OPT_STRING,
-            .help = "accelerator list",
-        },{
-            .name = "kernel_irqchip",
-            .type = QEMU_OPT_BOOL,
-            .help = "use KVM in-kernel irqchip",
-        },{
-            .name = "kvm_shadow_mem",
-            .type = QEMU_OPT_SIZE,
-            .help = "KVM shadow MMU size",
-        },{
-            .name = "kernel",
-            .type = QEMU_OPT_STRING,
-            .help = "Linux kernel image file",
-        },{
-            .name = "initrd",
-            .type = QEMU_OPT_STRING,
-            .help = "Linux initial ramdisk file",
-        },{
-            .name = "append",
-            .type = QEMU_OPT_STRING,
-            .help = "Linux kernel command line",
-        },{
-            .name = "dtb",
-            .type = QEMU_OPT_STRING,
-            .help = "Linux kernel device tree file",
-        },{
-            .name = "dumpdtb",
-            .type = QEMU_OPT_STRING,
-            .help = "Dump current dtb to a file and quit",
-        },{
-            .name = "phandle_start",
-            .type = QEMU_OPT_NUMBER,
-            .help = "The first phandle ID we may generate dynamically",
-        },{
-            .name = "dt_compatible",
-            .type = QEMU_OPT_STRING,
-            .help = "Overrides the \"compatible\" property of the dt root node",
-        },{
-            .name = "dump-guest-core",
-            .type = QEMU_OPT_BOOL,
-            .help = "Include guest memory in  a core dump",
-        },{
-            .name = "mem-merge",
-            .type = QEMU_OPT_BOOL,
-            .help = "enable/disable memory merge support",
-        },{
-            .name = "usb",
-            .type = QEMU_OPT_BOOL,
-            .help = "Set on/off to enable/disable usb",
-        },{
-            .name = "firmware",
-            .type = QEMU_OPT_STRING,
-            .help = "firmware image",
-        },{
-            .name = "iommu",
-            .type = QEMU_OPT_BOOL,
-            .help = "Set on/off to enable/disable Intel IOMMU (VT-d)",
-        },{
-            .name = "suppress-vmdesc",
-            .type = QEMU_OPT_BOOL,
-            .help = "Set on to disable self-describing migration",
-        },{
-            .name = "aes-key-wrap",
-            .type = QEMU_OPT_BOOL,
-            .help = "enable/disable AES key wrapping using the CPACF wrapping key",
-        },{
-            .name = "dea-key-wrap",
-            .type = QEMU_OPT_BOOL,
-            .help = "enable/disable DEA key wrapping using the CPACF wrapping key",
-        },{
-            .name = "loadparm",
-            .type = QEMU_OPT_STRING,
-            .help = "Up to 8 chars in set of [A-Za-z0-9. ](lower case chars"
-                    " converted to upper case) to pass to machine"
-                    " loader, boot manager, and guest kernel",
-        },
-        { /* End of list */ }
+static CommandLineParameterInfoList *query_machine_properties(void)
+{
+    CommandLineParameterInfoList *param_list = NULL;
+    CommandLineParameterInfo *info;
+    ObjectPropertyIterator iter;
+    ObjectProperty *prop;
+    ObjectClass *oc;
+
+    oc = object_get_class(container_get(object_get_root(), "/machine"));
+    assert(oc);
+
+    /* Add entry for the "type" parameter */
+    info = g_malloc0(sizeof(*info));
+    info->name = g_strdup("type");
+    info->type = COMMAND_LINE_PARAMETER_TYPE_STRING;
+    info->has_help = true;
+    info->help = g_strdup("emulated machine");
+    QAPI_LIST_PREPEND(param_list, info);
+
+    /* Now loop over the properties */
+    object_class_property_iter_init(&iter, oc);
+    while ((prop = object_property_iter_next(&iter))) {
+        if (!prop->set) {
+            continue;
+        }
+
+        info = g_malloc0(sizeof(*info));
+        info->name = g_strdup(prop->name);
+
+        if (g_str_equal(prop->type, "bool")) {
+            info->type = COMMAND_LINE_PARAMETER_TYPE_BOOLEAN;
+        } else if (g_str_equal(prop->type, "int")) {
+            info->type = COMMAND_LINE_PARAMETER_TYPE_NUMBER;
+        } else if (g_str_equal(prop->type, "size")) {
+            info->type = COMMAND_LINE_PARAMETER_TYPE_SIZE;
+        } else {
+            info->type = COMMAND_LINE_PARAMETER_TYPE_STRING;
+        }
+
+        if (prop->description) {
+            info->has_help = true;
+            info->help = g_strdup(prop->description);
+        }
+
+        QAPI_LIST_PREPEND(param_list, info);
     }
-};
+
+    return param_list;
+}
 
 CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option,
                                                           const char *option,
@@ -266,7 +225,7 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option,
     if (!has_option || !strcmp(option, "machine")) {
         info = g_malloc0(sizeof(*info));
         info->option = g_strdup("machine");
-        info->parameters = query_option_descs(machine_opts.desc);
+        info->parameters = query_machine_properties();
         QAPI_LIST_PREPEND(conf_list, info);
     }
 
-- 
2.31.1

Re: [PATCH] util/qemu-config: Fix "query-command-line-options" to provide the right values

[Thomas Huth]

On 09/11/2022 09.44, Thomas Huth wrote:
> The "query-command-line-options" command uses a hand-crafted list
> of options that should be returned for the "machine" parameter.
> This is pretty much out of sync with reality, for example settings
> like "kvm_shadow_mem" or "accel" are not parameters for the machine
> anymore. Also, there is no distinction between the targets here, so
> e.g. the s390x-specific values like "loadparm" in this list also
> show up with the other targets like x86_64.
> 
> Let's fix this now by geting rid of the hand-crafted list and by
> querying the properties of the machine class instead to assemble
> the list.
> 
> Fixes: 0a7cf217d8 ("fix regression of qmp_query_command_line_options")
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
[...]
> +static CommandLineParameterInfoList *query_machine_properties(void)
> +{
> +    CommandLineParameterInfoList *param_list = NULL;
> +    CommandLineParameterInfo *info;
> +    ObjectPropertyIterator iter;
> +    ObjectProperty *prop;
> +    ObjectClass *oc;
> +
> +    oc = object_get_class(container_get(object_get_root(), "/machine"));
> +    assert(oc);

Hmmm, thinking about this again, it likely doesn't work that well either. 
libvirt uses the "none" machine to query the things that QEMU provides, so 
the properties of the individual machines won't show up this way... I guess 
I have to loop over all machine classes to collect all properties from all 
machines or something like that...?

  Thomas