diff mbox

[v3,3/3] hw/boards: converted current_machine to be an instance of MachineCLass

Message ID 1394040647-20083-4-git-send-email-marcel.a@redhat.com
State New
Headers show

Commit Message

Marcel Apfelbaum March 5, 2014, 5:30 p.m. UTC 
In order to allow attaching machine options to a machine instance,
current_machine is converted into MachineState.
As a first step of deprecating QEMUMachine, some of the functions
were modified to return MachineCLass.

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

Comments

Eric Blake March 5, 2014, 5:36 p.m. UTC | #1 
On 03/05/2014 10:30 AM, Marcel Apfelbaum wrote:
> In order to allow attaching machine options to a machine instance,
> current_machine is converted into MachineState.
> As a first step of deprecating QEMUMachine, some of the functions
> were modified to return MachineCLass.

s/MachineCLass/MachineClass/ here and in subject

> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---

I'll leave the actual review to others, just pointing out the typo.
Marcel Apfelbaum March 5, 2014, 5:59 p.m. UTC | #2 
On Wed, 2014-03-05 at 10:36 -0700, Eric Blake wrote:
> On 03/05/2014 10:30 AM, Marcel Apfelbaum wrote:
> > In order to allow attaching machine options to a machine instance,
> > current_machine is converted into MachineState.
> > As a first step of deprecating QEMUMachine, some of the functions
> > were modified to return MachineCLass.
> 
> s/MachineCLass/MachineClass/ here and in subject
Thanks, I'll take care of it
Marcel

> 
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> 
> I'll leave the actual review to others, just pointing out the typo.
>
Alexey Kardashevskiy March 7, 2014, 1:16 a.m. UTC | #3 
On 03/07/2014 10:44 AM, Andreas Färber wrote:
> Am 05.03.2014 18:30, schrieb Marcel Apfelbaum:
>> In order to allow attaching machine options to a machine instance,
>> current_machine is converted into MachineState.
>> As a first step of deprecating QEMUMachine, some of the functions
>> were modified to return MachineCLass.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> 
> Looks mostly good, but same issue as Alexey's patch: We are risking
> qdev_get_machine() creating a Container-typed /machine node.

Sorry, I am not following you here. object_resolve_path() can create objects?


> What about the following on top?
> 
> Alexey, if we reach agreement here, this means for you that we can just
> use type_register_static() in place of qemu_machine_register() to
> register your custom machine type with interface added.

I am perfectly fine with that, I just do not see what difference does it
make and why do you still keep qemu_machine_register() (or this is in the
plan already?)?



> Regards,
> Andreas
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index b6deebd..749c83a 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -861,7 +861,8 @@ Object *qdev_get_machine(void)
>      static Object *dev;
> 
>      if (dev == NULL) {
> -        dev = container_get(object_get_root(), "/machine");
> +        dev = object_resolve_path("/machine", NULL);
> +        g_assert(dev);
>      }
> 
>      return dev;
> 
>
Marcel Apfelbaum March 7, 2014, 5:40 a.m. UTC | #4 
On Fri, 2014-03-07 at 12:16 +1100, Alexey Kardashevskiy wrote:
> On 03/07/2014 10:44 AM, Andreas Färber wrote:
> > Am 05.03.2014 18:30, schrieb Marcel Apfelbaum:
> >> In order to allow attaching machine options to a machine instance,
> >> current_machine is converted into MachineState.
> >> As a first step of deprecating QEMUMachine, some of the functions
> >> were modified to return MachineCLass.
> >>
> >> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > 
> > Looks mostly good, but same issue as Alexey's patch: We are risking
> > qdev_get_machine() creating a Container-typed /machine node.
> 
> Sorry, I am not following you here. object_resolve_path() can create objects?
Hi Alexey,
No, object_resolve_path() does not create objects, the point is that the machine
is created before, you only need to get it. You don't want to risk creating
a container. 

