* [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
* [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
* [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 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 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
* 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 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 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 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
* 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 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
* 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
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).