* [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type
@ 2014-12-10 13:19 Marcel Apfelbaum
2014-12-10 13:19 ` [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list Marcel Apfelbaum
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Marcel Apfelbaum @ 2014-12-10 13:19 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, greg.bellows, agraf, mst
This series follows the following upstream discussion:
http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00697.html
QEMU has support for options per machine, keeping
a global list of options is no longer necessary.
While at it, simplified machine_set_property and
added support for -machine,? help.
Marcel Apfelbaum (3):
machine: remove qemu_machine_opts global list
vl.c: simplified machine_set_property
vl.c: add HMP help to machine
hw/core/machine.c | 45 +++++++++++++++++++++
hw/i386/pc.c | 7 ++++
hw/ppc/spapr.c | 3 ++
vl.c | 117 ++++++++++++++++--------------------------------------
4 files changed, 90 insertions(+), 82 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list 2014-12-10 13:19 [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Marcel Apfelbaum @ 2014-12-10 13:19 ` Marcel Apfelbaum 2014-12-10 22:59 ` Greg Bellows 2014-12-10 13:19 ` [Qemu-devel] [PATCH 2/3] vl.c: simplified machine_set_property Marcel Apfelbaum ` (3 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Marcel Apfelbaum @ 2014-12-10 13:19 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, greg.bellows, agraf, mst QEMU has support for options per machine, keeping a global list of options is no longer necessary. Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> --- hw/core/machine.c | 45 +++++++++++++++++++++++++++++ hw/i386/pc.c | 7 +++++ hw/ppc/spapr.c | 3 ++ vl.c | 84 ++++--------------------------------------------------- 4 files changed, 61 insertions(+), 78 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 19d3e3a..a0ae5f9 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -291,48 +291,93 @@ static void machine_initfn(Object *obj) object_property_add_str(obj, "accel", machine_get_accel, machine_set_accel, NULL); + object_property_set_description(obj, "accel", + "Accelerator list", + NULL); object_property_add_bool(obj, "kernel-irqchip", machine_get_kernel_irqchip, machine_set_kernel_irqchip, NULL); + object_property_set_description(obj, "kernel-irqchip", + "Use KVM in-kernel irqchip", + NULL); object_property_add(obj, "kvm-shadow-mem", "int", machine_get_kvm_shadow_mem, machine_set_kvm_shadow_mem, NULL, NULL, NULL); + object_property_set_description(obj, "kvm-shadow-mem", + "KVM shadow MMU size", + NULL); object_property_add_str(obj, "kernel", machine_get_kernel, machine_set_kernel, NULL); + object_property_set_description(obj, "kernel", + "Linux kernel image file", + NULL); object_property_add_str(obj, "initrd", machine_get_initrd, machine_set_initrd, NULL); + object_property_set_description(obj, "initrd", + "Linux initial ramdisk file", + NULL); object_property_add_str(obj, "append", machine_get_append, machine_set_append, NULL); + object_property_set_description(obj, "append", + "Linux kernel command line", + NULL); object_property_add_str(obj, "dtb", machine_get_dtb, machine_set_dtb, NULL); + object_property_set_description(obj, "dtb", + "Linux kernel device tree file", + NULL); object_property_add_str(obj, "dumpdtb", machine_get_dumpdtb, machine_set_dumpdtb, NULL); + object_property_set_description(obj, "dumpdtb", + "Dump current dtb to a file and quit", + NULL); object_property_add(obj, "phandle-start", "int", machine_get_phandle_start, machine_set_phandle_start, NULL, NULL, NULL); + object_property_set_description(obj, "phandle-start", + "The first phandle ID we may generate dynamically", + NULL); object_property_add_str(obj, "dt-compatible", machine_get_dt_compatible, machine_set_dt_compatible, NULL); + object_property_set_description(obj, "dt-compatible", + "Overrides the \"compatible\" property of the dt root node", + NULL); object_property_add_bool(obj, "dump-guest-core", machine_get_dump_guest_core, machine_set_dump_guest_core, NULL); + object_property_set_description(obj, "dump-guest-core", + "Include guest memory in a core dump", + NULL); object_property_add_bool(obj, "mem-merge", machine_get_mem_merge, machine_set_mem_merge, NULL); + object_property_set_description(obj, "mem-merge", + "Enable/disable memory merge support", + NULL); object_property_add_bool(obj, "usb", machine_get_usb, machine_set_usb, NULL); + object_property_set_description(obj, "usb", + "Set on/off to enable/disable usb", + NULL); object_property_add_str(obj, "firmware", machine_get_firmware, machine_set_firmware, NULL); + object_property_set_description(obj, "firmware", + "Firmware image", + NULL); object_property_add_bool(obj, "iommu", machine_get_iommu, machine_set_iommu, NULL); + object_property_set_description(obj, "iommu", + "Set on/off to enable/disable Intel IOMMU (VT-d)", + NULL); /* Register notifier when init is done for sysbus sanity checks */ ms->sysbus_notifier.notify = machine_init_notify; diff --git a/hw/i386/pc.c b/hw/i386/pc.c index f31d55e..01ddd70 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1804,17 +1804,24 @@ static void pc_machine_initfn(Object *obj) object_property_add(obj, PC_MACHINE_MEMHP_REGION_SIZE, "int", pc_machine_get_hotplug_memory_region_size, NULL, NULL, NULL, NULL); + pcms->max_ram_below_4g = 1ULL << 32; /* 4G */ object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size", pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g, NULL, NULL, NULL); + object_property_set_description(obj, PC_MACHINE_MAX_RAM_BELOW_4G, + "Maximum ram below the 4G boundary (32bit boundary)", + NULL); pcms->vmport = ON_OFF_AUTO_AUTO; object_property_add(obj, PC_MACHINE_VMPORT, "OnOffAuto", pc_machine_get_vmport, pc_machine_set_vmport, NULL, NULL, NULL); + object_property_set_description(obj, PC_MACHINE_VMPORT, + "Enable vmport (pc & q35)", + NULL); pcms->enforce_aligned_dimm = true; object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALIGNED_DIMM, diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 30de25d..08401e0 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1655,6 +1655,9 @@ static void spapr_machine_initfn(Object *obj) { object_property_add_str(obj, "kvm-type", spapr_get_kvm_type, spapr_set_kvm_type, NULL); + object_property_set_description(obj, "kvm-type", + "Specifies the KVM virtualization mode (HV, PR)", + NULL); } static void ppc_cpu_do_nmi_on_cpu(void *arg) diff --git a/vl.c b/vl.c index eb89d62..80d30dd 100644 --- a/vl.c +++ b/vl.c @@ -311,84 +311,12 @@ static QemuOptsList qemu_machine_opts = { .merge_lists = true, .head = QTAILQ_HEAD_INITIALIZER(qemu_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)", - }, - { /* End of list */ } + /* + * no elements => accept any + * sanity checking will happen later + * when setting machine properties + */ + { } }, }; -- 1.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list 2014-12-10 13:19 ` [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list Marcel Apfelbaum @ 2014-12-10 22:59 ` Greg Bellows 2014-12-11 5:05 ` Marcel Apfelbaum 0 siblings, 1 reply; 17+ messages in thread From: Greg Bellows @ 2014-12-10 22:59 UTC (permalink / raw) To: Marcel Apfelbaum; +Cc: Peter Maydell, Alexander Graf, QEMU Developers, mst [-- Attachment #1: Type: text/plain, Size: 11810 bytes --] On 10 December 2014 at 07:19, Marcel Apfelbaum <marcel.a@redhat.com> wrote: > QEMU has support for options per machine, keeping > a global list of options is no longer necessary. > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > --- > hw/core/machine.c | 45 +++++++++++++++++++++++++++++ > hw/i386/pc.c | 7 +++++ > hw/ppc/spapr.c | 3 ++ > vl.c | 84 > ++++--------------------------------------------------- > 4 files changed, 61 insertions(+), 78 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 19d3e3a..a0ae5f9 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -291,48 +291,93 @@ static void machine_initfn(Object *obj) > > object_property_add_str(obj, "accel", > machine_get_accel, machine_set_accel, NULL); > + object_property_set_description(obj, "accel", > + "Accelerator list", + NULL); > If this is a common combination of calls would it make sense to create a wrapper routine that just does both of these steps together for readability and convenience? Maybe something like: object_property_add_str_with_desc(obj, "accel", "Accelerator list" machine_get_accel, machine_set_accel, NULL); > object_property_add_bool(obj, "kernel-irqchip", > machine_get_kernel_irqchip, > machine_set_kernel_irqchip, > NULL); > + object_property_set_description(obj, "kernel-irqchip", > + "Use KVM in-kernel irqchip", > + NULL); > object_property_add(obj, "kvm-shadow-mem", "int", > machine_get_kvm_shadow_mem, > machine_set_kvm_shadow_mem, > NULL, NULL, NULL); > + object_property_set_description(obj, "kvm-shadow-mem", > + "KVM shadow MMU size", > + NULL); > object_property_add_str(obj, "kernel", > machine_get_kernel, machine_set_kernel, NULL); > + object_property_set_description(obj, "kernel", > + "Linux kernel image file", > + NULL); > object_property_add_str(obj, "initrd", > machine_get_initrd, machine_set_initrd, NULL); > + object_property_set_description(obj, "initrd", > + "Linux initial ramdisk file", > + NULL); > object_property_add_str(obj, "append", > machine_get_append, machine_set_append, NULL); > + object_property_set_description(obj, "append", > + "Linux kernel command line", > + NULL); > object_property_add_str(obj, "dtb", > machine_get_dtb, machine_set_dtb, NULL); > + object_property_set_description(obj, "dtb", > + "Linux kernel device tree file", > + NULL); > object_property_add_str(obj, "dumpdtb", > machine_get_dumpdtb, machine_set_dumpdtb, > NULL); > + object_property_set_description(obj, "dumpdtb", > + "Dump current dtb to a file and quit", > + NULL); > object_property_add(obj, "phandle-start", "int", > machine_get_phandle_start, > machine_set_phandle_start, > NULL, NULL, NULL); > + object_property_set_description(obj, "phandle-start", > + "The first phandle ID we may generate > dynamically", > + NULL); > object_property_add_str(obj, "dt-compatible", > machine_get_dt_compatible, > machine_set_dt_compatible, > NULL); > + object_property_set_description(obj, "dt-compatible", > + "Overrides the \"compatible\" > property of the dt root node", > + NULL); > object_property_add_bool(obj, "dump-guest-core", > machine_get_dump_guest_core, > machine_set_dump_guest_core, > NULL); > + object_property_set_description(obj, "dump-guest-core", > + "Include guest memory in a core > dump", > FWIW, there is an extra space in the desc string that can probably be corrected. > + NULL); > object_property_add_bool(obj, "mem-merge", > machine_get_mem_merge, > machine_set_mem_merge, NULL); > + object_property_set_description(obj, "mem-merge", > + "Enable/disable memory merge support", > + NULL); > object_property_add_bool(obj, "usb", > machine_get_usb, > machine_set_usb, NULL); > + object_property_set_description(obj, "usb", > + "Set on/off to enable/disable usb", > + NULL); > object_property_add_str(obj, "firmware", > machine_get_firmware, > machine_set_firmware, NULL); > + object_property_set_description(obj, "firmware", > + "Firmware image", > + NULL); > object_property_add_bool(obj, "iommu", > machine_get_iommu, > machine_set_iommu, NULL); > + object_property_set_description(obj, "iommu", > + "Set on/off to enable/disable Intel > IOMMU (VT-d)", > + NULL); > > /* Register notifier when init is done for sysbus sanity checks */ > ms->sysbus_notifier.notify = machine_init_notify; > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index f31d55e..01ddd70 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1804,17 +1804,24 @@ static void pc_machine_initfn(Object *obj) > object_property_add(obj, PC_MACHINE_MEMHP_REGION_SIZE, "int", > pc_machine_get_hotplug_memory_region_size, > NULL, NULL, NULL, NULL); > + > pcms->max_ram_below_4g = 1ULL << 32; /* 4G */ > object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size", > pc_machine_get_max_ram_below_4g, > pc_machine_set_max_ram_below_4g, > NULL, NULL, NULL); > + object_property_set_description(obj, PC_MACHINE_MAX_RAM_BELOW_4G, > + "Maximum ram below the 4G boundary > (32bit boundary)", > + NULL); > > pcms->vmport = ON_OFF_AUTO_AUTO; > object_property_add(obj, PC_MACHINE_VMPORT, "OnOffAuto", > pc_machine_get_vmport, > pc_machine_set_vmport, > NULL, NULL, NULL); > + object_property_set_description(obj, PC_MACHINE_VMPORT, > + "Enable vmport (pc & q35)", > + NULL); > > pcms->enforce_aligned_dimm = true; > object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALIGNED_DIMM, > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 30de25d..08401e0 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1655,6 +1655,9 @@ static void spapr_machine_initfn(Object *obj) > { > object_property_add_str(obj, "kvm-type", > spapr_get_kvm_type, spapr_set_kvm_type, NULL); > + object_property_set_description(obj, "kvm-type", > + "Specifies the KVM virtualization > mode (HV, PR)", > + NULL); > } > > static void ppc_cpu_do_nmi_on_cpu(void *arg) > diff --git a/vl.c b/vl.c > index eb89d62..80d30dd 100644 > --- a/vl.c > +++ b/vl.c > @@ -311,84 +311,12 @@ static QemuOptsList qemu_machine_opts = { > .merge_lists = true, > .head = QTAILQ_HEAD_INITIALIZER(qemu_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)", > - }, > - { /* End of list */ } > + /* > + * no elements => accept any > + * sanity checking will happen later > + * when setting machine properties > + */ > + { } > }, > }; > > -- > 1.9.3 > > Otherwise... Reviewed-by: Greg Bellows <greg.bellows@linaro.org> [-- Attachment #2: Type: text/html, Size: 16919 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list 2014-12-10 22:59 ` Greg Bellows @ 2014-12-11 5:05 ` Marcel Apfelbaum 2014-12-11 14:57 ` Greg Bellows 2014-12-15 15:13 ` Alexander Graf 0 siblings, 2 replies; 17+ messages in thread From: Marcel Apfelbaum @ 2014-12-11 5:05 UTC (permalink / raw) To: Greg Bellows; +Cc: Peter Maydell, Alexander Graf, QEMU Developers, mst On Wed, 2014-12-10 at 16:59 -0600, Greg Bellows wrote: > > > On 10 December 2014 at 07:19, Marcel Apfelbaum <marcel.a@redhat.com> > wrote: > QEMU has support for options per machine, keeping > a global list of options is no longer necessary. > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > --- > hw/core/machine.c | 45 +++++++++++++++++++++++++++++ > hw/i386/pc.c | 7 +++++ > hw/ppc/spapr.c | 3 ++ > vl.c | 84 > ++++--------------------------------------------------- > 4 files changed, 61 insertions(+), 78 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 19d3e3a..a0ae5f9 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -291,48 +291,93 @@ static void machine_initfn(Object *obj) > > object_property_add_str(obj, "accel", > machine_get_accel, > machine_set_accel, NULL); > + object_property_set_description(obj, "accel", > + "Accelerator list", > + NULL); > Hi Greg, Thank you for your review! > > If this is a common combination of calls would it make sense to create > a wrapper routine that just does both of these steps together for > readability and convenience? Maybe something like: > > > object_property_add_str_with_desc(obj, "accel", "Accelerator > list" > machine_get_accel, > machine_set_accel, > NULL); I really thought about it, but one wrapper would not be enough, we need also one for object_property_add_bool, object_property_add and so on. The code needed would be twice as this one and for the moment this series is the only reason for that, maybe is not enough. If you or anyone else things we should still go for it, I'll be glad to. > > object_property_add_bool(obj, "kernel-irqchip", > machine_get_kernel_irqchip, > machine_set_kernel_irqchip, > NULL); > + object_property_set_description(obj, "kernel-irqchip", > + "Use KVM in-kernel > irqchip", > + NULL); > object_property_add(obj, "kvm-shadow-mem", "int", > machine_get_kvm_shadow_mem, > machine_set_kvm_shadow_mem, > NULL, NULL, NULL); > + object_property_set_description(obj, "kvm-shadow-mem", > + "KVM shadow MMU size", > + NULL); > object_property_add_str(obj, "kernel", > machine_get_kernel, > machine_set_kernel, NULL); > + object_property_set_description(obj, "kernel", > + "Linux kernel image > file", > + NULL); > object_property_add_str(obj, "initrd", > machine_get_initrd, > machine_set_initrd, NULL); > + object_property_set_description(obj, "initrd", > + "Linux initial ramdisk > file", > + NULL); > object_property_add_str(obj, "append", > machine_get_append, > machine_set_append, NULL); > + object_property_set_description(obj, "append", > + "Linux kernel command > line", > + NULL); > object_property_add_str(obj, "dtb", > machine_get_dtb, machine_set_dtb, > NULL); > + object_property_set_description(obj, "dtb", > + "Linux kernel device tree > file", > + NULL); > object_property_add_str(obj, "dumpdtb", > machine_get_dumpdtb, > machine_set_dumpdtb, NULL); > + object_property_set_description(obj, "dumpdtb", > + "Dump current dtb to a > file and quit", > + NULL); > object_property_add(obj, "phandle-start", "int", > machine_get_phandle_start, > machine_set_phandle_start, > NULL, NULL, NULL); > + object_property_set_description(obj, "phandle-start", > + "The first phandle ID we > may generate dynamically", > + NULL); > object_property_add_str(obj, "dt-compatible", > machine_get_dt_compatible, > machine_set_dt_compatible, > NULL); > + object_property_set_description(obj, "dt-compatible", > + "Overrides the > \"compatible\" property of the dt root node", > + NULL); > object_property_add_bool(obj, "dump-guest-core", > machine_get_dump_guest_core, > machine_set_dump_guest_core, > NULL); > + object_property_set_description(obj, "dump-guest-core", > + "Include guest memory in > a core dump", > > > FWIW, there is an extra space in the desc string that can probably be > corrected. Thanks! Copied exactly from the original :) I'll correct it if I send another version. > > + NULL); > object_property_add_bool(obj, "mem-merge", > machine_get_mem_merge, > machine_set_mem_merge, NULL); > + object_property_set_description(obj, "mem-merge", > + "Enable/disable memory > merge support", > + NULL); > object_property_add_bool(obj, "usb", > machine_get_usb, > machine_set_usb, NULL); > + object_property_set_description(obj, "usb", > + "Set on/off to > enable/disable usb", > + NULL); > object_property_add_str(obj, "firmware", > machine_get_firmware, > machine_set_firmware, NULL); > + object_property_set_description(obj, "firmware", > + "Firmware image", > + NULL); > object_property_add_bool(obj, "iommu", > machine_get_iommu, > machine_set_iommu, NULL); > + object_property_set_description(obj, "iommu", > + "Set on/off to > enable/disable Intel IOMMU (VT-d)", > + NULL); > > /* Register notifier when init is done for sysbus sanity > checks */ > ms->sysbus_notifier.notify = machine_init_notify; > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index f31d55e..01ddd70 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1804,17 +1804,24 @@ static void pc_machine_initfn(Object > *obj) > object_property_add(obj, PC_MACHINE_MEMHP_REGION_SIZE, > "int", > > pc_machine_get_hotplug_memory_region_size, > NULL, NULL, NULL, NULL); > + > pcms->max_ram_below_4g = 1ULL << 32; /* 4G */ > object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, > "size", > pc_machine_get_max_ram_below_4g, > pc_machine_set_max_ram_below_4g, > NULL, NULL, NULL); > + object_property_set_description(obj, > PC_MACHINE_MAX_RAM_BELOW_4G, > + "Maximum ram below the 4G > boundary (32bit boundary)", > + NULL); > > pcms->vmport = ON_OFF_AUTO_AUTO; > object_property_add(obj, PC_MACHINE_VMPORT, "OnOffAuto", > pc_machine_get_vmport, > pc_machine_set_vmport, > NULL, NULL, NULL); > + object_property_set_description(obj, PC_MACHINE_VMPORT, > + "Enable vmport (pc & > q35)", > + NULL); > > pcms->enforce_aligned_dimm = true; > object_property_add_bool(obj, > PC_MACHINE_ENFORCE_ALIGNED_DIMM, > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 30de25d..08401e0 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1655,6 +1655,9 @@ static void spapr_machine_initfn(Object > *obj) > { > object_property_add_str(obj, "kvm-type", > spapr_get_kvm_type, > spapr_set_kvm_type, NULL); > + object_property_set_description(obj, "kvm-type", > + "Specifies the KVM > virtualization mode (HV, PR)", > + NULL); > } > > static void ppc_cpu_do_nmi_on_cpu(void *arg) > diff --git a/vl.c b/vl.c > index eb89d62..80d30dd 100644 > --- a/vl.c > +++ b/vl.c > @@ -311,84 +311,12 @@ static QemuOptsList qemu_machine_opts = > { > .merge_lists = true, > .head = QTAILQ_HEAD_INITIALIZER(qemu_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)", > - }, > - { /* End of list */ } > + /* > + * no elements => accept any > + * sanity checking will happen later > + * when setting machine properties > + */ > + { } > }, > }; > > -- > 1.9.3 > > > > Otherwise... > > > Reviewed-by: Greg Bellows <greg.bellows@linaro.org> Appreciated. Thanks, Marcel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list 2014-12-11 5:05 ` Marcel Apfelbaum @ 2014-12-11 14:57 ` Greg Bellows 2014-12-15 15:13 ` Alexander Graf 1 sibling, 0 replies; 17+ messages in thread From: Greg Bellows @ 2014-12-11 14:57 UTC (permalink / raw) To: Marcel Apfelbaum; +Cc: Peter Maydell, Alexander Graf, QEMU Developers, mst [-- Attachment #1: Type: text/plain, Size: 15532 bytes --] On 10 December 2014 at 23:05, Marcel Apfelbaum <marcel.a@redhat.com> wrote: > On Wed, 2014-12-10 at 16:59 -0600, Greg Bellows wrote: > > > > > > On 10 December 2014 at 07:19, Marcel Apfelbaum <marcel.a@redhat.com> > > wrote: > > QEMU has support for options per machine, keeping > > a global list of options is no longer necessary. > > > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > > --- > > hw/core/machine.c | 45 +++++++++++++++++++++++++++++ > > hw/i386/pc.c | 7 +++++ > > hw/ppc/spapr.c | 3 ++ > > vl.c | 84 > > ++++--------------------------------------------------- > > 4 files changed, 61 insertions(+), 78 deletions(-) > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c > > index 19d3e3a..a0ae5f9 100644 > > --- a/hw/core/machine.c > > +++ b/hw/core/machine.c > > @@ -291,48 +291,93 @@ static void machine_initfn(Object *obj) > > > > object_property_add_str(obj, "accel", > > machine_get_accel, > > machine_set_accel, NULL); > > + object_property_set_description(obj, "accel", > > + "Accelerator list", > > + NULL); > > > Hi Greg, > Thank you for your review! > > > > > If this is a common combination of calls would it make sense to create > > a wrapper routine that just does both of these steps together for > > readability and convenience? Maybe something like: > > > > > > object_property_add_str_with_desc(obj, "accel", "Accelerator > > list" > > machine_get_accel, > > machine_set_accel, > > NULL); > I really thought about it, but one wrapper would not be enough, > we need also one for object_property_add_bool, object_property_add and so > on. > The code needed would be twice as this one and for the moment this > series is the only reason for that, maybe is not enough. > I'm fine either way, just thought it may cut down on the repetitiveness. > > If you or anyone else things we should still go for it, I'll be glad to. > > > > > object_property_add_bool(obj, "kernel-irqchip", > > machine_get_kernel_irqchip, > > machine_set_kernel_irqchip, > > NULL); > > + object_property_set_description(obj, "kernel-irqchip", > > + "Use KVM in-kernel > > irqchip", > > + NULL); > > object_property_add(obj, "kvm-shadow-mem", "int", > > machine_get_kvm_shadow_mem, > > machine_set_kvm_shadow_mem, > > NULL, NULL, NULL); > > + object_property_set_description(obj, "kvm-shadow-mem", > > + "KVM shadow MMU size", > > + NULL); > > object_property_add_str(obj, "kernel", > > machine_get_kernel, > > machine_set_kernel, NULL); > > + object_property_set_description(obj, "kernel", > > + "Linux kernel image > > file", > > + NULL); > > object_property_add_str(obj, "initrd", > > machine_get_initrd, > > machine_set_initrd, NULL); > > + object_property_set_description(obj, "initrd", > > + "Linux initial ramdisk > > file", > > + NULL); > > object_property_add_str(obj, "append", > > machine_get_append, > > machine_set_append, NULL); > > + object_property_set_description(obj, "append", > > + "Linux kernel command > > line", > > + NULL); > > object_property_add_str(obj, "dtb", > > machine_get_dtb, machine_set_dtb, > > NULL); > > + object_property_set_description(obj, "dtb", > > + "Linux kernel device tree > > file", > > + NULL); > > object_property_add_str(obj, "dumpdtb", > > machine_get_dumpdtb, > > machine_set_dumpdtb, NULL); > > + object_property_set_description(obj, "dumpdtb", > > + "Dump current dtb to a > > file and quit", > > + NULL); > > object_property_add(obj, "phandle-start", "int", > > machine_get_phandle_start, > > machine_set_phandle_start, > > NULL, NULL, NULL); > > + object_property_set_description(obj, "phandle-start", > > + "The first phandle ID we > > may generate dynamically", > > + NULL); > > object_property_add_str(obj, "dt-compatible", > > machine_get_dt_compatible, > > machine_set_dt_compatible, > > NULL); > > + object_property_set_description(obj, "dt-compatible", > > + "Overrides the > > \"compatible\" property of the dt root node", > > + NULL); > > object_property_add_bool(obj, "dump-guest-core", > > machine_get_dump_guest_core, > > machine_set_dump_guest_core, > > NULL); > > + object_property_set_description(obj, "dump-guest-core", > > + "Include guest memory in > > a core dump", > > > > > > FWIW, there is an extra space in the desc string that can probably be > > corrected. > Thanks! Copied exactly from the original :) > I'll correct it if I send another version. > Sounds good. > > > > > + NULL); > > object_property_add_bool(obj, "mem-merge", > > machine_get_mem_merge, > > machine_set_mem_merge, NULL); > > + object_property_set_description(obj, "mem-merge", > > + "Enable/disable memory > > merge support", > > + NULL); > > object_property_add_bool(obj, "usb", > > machine_get_usb, > > machine_set_usb, NULL); > > + object_property_set_description(obj, "usb", > > + "Set on/off to > > enable/disable usb", > > + NULL); > > object_property_add_str(obj, "firmware", > > machine_get_firmware, > > machine_set_firmware, NULL); > > + object_property_set_description(obj, "firmware", > > + "Firmware image", > > + NULL); > > object_property_add_bool(obj, "iommu", > > machine_get_iommu, > > machine_set_iommu, NULL); > > + object_property_set_description(obj, "iommu", > > + "Set on/off to > > enable/disable Intel IOMMU (VT-d)", > > + NULL); > > > > /* Register notifier when init is done for sysbus sanity > > checks */ > > ms->sysbus_notifier.notify = machine_init_notify; > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index f31d55e..01ddd70 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1804,17 +1804,24 @@ static void pc_machine_initfn(Object > > *obj) > > object_property_add(obj, PC_MACHINE_MEMHP_REGION_SIZE, > > "int", > > > > pc_machine_get_hotplug_memory_region_size, > > NULL, NULL, NULL, NULL); > > + > > pcms->max_ram_below_4g = 1ULL << 32; /* 4G */ > > object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, > > "size", > > pc_machine_get_max_ram_below_4g, > > pc_machine_set_max_ram_below_4g, > > NULL, NULL, NULL); > > + object_property_set_description(obj, > > PC_MACHINE_MAX_RAM_BELOW_4G, > > + "Maximum ram below the 4G > > boundary (32bit boundary)", > > + NULL); > > > > pcms->vmport = ON_OFF_AUTO_AUTO; > > object_property_add(obj, PC_MACHINE_VMPORT, "OnOffAuto", > > pc_machine_get_vmport, > > pc_machine_set_vmport, > > NULL, NULL, NULL); > > + object_property_set_description(obj, PC_MACHINE_VMPORT, > > + "Enable vmport (pc & > > q35)", > > + NULL); > > > > pcms->enforce_aligned_dimm = true; > > object_property_add_bool(obj, > > PC_MACHINE_ENFORCE_ALIGNED_DIMM, > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 30de25d..08401e0 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1655,6 +1655,9 @@ static void spapr_machine_initfn(Object > > *obj) > > { > > object_property_add_str(obj, "kvm-type", > > spapr_get_kvm_type, > > spapr_set_kvm_type, NULL); > > + object_property_set_description(obj, "kvm-type", > > + "Specifies the KVM > > virtualization mode (HV, PR)", > > + NULL); > > } > > > > static void ppc_cpu_do_nmi_on_cpu(void *arg) > > diff --git a/vl.c b/vl.c > > index eb89d62..80d30dd 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -311,84 +311,12 @@ static QemuOptsList qemu_machine_opts = > > { > > .merge_lists = true, > > .head = QTAILQ_HEAD_INITIALIZER(qemu_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)", > > - }, > > - { /* End of list */ } > > + /* > > + * no elements => accept any > > + * sanity checking will happen later > > + * when setting machine properties > > + */ > > + { } > > }, > > }; > > > > -- > > 1.9.3 > > > > > > > > Otherwise... > > > > > > Reviewed-by: Greg Bellows <greg.bellows@linaro.org> > Appreciated. > > Thanks, > Marcel > > > > [-- Attachment #2: Type: text/html, Size: 22484 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list 2014-12-11 5:05 ` Marcel Apfelbaum 2014-12-11 14:57 ` Greg Bellows @ 2014-12-15 15:13 ` Alexander Graf 2014-12-15 15:20 ` Paolo Bonzini 1 sibling, 1 reply; 17+ messages in thread From: Alexander Graf @ 2014-12-15 15:13 UTC (permalink / raw) To: Marcel Apfelbaum, Greg Bellows; +Cc: Peter Maydell, QEMU Developers, mst On 11.12.14 06:05, Marcel Apfelbaum wrote: > On Wed, 2014-12-10 at 16:59 -0600, Greg Bellows wrote: >> >> >> On 10 December 2014 at 07:19, Marcel Apfelbaum <marcel.a@redhat.com> >> wrote: >> QEMU has support for options per machine, keeping >> a global list of options is no longer necessary. >> >> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> >> --- >> hw/core/machine.c | 45 +++++++++++++++++++++++++++++ >> hw/i386/pc.c | 7 +++++ >> hw/ppc/spapr.c | 3 ++ >> vl.c | 84 >> ++++--------------------------------------------------- >> 4 files changed, 61 insertions(+), 78 deletions(-) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index 19d3e3a..a0ae5f9 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -291,48 +291,93 @@ static void machine_initfn(Object *obj) >> >> object_property_add_str(obj, "accel", >> machine_get_accel, >> machine_set_accel, NULL); >> + object_property_set_description(obj, "accel", >> + "Accelerator list", >> + NULL); >> > Hi Greg, > Thank you for your review! > >> >> If this is a common combination of calls would it make sense to create >> a wrapper routine that just does both of these steps together for >> readability and convenience? Maybe something like: >> >> >> object_property_add_str_with_desc(obj, "accel", "Accelerator >> list" >> machine_get_accel, >> machine_set_accel, >> NULL); > I really thought about it, but one wrapper would not be enough, > we need also one for object_property_add_bool, object_property_add and so on. > The code needed would be twice as this one and for the moment this > series is the only reason for that, maybe is not enough. > > If you or anyone else things we should still go for it, I'll be glad to. Maybe something like this? #define MACHINE_OPT(name, namestr, desc, type, ...) \ glue(object_property_add_, type)(obj, namestr, \ glue(machine_get, name), glue(machine_set, name), __VA_ARGS__); \ \ object_property_set_description(obj, namestr, desc, NULL); > >> >> object_property_add_bool(obj, "kernel-irqchip", >> machine_get_kernel_irqchip, >> machine_set_kernel_irqchip, >> NULL); >> + object_property_set_description(obj, "kernel-irqchip", >> + "Use KVM in-kernel >> irqchip", >> + NULL); MACHINE_OPT(kvm_shadow_mem, "kvm-shadow-mem", "Use KVM in-kernel irqchip", _bool, NULL); Not sure it's a great improvement though. I certainly wouldn't mind to leave it as this patch does it. Alex ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list 2014-12-15 15:13 ` Alexander Graf @ 2014-12-15 15:20 ` Paolo Bonzini 0 siblings, 0 replies; 17+ messages in thread From: Paolo Bonzini @ 2014-12-15 15:20 UTC (permalink / raw) To: Alexander Graf, Marcel Apfelbaum, Greg Bellows Cc: Peter Maydell, QEMU Developers, mst On 15/12/2014 16:13, Alexander Graf wrote: > MACHINE_OPT(kvm_shadow_mem, "kvm-shadow-mem", "Use KVM in-kernel > irqchip", _bool, NULL); > > Not sure it's a great improvement though. I certainly wouldn't mind to > leave it as this patch does it. I agree. If we want to "hint" that every property should have a description, the QOM functions should have the argument. Paolo ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 2/3] vl.c: simplified machine_set_property 2014-12-10 13:19 [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Marcel Apfelbaum 2014-12-10 13:19 ` [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list Marcel Apfelbaum @ 2014-12-10 13:19 ` Marcel Apfelbaum 2014-12-11 17:49 ` Greg Bellows 2014-12-10 13:19 ` [Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine Marcel Apfelbaum ` (2 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Marcel Apfelbaum @ 2014-12-10 13:19 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, greg.bellows, agraf, mst Refactored the code to re-use object_property_parse. Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> --- vl.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/vl.c b/vl.c index 80d30dd..f9757c3 100644 --- a/vl.c +++ b/vl.c @@ -2580,7 +2580,6 @@ static int machine_set_property(const char *name, const char *value, void *opaque) { Object *obj = OBJECT(opaque); - StringInputVisitor *siv; Error *local_err = NULL; char *c, *qom_name; @@ -2596,9 +2595,7 @@ static int machine_set_property(const char *name, const char *value, } } - siv = string_input_visitor_new(value); - object_property_set(obj, string_input_get_visitor(siv), qom_name, &local_err); - string_input_visitor_cleanup(siv); + object_property_parse(obj, value, qom_name, &local_err); g_free(qom_name); if (local_err) { -- 1.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] vl.c: simplified machine_set_property 2014-12-10 13:19 ` [Qemu-devel] [PATCH 2/3] vl.c: simplified machine_set_property Marcel Apfelbaum @ 2014-12-11 17:49 ` Greg Bellows 0 siblings, 0 replies; 17+ messages in thread From: Greg Bellows @ 2014-12-11 17:49 UTC (permalink / raw) To: Marcel Apfelbaum; +Cc: Peter Maydell, Alexander Graf, QEMU Developers, mst [-- Attachment #1: Type: text/plain, Size: 1109 bytes --] On 10 December 2014 at 07:19, Marcel Apfelbaum <marcel.a@redhat.com> wrote: > Refactored the code to re-use object_property_parse. > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > --- > vl.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/vl.c b/vl.c > index 80d30dd..f9757c3 100644 > --- a/vl.c > +++ b/vl.c > @@ -2580,7 +2580,6 @@ static int machine_set_property(const char *name, > const char *value, > void *opaque) > { > Object *obj = OBJECT(opaque); > - StringInputVisitor *siv; > Error *local_err = NULL; > char *c, *qom_name; > > @@ -2596,9 +2595,7 @@ static int machine_set_property(const char *name, > const char *value, > } > } > > - siv = string_input_visitor_new(value); > - object_property_set(obj, string_input_get_visitor(siv), qom_name, > &local_err); > - string_input_visitor_cleanup(siv); > + object_property_parse(obj, value, qom_name, &local_err); > g_free(qom_name); > > if (local_err) { > -- > 1.9.3 > > Reviewed-by: Greg Bellows <greg.bellows@linaro.org> [-- Attachment #2: Type: text/html, Size: 1745 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine 2014-12-10 13:19 [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Marcel Apfelbaum 2014-12-10 13:19 ` [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list Marcel Apfelbaum 2014-12-10 13:19 ` [Qemu-devel] [PATCH 2/3] vl.c: simplified machine_set_property Marcel Apfelbaum @ 2014-12-10 13:19 ` Marcel Apfelbaum 2014-12-15 15:16 ` Alexander Graf 2014-12-15 15:14 ` [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Alexander Graf 2014-12-15 17:24 ` Peter Maydell 4 siblings, 1 reply; 17+ messages in thread From: Marcel Apfelbaum @ 2014-12-10 13:19 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, greg.bellows, agraf, mst The help is based on the actual machine properties exposing only the relevant options. Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> --- vl.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/vl.c b/vl.c index f9757c3..fd54b44 100644 --- a/vl.c +++ b/vl.c @@ -1469,6 +1469,31 @@ MachineInfoList *qmp_query_machines(Error **errp) return mach_list; } +static int machine_help_func(QemuOpts *opts, MachineState *machine) +{ + ObjectProperty *prop; + + if (!qemu_opt_has_help_opt(opts)) { + return 0; + } + + QTAILQ_FOREACH(prop, &OBJECT(machine)->properties, node) { + if (!prop->set) { + continue; + } + + error_printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name, + prop->name, prop->type); + if (prop->description) { + error_printf(" (%s)\n", prop->description); + } else { + error_printf("\n"); + } + } + + return 1; +} + /***********************************************************/ /* main execution loop */ @@ -3767,6 +3792,9 @@ int main(int argc, char **argv, char **envp) current_machine = MACHINE(object_new(object_class_get_name( OBJECT_CLASS(machine_class)))); + if (machine_help_func(qemu_get_machine_opts(), current_machine)) { + exit(0); + } object_property_add_child(object_get_root(), "machine", OBJECT(current_machine), &error_abort); cpu_exec_init_all(); -- 1.9.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine 2014-12-10 13:19 ` [Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine Marcel Apfelbaum @ 2014-12-15 15:16 ` Alexander Graf 2014-12-15 15:20 ` Daniel P. Berrange 0 siblings, 1 reply; 17+ messages in thread From: Alexander Graf @ 2014-12-15 15:16 UTC (permalink / raw) To: Marcel Apfelbaum, qemu-devel; +Cc: peter.maydell, greg.bellows, mst On 10.12.14 14:19, Marcel Apfelbaum wrote: > The help is based on the actual machine properties > exposing only the relevant options. > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> Can libvirt make use of this or would it need QMP based exposure? Alex ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine 2014-12-15 15:16 ` Alexander Graf @ 2014-12-15 15:20 ` Daniel P. Berrange 2014-12-15 19:24 ` Marcel Apfelbaum 0 siblings, 1 reply; 17+ messages in thread From: Daniel P. Berrange @ 2014-12-15 15:20 UTC (permalink / raw) To: Alexander Graf Cc: peter.maydell, greg.bellows, mst, qemu-devel, Marcel Apfelbaum On Mon, Dec 15, 2014 at 04:16:02PM +0100, Alexander Graf wrote: > > > On 10.12.14 14:19, Marcel Apfelbaum wrote: > > The help is based on the actual machine properties > > exposing only the relevant options. > > > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> > > Can libvirt make use of this or would it need QMP based exposure? Libvirt *never* looks at any -help option anymore. We exclusively use the QMP query-XXXX commands to discover information about QEMU. So if its not already possible to query existance of these new machine properties via QMP, it'll need work. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine 2014-12-15 15:20 ` Daniel P. Berrange @ 2014-12-15 19:24 ` Marcel Apfelbaum 0 siblings, 0 replies; 17+ messages in thread From: Marcel Apfelbaum @ 2014-12-15 19:24 UTC (permalink / raw) To: Daniel P. Berrange, Alexander Graf Cc: Marcel Apfelbaum, peter.maydell, greg.bellows, qemu-devel, mst On 12/15/2014 05:20 PM, Daniel P. Berrange wrote: > On Mon, Dec 15, 2014 at 04:16:02PM +0100, Alexander Graf wrote: >> >> On 10.12.14 14:19, Marcel Apfelbaum wrote: >>> The help is based on the actual machine properties >>> exposing only the relevant options. >>> >>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com> >> Can libvirt make use of this or would it need QMP based exposure? > Libvirt *never* looks at any -help option anymore. We exclusively use > the QMP query-XXXX commands to discover information about QEMU. So > if its not already possible to query existance of these new machine > properties via QMP, it'll need work. Hi Daniel, I thought we already have something generic that creates temp QOM object, query its properties and then deletes it. Who can answer this question? (I didn't get the chance to work with qmp yet.) Thanks, Marcel > > Regards, > Daniel > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type 2014-12-10 13:19 [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Marcel Apfelbaum ` (2 preceding siblings ...) 2014-12-10 13:19 ` [Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine Marcel Apfelbaum @ 2014-12-15 15:14 ` Alexander Graf 2014-12-15 17:24 ` Peter Maydell 4 siblings, 0 replies; 17+ messages in thread From: Alexander Graf @ 2014-12-15 15:14 UTC (permalink / raw) To: Marcel Apfelbaum, qemu-devel; +Cc: peter.maydell, greg.bellows, mst On 10.12.14 14:19, Marcel Apfelbaum wrote: > This series follows the following upstream discussion: > http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00697.html > > QEMU has support for options per machine, keeping > a global list of options is no longer necessary. > > While at it, simplified machine_set_property and > added support for -machine,? help. Reviewed-by: Alexander Graf <agraf@suse.de> Alex ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type 2014-12-10 13:19 [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Marcel Apfelbaum ` (3 preceding siblings ...) 2014-12-15 15:14 ` [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Alexander Graf @ 2014-12-15 17:24 ` Peter Maydell 2014-12-15 17:33 ` Alexander Graf 4 siblings, 1 reply; 17+ messages in thread From: Peter Maydell @ 2014-12-15 17:24 UTC (permalink / raw) To: Marcel Apfelbaum Cc: Alexander Graf, Greg Bellows, QEMU Developers, Michael S. Tsirkin On 10 December 2014 at 13:19, Marcel Apfelbaum <marcel.a@redhat.com> wrote: > This series follows the following upstream discussion: > http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00697.html > > QEMU has support for options per machine, keeping > a global list of options is no longer necessary. > > While at it, simplified machine_set_property and > added support for -machine,? help. > > Marcel Apfelbaum (3): > machine: remove qemu_machine_opts global list > vl.c: simplified machine_set_property > vl.c: add HMP help to machine Are people happy for me to take this series via target-arm.next? Greg's patchset depends on it, so it seems easiest to do that. thanks -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type 2014-12-15 17:24 ` Peter Maydell @ 2014-12-15 17:33 ` Alexander Graf 2014-12-15 17:55 ` Greg Bellows 0 siblings, 1 reply; 17+ messages in thread From: Alexander Graf @ 2014-12-15 17:33 UTC (permalink / raw) To: Peter Maydell, Marcel Apfelbaum Cc: Greg Bellows, QEMU Developers, Michael S. Tsirkin On 15.12.14 18:24, Peter Maydell wrote: > On 10 December 2014 at 13:19, Marcel Apfelbaum <marcel.a@redhat.com> wrote: >> This series follows the following upstream discussion: >> http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00697.html >> >> QEMU has support for options per machine, keeping >> a global list of options is no longer necessary. >> >> While at it, simplified machine_set_property and >> added support for -machine,? help. >> >> Marcel Apfelbaum (3): >> machine: remove qemu_machine_opts global list >> vl.c: simplified machine_set_property >> vl.c: add HMP help to machine > > Are people happy for me to take this series via target-arm.next? > Greg's patchset depends on it, so it seems easiest to do that. Works for me. Alex ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type 2014-12-15 17:33 ` Alexander Graf @ 2014-12-15 17:55 ` Greg Bellows 0 siblings, 0 replies; 17+ messages in thread From: Greg Bellows @ 2014-12-15 17:55 UTC (permalink / raw) To: Alexander Graf Cc: Peter Maydell, Michael S. Tsirkin, QEMU Developers, Marcel Apfelbaum [-- Attachment #1: Type: text/plain, Size: 899 bytes --] Same here. Greg On 15 December 2014 at 11:33, Alexander Graf <agraf@suse.de> wrote: > > > > On 15.12.14 18:24, Peter Maydell wrote: > > On 10 December 2014 at 13:19, Marcel Apfelbaum <marcel.a@redhat.com> > wrote: > >> This series follows the following upstream discussion: > >> http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg00697.html > >> > >> QEMU has support for options per machine, keeping > >> a global list of options is no longer necessary. > >> > >> While at it, simplified machine_set_property and > >> added support for -machine,? help. > >> > >> Marcel Apfelbaum (3): > >> machine: remove qemu_machine_opts global list > >> vl.c: simplified machine_set_property > >> vl.c: add HMP help to machine > > > > Are people happy for me to take this series via target-arm.next? > > Greg's patchset depends on it, so it seems easiest to do that. > > Works for me. > > > Alex > [-- Attachment #2: Type: text/html, Size: 1570 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-12-15 19:24 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-10 13:19 [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Marcel Apfelbaum 2014-12-10 13:19 ` [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list Marcel Apfelbaum 2014-12-10 22:59 ` Greg Bellows 2014-12-11 5:05 ` Marcel Apfelbaum 2014-12-11 14:57 ` Greg Bellows 2014-12-15 15:13 ` Alexander Graf 2014-12-15 15:20 ` Paolo Bonzini 2014-12-10 13:19 ` [Qemu-devel] [PATCH 2/3] vl.c: simplified machine_set_property Marcel Apfelbaum 2014-12-11 17:49 ` Greg Bellows 2014-12-10 13:19 ` [Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine Marcel Apfelbaum 2014-12-15 15:16 ` Alexander Graf 2014-12-15 15:20 ` Daniel P. Berrange 2014-12-15 19:24 ` Marcel Apfelbaum 2014-12-15 15:14 ` [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type Alexander Graf 2014-12-15 17:24 ` Peter Maydell 2014-12-15 17:33 ` Alexander Graf 2014-12-15 17:55 ` Greg Bellows
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).