> 
> 
> > What about the following on top?
> > 
> > Alexey, if we reach agreement here, this means for you that we can just
> > use type_register_static() in place of qemu_machine_register() to
> > register your custom machine type with interface added.
> 
> I am perfectly fine with that, I just do not see what difference does it
> make and why do you still keep qemu_machine_register() (or this is in the
> plan already?)?
It is in my plan to eliminate qemu_machine_register(), however it will
take some time as it includes making changes to lots of files.
For the moment, subclassing MachineClass and registering it to QOM
will be exactly like calling qemu_machine_register().

Thanks,
Marcel

> 
> 
> 
> > Regards,
> > Andreas
> > 
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index b6deebd..749c83a 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -861,7 +861,8 @@ Object *qdev_get_machine(void)
> >      static Object *dev;
> > 
> >      if (dev == NULL) {
> > -        dev = container_get(object_get_root(), "/machine");
> > +        dev = object_resolve_path("/machine", NULL);
> > +        g_assert(dev);
> >      }
> > 
> >      return dev;
> > 
> > 
> 
>
diff mbox

Patch

diff --git a/device-hotplug.c b/device-hotplug.c
index 103d34a..ebfa6b1 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -33,12 +33,14 @@  DriveInfo *add_init_drive(const char *optstr)
 {
     DriveInfo *dinfo;
     QemuOpts *opts;
+    MachineClass *mc;
 
     opts = drive_def(optstr);
     if (!opts)
         return NULL;
 
-    dinfo = drive_init(opts, current_machine->block_default_type);
+    mc = MACHINE_GET_CLASS(current_machine);
+    dinfo = drive_init(opts, mc->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 7c93384..33debc2 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -51,9 +51,6 @@  struct QEMUMachine {
 
 #define TYPE_MACHINE_SUFFIX "-machine"
 int qemu_register_machine(QEMUMachine *m);
-QEMUMachine *find_default_machine(void);
-
-extern QEMUMachine *current_machine;
 
 #define TYPE_MACHINE "machine"
 #define MACHINE(obj) \
@@ -66,6 +63,9 @@  extern QEMUMachine *current_machine;
 typedef struct MachineState MachineState;
 typedef struct MachineClass MachineClass;
 
+MachineClass *find_default_machine(void);
+extern MachineState *current_machine;
+
 /**
  * @MachineClass
  *
diff --git a/qmp.c b/qmp.c
index f556a04..87a28f7 100644
--- a/qmp.c
+++ b/qmp.c
@@ -114,8 +114,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);
+    MachineClass *mc;
+
+    mc = MACHINE_GET_CLASS(current_machine);
+    if (mc->qemu_machine->hot_add_cpu) {
+        mc->qemu_machine->hot_add_cpu(id, errp);
     } else {
         error_setg(errp, "Not supported");
     }
diff --git a/vl.c b/vl.c
index 9fff2eb..ed1bb19 100644
--- a/vl.c
+++ b/vl.c
@@ -1525,7 +1525,7 @@  void pcmcia_info(Monitor *mon, const QDict *qdict)
 /***********************************************************/
 /* machine registration */
 
-QEMUMachine *current_machine = NULL;
+MachineState *current_machine;
 
 static void machine_class_init(ObjectClass *oc, void *data)
 {
@@ -1548,44 +1548,45 @@  int qemu_register_machine(QEMUMachine *m)
     return 0;
 }
 
-static QEMUMachine *find_machine(const char *name)
+static MachineClass *find_machine(const char *name)
 {
     GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
-    QEMUMachine *m = NULL;
+    MachineClass *mc = NULL;
 
     for (el = machines; el; el = el->next) {
-        MachineClass *mc = el->data;
+        MachineClass *temp = el->data;
 
-        if (!strcmp(mc->qemu_machine->name, name)) {
-            m = mc->qemu_machine;
+        if (!strcmp(temp->qemu_machine->name, name)) {
+            mc = temp;
             break;
         }
-        if (mc->qemu_machine->alias && !strcmp(mc->qemu_machine->alias, name)) {
-            m = mc->qemu_machine;
+        if (temp->qemu_machine->alias &&
+            !strcmp(temp->qemu_machine->alias, name)) {
+            mc = temp;
             break;
         }
     }
 
     g_slist_free(machines);
-    return m;
+    return mc;
 }
 
-QEMUMachine *find_default_machine(void)
+MachineClass *find_default_machine(void)
 {
     GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
-    QEMUMachine *m = NULL;
+    MachineClass *mc = NULL;
 
     for (el = machines; el; el = el->next) {
-        MachineClass *mc = el->data;
+        MachineClass *temp = el->data;
 
-        if (mc->qemu_machine->is_default) {
-            m = mc->qemu_machine;
+        if (temp->qemu_machine->is_default) {
+            mc = temp;
             break;
         }
     }
 
     g_slist_free(machines);
-    return m;
+    return mc;
 }
 
 MachineInfoList *qmp_query_machines(Error **errp)
@@ -1814,8 +1815,12 @@  void qemu_devices_reset(void)
 
 void qemu_system_reset(bool report)
 {
-    if (current_machine && current_machine->reset) {
-        current_machine->reset();
+    MachineClass *mc;
+
+    mc = current_machine ? MACHINE_GET_CLASS(current_machine) : NULL;
+
+    if (mc && mc->qemu_machine->reset) {
+        mc->qemu_machine->reset();
     } else {
         qemu_devices_reset();
     }
@@ -2585,21 +2590,21 @@  static int debugcon_parse(const char *devname)
     return 0;
 }
 
-static QEMUMachine *machine_parse(const char *name)
+static MachineClass *machine_parse(const char *name)
 {
-    QEMUMachine *m, *machine = NULL;
+    MachineClass *mc = NULL;
     GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
 
     if (name) {
-        machine = find_machine(name);
+        mc = find_machine(name);
     }
-    if (machine) {
-        return machine;
+    if (mc) {
+        return mc;
     }
     printf("Supported machines are:\n");
     for (el = machines; el; el = el->next) {
         MachineClass *mc = el->data;
-        m = mc->qemu_machine;
+        QEMUMachine *m = mc->qemu_machine;
         if (m->alias) {
             printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name);
         }
@@ -2856,6 +2861,7 @@  int main(int argc, char **argv, char **envp)
     int optind;
     const char *optarg;
     const char *loadvm = NULL;
+    MachineClass *machine_class;
     QEMUMachine *machine;
     const char *cpu_model;
     const char *vga_model = "none";
@@ -2929,7 +2935,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;
@@ -2995,7 +3001,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");
@@ -3551,7 +3557,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:
@@ -3865,11 +3871,17 @@  int main(int argc, char **argv, char **envp)
     }
 #endif
 
-    if (machine == NULL) {
+    if (machine_class == NULL) {
         fprintf(stderr, "No machine found.\n");
         exit(1);
     }
 
+    current_machine = MACHINE(object_new(object_class_get_name(
+                          OBJECT_CLASS(machine_class))));
+    object_property_add_child(object_get_root(), "machine",
+                              OBJECT(current_machine), &error_abort);
+
+    machine = machine_class->qemu_machine;
     if (machine->hw_version) {
         qemu_set_version(machine->hw_version);
     }
@@ -4298,7 +4310,9 @@  int main(int argc, char **argv, char **envp)
                                  .kernel_cmdline = kernel_cmdline,
                                  .initrd_filename = initrd_filename,
                                  .cpu_model = cpu_model };
-    machine->init(&args);
+
+    current_machine->init_args = args;
+    machine->init(&current_machine->init_args);
 
     audio_init();
 
@@ -4306,8 +4320,6 @@  int main(int argc, char **argv, char **envp)
 
     set_numa_modes();
 
-    current_machine = machine;
-
     /* init USB devices */
     if (usb_enabled(false)) {
         if (foreach_device_config(DEV_USB, usb_parse) < 0)