[Qemu-devel] [PATCH RFC 0/5] qemu-machine as a QOM object

[Qemu-devel] [PATCH RFC 0/5] qemu-machine as a QOM object

[Marcel Apfelbaum]

This is an early RFC, the work is in very early stages, I am interested
to know if there is a consensus that this is the right path.

The main benefit of QOMifying the qemu machine would be the possibility
to have options per machine type and not global.
However, there are other benefits as:
  - accessing qemu object properties instead of a global QemuOpts
    list from different code subsystems.
  - improving the machine "initialization" code (compat and stuff)
  - getting more close to QOM's vision of single interface for
    device creation and so on.

Basically the series aims (in the long run) to convert:
    QEMUMachine -> QemuMachineClass
    QEMUMachineInitArgs -> QemuMachineState.
As a first step, in order to make possible an incremental development,
both QEMUMachine and QEMUMachineInitArgs are being embedded into the
new types.

Your comments are welcomed,
Marcel

Marcel Apfelbaum (5):
  hw/core: introduced qemu machine as QOM object
  vl: use qemu machine QOM class instead of global machines list
  hw/boards: converted current_machine to be an instance of
    QemuMachineCLass
  hw/machine: add qemu machine opts as properties to QemuMachineState
  vl.c: set current_machine's properties

 device-hotplug.c      |   4 +-
 hw/core/Makefile.objs |   2 +-
 hw/core/machine.c     | 289 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/boards.h   |  55 +++++++++-
 qmp.c                 |   7 +-
 vl.c                  | 122 ++++++++++++++-------
 6 files changed, 435 insertions(+), 44 deletions(-)
 create mode 100644 hw/core/machine.c

-- 
1.8.3.1

[Qemu-devel] [PATCH RFC 1/5] hw/core: introduced qemu machine as QOM object

[Marcel Apfelbaum]

The main functionality change is to convert QEMUMachine into QemuMachineClass
and QEMUMachineInitArgs into QemuMachineState, instance of QemuMachineClass.

As a first step, in order to make possible an incremental developement,
both QEMUMachine and QEMUMachineInitArgs are being embeded into the
new types.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/core/Makefile.objs |  2 +-
 hw/core/machine.c     | 38 ++++++++++++++++++++++++++++++++++++++
 include/hw/boards.h   | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 hw/core/machine.c

diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 950146c..aabd1ef 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -10,4 +10,4 @@ common-obj-$(CONFIG_SOFTMMU) += sysbus.o
 common-obj-$(CONFIG_SOFTMMU) += null-machine.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
-
+common-obj-$(CONFIG_SOFTMMU) += machine.o
diff --git a/hw/core/machine.c b/hw/core/machine.c
new file mode 100644
index 0000000..2c6e1a3
--- /dev/null
+++ b/hw/core/machine.c
@@ -0,0 +1,38 @@
+/*
+ * QEMU Machine
+ *
+ * Copyright (C) 2013 Red Hat Inc
+ *
+ * Authors:
+ *   Marcel Apfelbaum <marcel.a@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "hw/boards.h"
+
+static void qemu_machine_initfn(Object *obj)
+{
+}
+
+static void qemu_machine_class_init(ObjectClass *oc, void *data)
+{
+}
+
+static const TypeInfo qemu_machine_info = {
+    .name = TYPE_QEMU_MACHINE,
+    .parent = TYPE_OBJECT,
+    .abstract = true,
+    .class_size = sizeof(QemuMachineClass),
+    .class_init = qemu_machine_class_init,
+    .instance_size = sizeof(QemuMachineState),
+    .instance_init = qemu_machine_initfn,
+};
+
+static void register_types(void)
+{
+    type_register_static(&qemu_machine_info);
+}
+
+type_init(register_types);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2151460..682cd7f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -5,6 +5,7 @@
 
 #include "sysemu/blockdev.h"
 #include "hw/qdev.h"
+#include "qom/object.h"
 
 typedef struct QEMUMachine QEMUMachine;
 
@@ -53,4 +54,39 @@ QEMUMachine *find_default_machine(void);
 
 extern QEMUMachine *current_machine;
 
+#define TYPE_QEMU_MACHINE "machine"
+#define QEMU_MACHINE(obj) \
+    OBJECT_CHECK(QemuMachineState, (obj), TYPE_QEMU_MACHINE)
+#define QEMU_MACHINE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(QemuMachineClass, (obj), TYPE_QEMU_MACHINE)
+#define QEMU_MACHINE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(QemuMachineClass, (klass), TYPE_QEMU_MACHINE)
+
+typedef struct QemuMachineState QemuMachineState;
+typedef struct QemuMachineClass QemuMachineClass;
+
+/**
+ * @QemuMachineClass
+ *
+ * @parent_class: opaque parent class container
+ */
+struct QemuMachineClass {
+    ObjectClass parent_class;
+
+    QEMUMachine *qemu_machine;
+};
+
+/**
+ * @QemuMachineState
+ *
+ * @parent: opaque parent object container
+ */
+struct QemuMachineState {
+    /* private */
+    Object parent;
+    /* public */
+
+    QEMUMachineInitArgs *init_args;
+};
+
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC 2/5] vl: use qemu machine QOM class instead of global machines list

