|
| From: | Greg Bellows |
| Subject: | Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list |
| Date: | Thu, 11 Dec 2014 08:57:46 -0600 |
On Wed, 2014-12-10 at 16:59 -0600, Greg Bellows wrote:
>
>
> On 10 December 2014 at 07:19, Marcel Apfelbaum <address@hidden>
> wrote:
> QEMU has support for options per machine, keeping
> a global list of options is no longer necessary.
>
> Signed-off-by: Marcel Apfelbaum <address@hidden>
> ---
> 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.
Thanks! Copied exactly from the original :)
>
> 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.
I'll correct it if I send another version.
Appreciated.
>
> + 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 <address@hidden>
Thanks,
Marcel
| [Prev in Thread] | Current Thread | [Next in Thread] |