diff mbox

[RFC,4/5] hw/machine: add qemu machine opts as properties to QemuMachineState

Message ID 1391093245-14277-5-git-send-email-marcel.a@redhat.com
State New
Headers show

Commit Message

Marcel Apfelbaum Jan. 30, 2014, 2:47 p.m. UTC 
It is cleaner to query current_machine's properties than
accessing QemuOpts from the code.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/core/machine.c   | 251 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/boards.h |  15 ++++
 2 files changed, 266 insertions(+)

Comments

Paolo Bonzini Jan. 30, 2014, 4:48 p.m. UTC | #1 
Il 30/01/2014 15:47, Marcel Apfelbaum ha scritto:
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3cd48fe..51bcaba 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -86,6 +86,21 @@ struct QemuMachineState {
>      Object parent;
>      /* public */
>
> +    char *accel;
> +    bool kernel_irqchip;
> +    int kvm_shadow_mem;
> +    char *kernel;
> +    char *initrd;
> +    char *append;

Many of these are in init_args as well.

Perhaps you can include the init_args by value instead of having a 
pointer, and make the setters store into the init_args.  It should be 
fairly easy to use &current_machine->init_args in vl.c instead of the 
current

     QEMUMachineInitArgs args = { .machine = machine,
                                  .ram_size = ram_size,
                                  .boot_order = boot_order,
                                  .kernel_filename = kernel_filename,
                                  .kernel_cmdline = kernel_cmdline,
                                  .initrd_filename = initrd_filename,
                                  .cpu_model = cpu_model };
     machine->init(&args);

Otherwise the series is nice!

Do you think it makes sense to prepend something like "machine::" or 
"machine-" to the class name?

Paolo

> +    char *dtb;
> +    char *dumpdtb;
> +    int phandle_start;
> +    char *dt_compatible;
> +    bool dump_guest_core;
> +    bool mem_merge;
> +    bool usb;
> +    char *firmware;
> +
>      QEMUMachineInitArgs *init_args;
Marcel Apfelbaum Jan. 30, 2014, 5:28 p.m. UTC | #2 
On Thu, 2014-01-30 at 17:48 +0100, Paolo Bonzini wrote:
> Il 30/01/2014 15:47, Marcel Apfelbaum ha scritto:
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 3cd48fe..51bcaba 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -86,6 +86,21 @@ struct QemuMachineState {
> >      Object parent;
> >      /* public */
> >
> > +    char *accel;
> > +    bool kernel_irqchip;
> > +    int kvm_shadow_mem;
> > +    char *kernel;
> > +    char *initrd;
> > +    char *append;

Hi Paolo,

Thanks for the review,
> 
> Many of these are in init_args as well.
> 
> Perhaps you can include the init_args by value instead of having a 
> pointer, and make the setters store into the init_args.  It should be 
> fairly easy to use &current_machine->init_args in vl.c instead of the 
> current
> 
>      QEMUMachineInitArgs args = { .machine = machine,
>                                   .ram_size = ram_size,
>                                   .boot_order = boot_order,
>                                   .kernel_filename = kernel_filename,
>                                   .kernel_cmdline = kernel_cmdline,
>                                   .initrd_filename = initrd_filename,
>                                   .cpu_model = cpu_model };
>      machine->init(&args);
Sure it will be by value or... I plan to replace QEMUMachineInitArgs
with regular QOM properties of QemuMachineState.

> 
> Otherwise the series is nice!
Thanks!

> 
> Do you think it makes sense to prepend something like "machine::" or 
> "machine-" to the class name?
I am not familiar with the QOM classes naming convention,
so I see no reason why not... what are the current guidelines?
prefix-<name>, prefix::<name>, what is preferable?

Thanks,
Marcel

> 
> Paolo
> 
> > +    char *dtb;
> > +    char *dumpdtb;
> > +    int phandle_start;
> > +    char *dt_compatible;
> > +    bool dump_guest_core;
> > +    bool mem_merge;
> > +    bool usb;
> > +    char *firmware;
> > +
> >      QEMUMachineInitArgs *init_args;
>
Paolo Bonzini Jan. 30, 2014, 5:29 p.m. UTC | #3 
Il 30/01/2014 18:28, Marcel Apfelbaum ha scritto:
>> >      QEMUMachineInitArgs args = { .machine = machine,
>> >                                   .ram_size = ram_size,
>> >                                   .boot_order = boot_order,
>> >                                   .kernel_filename = kernel_filename,
>> >                                   .kernel_cmdline = kernel_cmdline,
>> >                                   .initrd_filename = initrd_filename,
>> >                                   .cpu_model = cpu_model };
>> >      machine->init(&args);
> Sure it will be by value or... I plan to replace QEMUMachineInitArgs
> with regular QOM properties of QemuMachineState.

Yes---by value for now, then it can removed when you get to 
s/QEMUMachineInitArgs/QemuMachineState.

Including the "old" object by value BTW is how the CPU conversion 
started as well.

Paolo
diff mbox

Patch

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2c6e1a3..2ad12ad 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -11,15 +11,265 @@ 
  */
 
 #include "hw/boards.h"
+#include "qapi/visitor.h"
+
+static char *machine_get_accel(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return g_strdup(machine_state->accel);
+}
+
+static void machine_set_accel(Object *obj, const char *value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->accel = g_strdup(value);
+}
+
+static bool machine_get_kernel_irqchip(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return machine_state->kernel_irqchip;
+}
+
+static void machine_set_kernel_irqchip(Object *obj, bool value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_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)
+{
+    QemuMachineState *machine_state = QEMU_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)
+{
+    QemuMachineState *machine_state = QEMU_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)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return g_strdup(machine_state->kernel);
+}
+
+static void machine_set_kernel(Object *obj, const char *value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->kernel = g_strdup(value);
+}
+
+static char *machine_get_initrd(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return g_strdup(machine_state->initrd);
+}
+
+static void machine_set_initrd(Object *obj, const char *value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->initrd = g_strdup(value);
+}
+
+static char *machine_get_append(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return g_strdup(machine_state->append);
+}
+
+static void machine_set_append(Object *obj, const char *value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->append = g_strdup(value);
+}
+
+static char *machine_get_dtb(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return g_strdup(machine_state->dtb);
+}
+
+static void machine_set_dtb(Object *obj, const char *value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->dtb = g_strdup(value);
+}
+
+static char *machine_get_dumpdtb(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return g_strdup(machine_state->dumpdtb);
+}
+
+static void machine_set_dumpdtb(Object *obj, const char *value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_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)
+{
+    QemuMachineState *machine_state = QEMU_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)
+{
+    QemuMachineState *machine_state = QEMU_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)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return g_strdup(machine_state->dt_compatible);
+}
+
+static void machine_set_dt_compatible(Object *obj, const char *value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->dt_compatible = g_strdup(value);
+}
+
+static bool machine_get_dump_guest_core(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return machine_state->dump_guest_core;
+}
+
+static void machine_set_dump_guest_core(Object *obj, bool value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->dump_guest_core = value;
+}
+
+static bool machine_get_mem_merge(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return machine_state->mem_merge;
+}
+
+static void machine_set_mem_merge(Object *obj, bool value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->mem_merge = value;
+}
+
+static bool machine_get_usb(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return machine_state->usb;
+}
+
+static void machine_set_usb(Object *obj, bool value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->usb = value;
+}
+
+static char *machine_get_firmware(Object *obj, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    return g_strdup(machine_state->firmware);
+}
+
+static void machine_set_firmware(Object *obj, const char *value, Error **errp)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+    machine_state->firmware = g_strdup(value);
+}
 
 static void qemu_machine_initfn(Object *obj)
 {
+    object_property_add_str(obj, "accel",
+                            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_add(obj, "kvm-shadow-mem", "int",
+                        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",
+                        machine_get_phandle_start,
+                        machine_set_phandle_start,
+                        NULL, NULL, NULL);
+    object_property_add_str(obj, "dt_compatible",
+                            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_class_init(ObjectClass *oc, void *data)
 {
 }
 
+static void qemu_machine_finalize(Object *obj)
+{
+    QemuMachineState *machine_state = QEMU_MACHINE(obj);
+
+    g_free(machine_state->accel);
+    g_free(machine_state->kernel);
+    g_free(machine_state->initrd);
+    g_free(machine_state->append);
+    g_free(machine_state->dtb);
+    g_free(machine_state->dumpdtb);
+    g_free(machine_state->dt_compatible);
+    g_free(machine_state->firmware);
+}
+
 static const TypeInfo qemu_machine_info = {
     .name = TYPE_QEMU_MACHINE,
     .parent = TYPE_OBJECT,
@@ -28,6 +278,7 @@  static const TypeInfo qemu_machine_info = {
     .class_init = qemu_machine_class_init,
     .instance_size = sizeof(QemuMachineState),
     .instance_init = qemu_machine_initfn,
+    .instance_finalize = qemu_machine_finalize,
 };
 
 static void register_types(void)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3cd48fe..51bcaba 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -86,6 +86,21 @@  struct QemuMachineState {
     Object parent;
     /* public */
 
+    char *accel;
+    bool kernel_irqchip;
+    int kvm_shadow_mem;
+    char *kernel;
+    char *initrd;
+    char *append;
+    char *dtb;
+    char *dumpdtb;
+    int phandle_start;
+    char *dt_compatible;
+    bool dump_guest_core;
+    bool mem_merge;
+    bool usb;
+    char *firmware;
+
     QEMUMachineInitArgs *init_args;
 };