[Marcel Apfelbaum]

The machine registration flow is refactored to use the QOM functionality.
Instead of linking the machines into a list, each machine has a type
and the types can be traversed in the QOM way.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 vl.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index 7f4fe0d..9c7f599 100644
--- a/vl.c
+++ b/vl.c
@@ -1581,54 +1581,81 @@ void pcmcia_info(Monitor *mon, const QDict *qdict)
 /***********************************************************/
 /* machine registration */
 
-static QEMUMachine *first_machine = NULL;
 QEMUMachine *current_machine = NULL;
 
+static void qemu_machine_class_init(ObjectClass *klass, void *data)
+{
+    QemuMachineClass *k = QEMU_MACHINE_CLASS(klass);
+
+    k->qemu_machine = data;
+}
+
 int qemu_register_machine(QEMUMachine *m)
 {
-    QEMUMachine **pm;
-    pm = &first_machine;
-    while (*pm != NULL)
-        pm = &(*pm)->next;
-    m->next = NULL;
-    *pm = m;
+    TypeInfo ti = {
+        .name       = m->name,
+        .parent     = TYPE_QEMU_MACHINE,
+        .class_init = qemu_machine_class_init,
+        .class_data = (void *)m,
+    };
+
+    type_register(&ti);
+
     return 0;
 }
 
 static QEMUMachine *find_machine(const char *name)
 {
-    QEMUMachine *m;
+    GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false);
+    QEMUMachine *m = NULL;
+
+    for (el = machines; el; el = el->next) {
+        QemuMachineClass *k = el->data;
 
-    for(m = first_machine; m != NULL; m = m->next) {
-        if (!strcmp(m->name, name))
-            return m;
-        if (m->alias && !strcmp(m->alias, name))
-            return m;
+        if (!strcmp(k->qemu_machine->name, name)) {
+            m = k->qemu_machine;
+            break;
+        }
+        if (k->qemu_machine->alias && !strcmp(k->qemu_machine->alias, name)) {
+            m = k->qemu_machine;
+            break;
+        }
     }
-    return NULL;
+
+    g_slist_free(machines);
+    return m;
 }
 
 QEMUMachine *find_default_machine(void)
 {
-    QEMUMachine *m;
+    GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false);
+    QEMUMachine *m = NULL;
 
