On 10 December 2014 at 23:05, Marcel Apfelbaum <marce...@redhat.com> wrote:
> On Wed, 2014-12-10 at 16:59 -0600, Greg Bellows wrote:
> >
> >
> > On 10 December 2014 at 07:19, Marcel Apfelbaum <marce...@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 <marce...@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.bell...@linaro.org>
> Appreciated.
>
> Thanks,
> Marcel
>
>
>
>
