| Message ID | 1427897284-31473-1-git-send-email-marcel@redhat.com |
|---|---|
| State | New |
| Headers | show |
I noticed the typo in subject :) maybe the maintainer that will take it can fix this if no further work is necessary, of course. On 04/01/2015 05:08 PM, Marcel Apfelbaum wrote: > Commit 49d2e64 (machine: remove qemu_machine_opts global list) made > machine machine options specific to machine sub-type, leaving > the qemu_machine_opts desc array empty. Sadly this is the place > qmp_query_command_line_options is looking for supported options. > > As a fix for for 2.3 the machine_qemu_opts are restored only > for the scope of qemu-config, bringing together all machines > properties. We need to find a better fix for 2.4. > > Reported-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> Hi Tony, I'll appreciate if you check that it works now. I checked query-command-line-options and it returns the machine options for now. No further changes should be necessary, Thanks, Marcel > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > --- > > I don't like this approach, but I wouldn't want to loose the > 'options per machine type' feature. We'll find something better for 2.4/ > > util/qemu-config.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 108 insertions(+) > > diff --git a/util/qemu-config.c b/util/qemu-config.c > index f3463df..9e8edda 100644 > --- a/util/qemu-config.c > +++ b/util/qemu-config.c > @@ -6,6 +6,7 @@ > #include "hw/qdev.h" > #include "qapi/error.h" > #include "qmp-commands.h" > +#include "hw/i386/pc.h" > > static QemuOptsList *vm_config_groups[32]; > static QemuOptsList *drive_config_groups[4]; > @@ -148,6 +149,111 @@ 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 = "kvm-type", > + .type = QEMU_OPT_STRING, > + .help = "Specifies the KVM virtualization mode (HV, PR)", > + },{ > + .name = PC_MACHINE_MAX_RAM_BELOW_4G, > + .type = QEMU_OPT_SIZE, > + .help = "maximum ram below the 4G boundary (32bit boundary)", > + },{ > + .name = PC_MACHINE_VMPORT, > + .type = QEMU_OPT_STRING, > + .help = "Enable vmport (pc & q35)", > + },{ > + .name = "iommu", > + .type = QEMU_OPT_BOOL, > + .help = "Set on/off to enable/disable Intel IOMMU (VT-d)", > + },{ > + .name = "secure", > + .type = QEMU_OPT_BOOL, > + .help = "Set on/off to enable/disable the ARM " > + "Security Extensions (TrustZone)", > + },{ > + .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", > + }, > + { /* End of list */ } > + } > +}; > + > CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, > const char *option, > Error **errp) > @@ -162,6 +268,8 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, > info->option = g_strdup(vm_config_groups[i]->name); > if (!strcmp("drive", vm_config_groups[i]->name)) { > info->parameters = get_drive_infolist(); > + } else if (!strcmp("machine", vm_config_groups[i]->name)) { > + info->parameters = query_option_descs(machine_opts.desc); > } else { > info->parameters = > query_option_descs(vm_config_groups[i]->desc); >
On 04/01/2015 08:08 AM, Marcel Apfelbaum wrote: > Commit 49d2e64 (machine: remove qemu_machine_opts global list) made > machine machine options specific to machine sub-type, leaving At the risk of sounding like a machine: s/machine machine/machine/ > the qemu_machine_opts desc array empty. Sadly this is the place > qmp_query_command_line_options is looking for supported options. > > As a fix for for 2.3 the machine_qemu_opts are restored only > for the scope of qemu-config, bringing together all machines s/machines/machines'/ > properties. We need to find a better fix for 2.4. > > Reported-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> > Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> > --- > > I don't like this approach, but I wouldn't want to loose the > 'options per machine type' feature. We'll find something better for 2.4/ > > util/qemu-config.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 108 insertions(+) It's a hack, but it avoids a regression. Reviewed-by: Eric Blake <eblake@redhat.com> I also tested that query-command-line-options on qemu.git + this patch provided a superset of the options exposed by 2:qemu-system-x86-2.1.3-3.fc21.x86_64 as installed on Fedora 21, while without this patch it had no options. Tested-by: Eric Blake <eblake@redhat.com>
On 04/01/2015 09:17 AM, Eric Blake wrote: > On 04/01/2015 08:08 AM, Marcel Apfelbaum wrote: >> Commit 49d2e64 (machine: remove qemu_machine_opts global list) made >> machine machine options specific to machine sub-type, leaving > > At the risk of sounding like a machine: s/machine machine/machine/ > >> the qemu_machine_opts desc array empty. Sadly this is the place >> qmp_query_command_line_options is looking for supported options. >> >> As a fix for for 2.3 the machine_qemu_opts are restored only >> for the scope of qemu-config, bringing together all machines > > s/machines/machines'/ > >> properties. We need to find a better fix for 2.4. As discussed on the other thread, we may want a v2: > I also tested that query-command-line-options on qemu.git + this patch > provided a superset of the options exposed by > 2:qemu-system-x86-2.1.3-3.fc21.x86_64 as installed on Fedora 21, while > without this patch it had no options. The problem is that the superset includes per-machine options, even for machines where they don't apply. I think it's better for 2.3 to be conservative (advertise no less than before, so we don't regress) than to lie (advertising something we don't support is risky).
On 04/01/2015 07:30 PM, Eric Blake wrote: > On 04/01/2015 09:17 AM, Eric Blake wrote: >> On 04/01/2015 08:08 AM, Marcel Apfelbaum wrote: >>> Commit 49d2e64 (machine: remove qemu_machine_opts global list) made >>> machine machine options specific to machine sub-type, leaving >> >> At the risk of sounding like a machine: s/machine machine/machine/ >> >>> the qemu_machine_opts desc array empty. Sadly this is the place >>> qmp_query_command_line_options is looking for supported options. >>> >>> As a fix for for 2.3 the machine_qemu_opts are restored only >>> for the scope of qemu-config, bringing together all machines >> >> s/machines/machines'/ >> >>> properties. We need to find a better fix for 2.4. Hi Eric, Thank you for your help. > > As discussed on the other thread, we may want a v2: Submitted. > > >> I also tested that query-command-line-options on qemu.git + this patch >> provided a superset of the options exposed by >> 2:qemu-system-x86-2.1.3-3.fc21.x86_64 as installed on Fedora 21, while >> without this patch it had no options. > > The problem is that the superset includes per-machine options, even for > machines where they don't apply. I think it's better for 2.3 to be > conservative (advertise no less than before, so we don't regress) than > to lie (advertising something we don't support is risky). I agree, this is the reason we wanted per-machine options, to not lie to higher levels. Thanks, Marcel >
On 04/01/2015 10:13 AM, Marcel Apfelbaum wrote: > I noticed the typo in subject :) maybe the maintainer that will take > it can fix this if no further work is necessary, of course. > > On 04/01/2015 05:08 PM, Marcel Apfelbaum wrote: >> Commit 49d2e64 (machine: remove qemu_machine_opts global list) made >> machine machine options specific to machine sub-type, leaving >> the qemu_machine_opts desc array empty. Sadly this is the place >> qmp_query_command_line_options is looking for supported options. >> >> As a fix for for 2.3 the machine_qemu_opts are restored only >> for the scope of qemu-config, bringing together all machines >> properties. We need to find a better fix for 2.4. >> >> Reported-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> > Hi Tony, > I'll appreciate if you check that it works now. > > I checked query-command-line-options and it returns the machine > options for now. > No further changes should be necessary, > > Thanks, > Marcel Marcel, I installed your patch on my system and it resolved the problem I was seeing. I'm looking forward to seeing how you are going to resolve this per machine. Tony Krowiak > >> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> >> --- >> >> I don't like this approach, but I wouldn't want to loose the >> 'options per machine type' feature. We'll find something better for 2.4/ >> >> util/qemu-config.c | 108 >> +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 108 insertions(+) >> >> diff --git a/util/qemu-config.c b/util/qemu-config.c >> index f3463df..9e8edda 100644 >> --- a/util/qemu-config.c >> +++ b/util/qemu-config.c >> @@ -6,6 +6,7 @@ >> #include "hw/qdev.h" >> #include "qapi/error.h" >> #include "qmp-commands.h" >> +#include "hw/i386/pc.h" >> >> static QemuOptsList *vm_config_groups[32]; >> static QemuOptsList *drive_config_groups[4]; >> @@ -148,6 +149,111 @@ 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 = "kvm-type", >> + .type = QEMU_OPT_STRING, >> + .help = "Specifies the KVM virtualization mode (HV, PR)", >> + },{ >> + .name = PC_MACHINE_MAX_RAM_BELOW_4G, >> + .type = QEMU_OPT_SIZE, >> + .help = "maximum ram below the 4G boundary (32bit >> boundary)", >> + },{ >> + .name = PC_MACHINE_VMPORT, >> + .type = QEMU_OPT_STRING, >> + .help = "Enable vmport (pc & q35)", >> + },{ >> + .name = "iommu", >> + .type = QEMU_OPT_BOOL, >> + .help = "Set on/off to enable/disable Intel IOMMU (VT-d)", >> + },{ >> + .name = "secure", >> + .type = QEMU_OPT_BOOL, >> + .help = "Set on/off to enable/disable the ARM " >> + "Security Extensions (TrustZone)", >> + },{ >> + .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", >> + }, >> + { /* End of list */ } >> + } >> +}; >> + >> CommandLineOptionInfoList *qmp_query_command_line_options(bool >> has_option, >> const char *option, >> Error **errp) >> @@ -162,6 +268,8 @@ CommandLineOptionInfoList >> *qmp_query_command_line_options(bool has_option, >> info->option = g_strdup(vm_config_groups[i]->name); >> if (!strcmp("drive", vm_config_groups[i]->name)) { >> info->parameters = get_drive_infolist(); >> + } else if (!strcmp("machine", vm_config_groups[i]->name)) { >> + info->parameters = >> query_option_descs(machine_opts.desc); >> } else { >> info->parameters = >> query_option_descs(vm_config_groups[i]->desc); >> >
diff --git a/util/qemu-config.c b/util/qemu-config.c index f3463df..9e8edda 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -6,6 +6,7 @@ #include "hw/qdev.h" #include "qapi/error.h" #include "qmp-commands.h" +#include "hw/i386/pc.h" static QemuOptsList *vm_config_groups[32]; static QemuOptsList *drive_config_groups[4]; @@ -148,6 +149,111 @@ 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 = "kvm-type", + .type = QEMU_OPT_STRING, + .help = "Specifies the KVM virtualization mode (HV, PR)", + },{ + .name = PC_MACHINE_MAX_RAM_BELOW_4G, + .type = QEMU_OPT_SIZE, + .help = "maximum ram below the 4G boundary (32bit boundary)", + },{ + .name = PC_MACHINE_VMPORT, + .type = QEMU_OPT_STRING, + .help = "Enable vmport (pc & q35)", + },{ + .name = "iommu", + .type = QEMU_OPT_BOOL, + .help = "Set on/off to enable/disable Intel IOMMU (VT-d)", + },{ + .name = "secure", + .type = QEMU_OPT_BOOL, + .help = "Set on/off to enable/disable the ARM " + "Security Extensions (TrustZone)", + },{ + .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", + }, + { /* End of list */ } + } +}; + CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, const char *option, Error **errp) @@ -162,6 +268,8 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, info->option = g_strdup(vm_config_groups[i]->name); if (!strcmp("drive", vm_config_groups[i]->name)) { info->parameters = get_drive_infolist(); + } else if (!strcmp("machine", vm_config_groups[i]->name)) { + info->parameters = query_option_descs(machine_opts.desc); } else { info->parameters = query_option_descs(vm_config_groups[i]->desc);
Commit 49d2e64 (machine: remove qemu_machine_opts global list) made machine machine options specific to machine sub-type, leaving the qemu_machine_opts desc array empty. Sadly this is the place qmp_query_command_line_options is looking for supported options. As a fix for for 2.3 the machine_qemu_opts are restored only for the scope of qemu-config, bringing together all machines properties. We need to find a better fix for 2.4. Reported-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> --- I don't like this approach, but I wouldn't want to loose the 'options per machine type' feature. We'll find something better for 2.4/ util/qemu-config.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+)