-    for(m = first_machine; m != NULL; m = m->next) {
-        if (m->is_default) {
-            return m;
+    for (el = machines; el; el = el->next) {
+        QemuMachineClass *k = el->data;
+
+        if (k->qemu_machine->is_default) {
+            m = k->qemu_machine;
+            break;
         }
     }
-    return NULL;
+
+    g_slist_free(machines);
+    return m;
 }
 
 MachineInfoList *qmp_query_machines(Error **errp)
 {
+    GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false);
     MachineInfoList *mach_list = NULL;
     QEMUMachine *m;
 
-    for (m = first_machine; m; m = m->next) {
+    for (el = machines; el; el = el->next) {
+        QemuMachineClass *k = el->data;
         MachineInfoList *entry;
         MachineInfo *info;
 
+        m = k->qemu_machine;
         info = g_malloc0(sizeof(*info));
         if (m->is_default) {
             info->has_is_default = true;
@@ -1649,6 +1676,7 @@ MachineInfoList *qmp_query_machines(Error **errp)
         mach_list = entry;
     }
 
+    g_slist_free(machines);
     return mach_list;
 }
 
@@ -2592,6 +2620,7 @@ static int debugcon_parse(const char *devname)
 static QEMUMachine *machine_parse(const char *name)
 {
     QEMUMachine *m, *machine = NULL;
+    GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false);
 
     if (name) {
         machine = find_machine(name);
@@ -2600,13 +2629,17 @@ static QEMUMachine *machine_parse(const char *name)
         return machine;
     }
     printf("Supported machines are:\n");
-    for (m = first_machine; m != NULL; m = m->next) {
+    for (el = machines; el; el = el->next) {
+        QemuMachineClass *k = el->data;
+        m = k->qemu_machine;
         if (m->alias) {
             printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name);
         }
         printf("%-20s %s%s\n", m->name, m->desc,
                m->is_default ? " (default)" : "");
     }
+
+    g_slist_free(machines);
     exit(!name || !is_help_option(name));
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC 3/5] hw/boards: converted current_machine to be an instance of QemuMachineCLass

[Marcel Apfelbaum]

In order to allow attaching machine options to a machine instance,
current_machine is converted into QemuMachineState.
As a first step of deprecating QEMUMachine, some of the functions
were modified to return QemuMachineCLass.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 device-hotplug.c    |  4 +++-
 include/hw/boards.h |  6 +++---
 qmp.c               |  7 +++++--
 vl.c                | 55 +++++++++++++++++++++++++++++------------------------
 4 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/device-hotplug.c b/device-hotplug.c
index 103d34a..fb5eb01 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -33,12 +33,14 @@ DriveInfo *add_init_drive(const char *optstr)
 {
     DriveInfo *dinfo;
     QemuOpts *opts;
+    QemuMachineClass *machine_class;
 
     opts = drive_def(optstr);
     if (!opts)
         return NULL;
 
-    dinfo = drive_init(opts, current_machine->block_default_type);
+    machine_class = QEMU_MACHINE_GET_CLASS(current_machine);
+    dinfo = drive_init(opts, machine_class->qemu_machine->block_default_type);
     if (!dinfo) {
         qemu_opts_del(opts);
         return NULL;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 682cd7f..3cd48fe 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -50,9 +50,6 @@ struct QEMUMachine {
 };
 
 int qemu_register_machine(QEMUMachine *m);
-QEMUMachine *find_default_machine(void);
-
-extern QEMUMachine *current_machine;
 
 #define TYPE_QEMU_MACHINE "machine"
 #define QEMU_MACHINE(obj) \
@@ -65,6 +62,9 @@ extern QEMUMachine *current_machine;
 typedef struct QemuMachineState QemuMachineState;
 typedef struct QemuMachineClass QemuMachineClass;
 
+QemuMachineClass *find_default_machine(void);
+extern QemuMachineState *current_machine;
+
 /**
  * @QemuMachineClass
  *
diff --git a/qmp.c b/qmp.c
index 0f46171..a88694f 100644
--- a/qmp.c
+++ b/qmp.c
@@ -113,8 +113,11 @@ void qmp_cpu(int64_t index, Error **errp)
 
 void qmp_cpu_add(int64_t id, Error **errp)
 {
-    if (current_machine->hot_add_cpu) {
-        current_machine->hot_add_cpu(id, errp);
+    QemuMachineClass *machine_class;
+
+    machine_class = QEMU_MACHINE_GET_CLASS(current_machine);
+    if (machine_class->qemu_machine->hot_add_cpu) {
+        machine_class->qemu_machine->hot_add_cpu(id, errp);
     } else {
         error_setg(errp, "Not supported");
     }
diff --git a/vl.c b/vl.c
index 9c7f599..4f4722e 100644
--- a/vl.c
+++ b/vl.c
@@ -1581,7 +1581,7 @@ void pcmcia_info(Monitor *mon, const QDict *qdict)
 /***********************************************************/
 /* machine registration */
 
-QEMUMachine *current_machine = NULL;
+QemuMachineState *current_machine;
 
 static void qemu_machine_class_init(ObjectClass *klass, void *data)
 {
@@ -1604,44 +1604,41 @@ int qemu_register_machine(QEMUMachine *m)
     return 0;
 }
 
-static QEMUMachine *find_machine(const char *name)
+static QemuMachineClass *find_machine(const char *name)
 {
     GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false);
-    QEMUMachine *m = NULL;
+    QemuMachineClass *k = NULL;
 
     for (el = machines; el; el = el->next) {
-        QemuMachineClass *k = el->data;
+        k = el->data;
 
         if (!strcmp(k->qemu_machine->name, name)) {
-            m = k->qemu_machine;
             break;
         }
         if (k->qemu_machine->alias && !strcmp(k->qemu_machine->alias, name)) {
-            m = k->qemu_machine;
             break;
         }
     }
 
     g_slist_free(machines);
-    return m;
+    return k;
 }
 
-QEMUMachine *find_default_machine(void)
+QemuMachineClass *find_default_machine(void)
 {
     GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false);
-    QEMUMachine *m = NULL;
+    QemuMachineClass *k = NULL;
 
     for (el = machines; el; el = el->next) {
-        QemuMachineClass *k = el->data;
+        k = el->data;
 
         if (k->qemu_machine->is_default) {
-            m = k->qemu_machine;
             break;
         }
     }
 
     g_slist_free(machines);
-    return m;
+    return k;
 }
 
 MachineInfoList *qmp_query_machines(Error **errp)
@@ -1870,8 +1867,13 @@ void qemu_devices_reset(void)
 
 void qemu_system_reset(bool report)
 {
-    if (current_machine && current_machine->reset) {
-        current_machine->reset();
+    QemuMachineClass *machine_class;
+
+    machine_class = current_machine ? QEMU_MACHINE_GET_CLASS(current_machine)
+                    : NULL;
+
+    if (machine_class && machine_class->qemu_machine->reset) {
+        machine_class->qemu_machine->reset();
     } else {
         qemu_devices_reset();
     }
@@ -2617,21 +2619,21 @@ static int debugcon_parse(const char *devname)
     return 0;
 }
 
-static QEMUMachine *machine_parse(const char *name)
+static QemuMachineClass *machine_parse(const char *name)
 {
-    QEMUMachine *m, *machine = NULL;
+    QemuMachineClass *machine_class = NULL;
     GSList *el, *machines = object_class_get_list(TYPE_QEMU_MACHINE, false);
 
     if (name) {
-        machine = find_machine(name);
+        machine_class = find_machine(name);
     }
-    if (machine) {
-        return machine;
+    if (machine_class) {
+        return machine_class;
     }
     printf("Supported machines are:\n");
     for (el = machines; el; el = el->next) {
         QemuMachineClass *k = el->data;
-        m = k->qemu_machine;
+        QEMUMachine *m = k->qemu_machine;
         if (m->alias) {
             printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name);
         }
@@ -2869,6 +2871,7 @@ int main(int argc, char **argv, char **envp)
     int optind;
     const char *optarg;
     const char *loadvm = NULL;
+    QemuMachineClass *machine_class;
     QEMUMachine *machine;
     const char *cpu_model;
     const char *vga_model = "none";
@@ -2941,7 +2944,7 @@ int main(int argc, char **argv, char **envp)
     os_setup_early_signal_handling();
 
     module_call_init(MODULE_INIT_MACHINE);
-    machine = find_default_machine();
+    machine_class = find_default_machine();
     cpu_model = NULL;
     ram_size = 0;
     snapshot = 0;
@@ -3007,7 +3010,7 @@ int main(int argc, char **argv, char **envp)
             }
             switch(popt->index) {
             case QEMU_OPTION_M:
-                machine = machine_parse(optarg);
+                machine_class = machine_parse(optarg);
                 break;
             case QEMU_OPTION_no_kvm_irqchip: {
                 olist = qemu_find_opts("machine");
@@ -3553,7 +3556,7 @@ int main(int argc, char **argv, char **envp)
                 }
                 optarg = qemu_opt_get(opts, "type");
                 if (optarg) {
-                    machine = machine_parse(optarg);
+                    machine_class = machine_parse(optarg);
                 }
                 break;
              case QEMU_OPTION_no_kvm:
@@ -3867,11 +3870,12 @@ int main(int argc, char **argv, char **envp)
     }
 #endif
 
-    if (machine == NULL) {
+    if (machine_class == NULL) {
         fprintf(stderr, "No machine found.\n");
         exit(1);
     }
 
+    machine = machine_class->qemu_machine;
     if (machine->hw_version) {
         qemu_set_version(machine->hw_version);
     }
@@ -4300,7 +4304,8 @@ int main(int argc, char **argv, char **envp)
 
     set_numa_modes();
 
-    current_machine = machine;
+    current_machine = QEMU_MACHINE(object_new(object_class_get_name(
+                          OBJECT_CLASS(machine_class))));
 
     /* init USB devices */
     if (usb_enabled(false)) {
-- 
1.8.3.1

[Qemu-devel] [PATCH RFC 4/5] hw/machine: add qemu machine opts as properties to QemuMachineState

[Marcel Apfelbaum]

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(+)

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;
 };
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH RFC 4/5] hw/machine: add qemu machine opts as properties to QemuMachineState

[Paolo Bonzini]

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;

Re: [Qemu-devel] [PATCH RFC 4/5] hw/machine: add qemu machine opts as properties to QemuMachineState

[Marcel Apfelbaum]

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;
> 

Re: [Qemu-devel] [PATCH RFC 4/5] hw/machine: add qemu machine opts as properties to QemuMachineState

[Paolo Bonzini]

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

[Qemu-devel] [PATCH RFC 5/5] vl.c: set current_machine's properties

[Marcel Apfelbaum]

Setting machine's properties provides a cleaner
way to get the properties values than using
QemuOpts.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 vl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/vl.c b/vl.c
index 4f4722e..fcd56db 100644
--- a/vl.c
+++ b/vl.c
@@ -4307,6 +4307,11 @@ int main(int argc, char **argv, char **envp)
     current_machine = QEMU_MACHINE(object_new(object_class_get_name(
                           OBJECT_CLASS(machine_class))));
 
+    if (qemu_opt_foreach(machine_opts, object_set_property, current_machine, 1) < 0) {
+        object_unref(OBJECT(current_machine));
+        exit(1);
+    }
+
     /* init USB devices */
     if (usb_enabled(false)) {
         if (foreach_device_config(DEV_USB, usb_parse) < 0)
-- 
1.8.3.1