Am 07.05.2014 16:43, schrieb Marcel Apfelbaum:
> Make machine's QemuOpts QOM properties of machine. The properties
> are automatically filled in. This opens the possiblity to create
> opts per machine rather than global.
>
> Signed-off-by: Marcel Apfelbaum <marce...@redhat.com>
> ---
> hw/core/machine.c | 256
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/boards.h | 6 +-
> vl.c | 9 +-
> 3 files changed, 265 insertions(+), 6 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index d3ffef7..fff8317 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -11,6 +11,260 @@
> */
>
> #include "hw/boards.h"
> +#include "qapi/visitor.h"
> +
> +static char *machine_get_accel(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
Contrary to 1/4, you here use the overly verbose "machine_state".
> + return g_strdup(machine_state->accel);
> +}
> +
> +static void machine_set_accel(Object *obj, const char *value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->accel = g_strdup(value);
> +}
> +
> +static bool machine_get_kernel_irqchip(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return machine_state->kernel_irqchip;
> +}
> +
> +static void machine_set_kernel_irqchip(Object *obj, bool value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->kernel_irqchip = value;
> +}
> +
> +static void machine_get_kvm_shadow_mem(Object *obj, Visitor *v,
> + void *opaque, const char *name,
> + Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + int64_t value = machine_state->kvm_shadow_mem;
> +
> + visit_type_int(v, &value, name, errp);
> +}
> +
> +static void machine_set_kvm_shadow_mem(Object *obj, Visitor *v,
> + void *opaque, const char *name,
> + Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + Error *error = NULL;
> + int64_t value;
> +
> + visit_type_int(v, &value, name, &error);
> + if (error) {
> + error_propagate(errp, error);
> + return;
> + }
> +
> + machine_state->kvm_shadow_mem = value;
> +}
> +
> +static char *machine_get_kernel(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return g_strdup(machine_state->kernel_filename);
> +}
> +
> +static void machine_set_kernel(Object *obj, const char *value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->kernel_filename = g_strdup(value);
> +}
> +
> +static char *machine_get_initrd(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return g_strdup(machine_state->initrd_filename);
> +}
> +
> +static void machine_set_initrd(Object *obj, const char *value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->initrd_filename = g_strdup(value);
> +}
> +
> +static char *machine_get_append(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return g_strdup(machine_state->kernel_cmdline);
> +}
> +
> +static void machine_set_append(Object *obj, const char *value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->kernel_cmdline = g_strdup(value);
> +}
> +
> +static char *machine_get_dtb(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return g_strdup(machine_state->dtb);
> +}
> +
> +static void machine_set_dtb(Object *obj, const char *value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->dtb = g_strdup(value);
> +}
> +
> +static char *machine_get_dumpdtb(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return g_strdup(machine_state->dumpdtb);
> +}
> +
> +static void machine_set_dumpdtb(Object *obj, const char *value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->dumpdtb = g_strdup(value);
> +}
> +
> +static void machine_get_phandle_start(Object *obj, Visitor *v,
> + void *opaque, const char *name,
> + Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + int64_t value = machine_state->phandle_start;
> +
> + visit_type_int(v, &value, name, errp);
> +}
> +
> +static void machine_set_phandle_start(Object *obj, Visitor *v,
> + void *opaque, const char *name,
> + Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + Error *error = NULL;
> + int64_t value;
> +
> + visit_type_int(v, &value, name, &error);
> + if (error) {
> + error_propagate(errp, error);
> + return;
> + }
> +
> + machine_state->phandle_start = value;
> +}
> +
> +static char *machine_get_dt_compatible(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return g_strdup(machine_state->dt_compatible);
> +}
> +
> +static void machine_set_dt_compatible(Object *obj, const char *value, Error
> **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->dt_compatible = g_strdup(value);
> +}
> +
> +static bool machine_get_dump_guest_core(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return machine_state->dump_guest_core;
> +}
> +
> +static void machine_set_dump_guest_core(Object *obj, bool value, Error
> **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->dump_guest_core = value;
> +}
> +
> +static bool machine_get_mem_merge(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return machine_state->mem_merge;
> +}
> +
> +static void machine_set_mem_merge(Object *obj, bool value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->mem_merge = value;
> +}
> +
> +static bool machine_get_usb(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return machine_state->usb;
> +}
> +
> +static void machine_set_usb(Object *obj, bool value, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->usb = value;
> +}
> +
> +static char *machine_get_firmware(Object *obj, Error **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + return g_strdup(machine_state->firmware);
> +}
> +
> +static void machine_set_firmware(Object *obj, const char *value, Error
> **errp)
> +{
> + MachineState *machine_state = MACHINE(obj);
> + machine_state->firmware = g_strdup(value);
> +}
Naming aside, this all looks good up to here.
> +
> +static void machine_initfn(Object *obj)
> +{
> + object_property_add_str(obj, "accel",
> + machine_get_accel, machine_set_accel, NULL);
> + object_property_add_bool(obj, "kernel_irqchip",
"kernel-irqchip"
> + machine_get_kernel_irqchip,
> + machine_set_kernel_irqchip,
> + NULL);
> + object_property_add(obj, "kvm_shadow_mem", "int",
"kvm-shadow-mem"
> + machine_get_kvm_shadow_mem,
> + machine_set_kvm_shadow_mem,
> + NULL, NULL, NULL);
> + object_property_add_str(obj, "kernel",
> + machine_get_kernel, machine_set_kernel, NULL);
> + object_property_add_str(obj, "initrd",
> + machine_get_initrd, machine_set_initrd, NULL);
> + object_property_add_str(obj, "append",
> + machine_get_append, machine_set_append, NULL);
> + object_property_add_str(obj, "dtb",
> + machine_get_dtb, machine_set_dtb, NULL);
> + object_property_add_str(obj, "dumpdtb",
> + machine_get_dumpdtb, machine_set_dumpdtb, NULL);
> + object_property_add(obj, "phandle_start", "int",
"phandle-start"
> + machine_get_phandle_start,
> + machine_set_phandle_start,
> + NULL, NULL, NULL);
> + object_property_add_str(obj, "dt_compatible",
"dt-compatible"
QOM conventions require dashes rather than underscores. We faced the
same issue with -cpu legacy options, and the safest solution probably is
to replace patch 3/4 and its usage below with a machine-specific
function that converts from underscore to dash before setting the property.
> + machine_get_dt_compatible,
> + machine_set_dt_compatible,
> + NULL);
> + object_property_add_bool(obj, "dump-guest-core",
> + machine_get_dump_guest_core,
> + machine_set_dump_guest_core,
> + NULL);
> + object_property_add_bool(obj, "mem-merge",
> + machine_get_mem_merge, machine_set_mem_merge,
> NULL);
> + object_property_add_bool(obj, "usb", machine_get_usb, machine_set_usb,
> NULL);
> + object_property_add_str(obj, "firmware",
> + machine_get_firmware, machine_set_firmware,
> NULL);
> +}
> +
> +static void qemu_machine_finalize(Object *obj)
> +{
> + MachineState *machine_state = MACHINE(obj);
> +
> + g_free(machine_state->accel);
> + g_free(machine_state->kernel_filename);
> + g_free(machine_state->initrd_filename);
> + g_free(machine_state->kernel_cmdline);
> + g_free(machine_state->dtb);
> + g_free(machine_state->dumpdtb);
> + g_free(machine_state->dt_compatible);
> + g_free(machine_state->firmware);
> +}
>
> static const TypeInfo machine_info = {
> .name = TYPE_MACHINE,
> @@ -18,6 +272,8 @@ static const TypeInfo machine_info = {
> .abstract = true,
> .class_size = sizeof(MachineClass),
> .instance_size = sizeof(MachineState),
> + .instance_init = machine_initfn,
> + .instance_finalize = qemu_machine_finalize,
Do we need an .instance_post_init to apply -global property defaults?
> };
>
> static void machine_register_types(void)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index eba0574..69664a5 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -114,9 +114,9 @@ struct MachineState {
> const MachineClass *machine;
> ram_addr_t ram_size;
> const char *boot_order;
> - const char *kernel_filename;
> - const char *kernel_cmdline;
> - const char *initrd_filename;
> + char *kernel_filename;
> + char *kernel_cmdline;
> + char *initrd_filename;
> const char *cpu_model;
> };
>
> diff --git a/vl.c b/vl.c
> index 6ec6c1a..e92871c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4216,6 +4216,12 @@ int main(int argc, char **argv, char **envp)
> exit(0);
> }
>
> + machine_opts = qemu_get_machine_opts();
> + if (qemu_opt_foreach(machine_opts, object_set_property, current_machine,
> 1) < 0) {
> + object_unref(OBJECT(current_machine));
> + exit(1);
> + }
> +
> configure_accelerator(machine_class);
>
> if (qtest_chrdev) {
> @@ -4426,9 +4432,6 @@ int main(int argc, char **argv, char **envp)
> current_machine->machine = machine_class;
> current_machine->ram_size = ram_size;
> current_machine->boot_order = boot_order;
> - current_machine->kernel_filename = kernel_filename;
> - current_machine->kernel_cmdline = kernel_cmdline;
> - current_machine->initrd_filename = initrd_filename;
So these three are the only fields set from -machine itself?
> current_machine->cpu_model = cpu_model;
>
> machine_class->init(current_machine);
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
