[Qemu-devel] [PATCH V2 0/5] remove QEMUMachine indirection from MachineClass

[Qemu-devel] [PATCH V2 0/5] remove QEMUMachine indirection from MachineClass

[Marcel Apfelbaum]

V1 -> V2:
  - Addressed Paolo's comments:
    - replaced commas by semicolons on patch 4/5.
  - Rebased to master.

This is a continuation of 'QEMU Machine as QOM object' effort.
The scope of this series is to allow machine QOM-ification
of all machines gradually, by removing the need for QEMUMachine registration through vl.c .

Now we will have 2 paths:
1. Non QOM-ified machines will be converted to QOM on the fly
   in vl.c by qemu machine registration.
2. QOM-ified machines will behave as regular QOM classes setting
   MachineClass fields in class_init.
   - Patch 4/5 demonstrates this.

Next steps:
 - Replace QemuOpts queries by MachineState fields.
 - Follow Paolo's suggestions to get rid of QEMUMachineInitArgs.

Comments are appreciated,

Thanks,
Marcel

Marcel Apfelbaum (5):
  hw/boards.h: add QEMUMachine's fields to MachineClass
  machine: remove QEMUMachine indirection from MachineClass
  machine: replace QEMUMachine by MachineClass in accelerator
    configuration
  hw/ppc: remove QEMUMachine indirection
  vl.c: Remove QEMUMachine usage

 device-hotplug.c        |   2 +-
 hw/ppc/spapr.c          |  26 +++++------
 include/hw/boards.h     |  30 +++++++++++--
 include/hw/xen/xen.h    |   2 +-
 include/qemu/typedefs.h |   1 +
 include/sysemu/kvm.h    |   2 +-
 include/sysemu/qtest.h  |   2 +-
 kvm-all.c               |   6 +--
 kvm-stub.c              |   2 +-
 qmp.c                   |   4 +-
 qtest.c                 |   2 +-
 vl.c                    | 114 +++++++++++++++++++++++++++---------------------
 xen-all.c               |   2 +-
 xen-stub.c              |   2 +-
 14 files changed, 116 insertions(+), 81 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH V2 1/5] hw/boards.h: add QEMUMachine's fields to MachineClass

[Marcel Apfelbaum]

In order to eliminate the QEMUMachine indirection,
add its fields directly to MachineClass.
Do not remove yet qemu_machine field because it is
in use already by sparpr.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 include/hw/boards.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index dd2c70d..7cf1f07 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -78,6 +78,29 @@ struct MachineClass {
     /*< public >*/
 
     QEMUMachine *qemu_machine;
+    const char *name;
+    const char *alias;
+    const char *desc;
+
+    void (*init)(QEMUMachineInitArgs *args);
+    void (*reset)(void);
+    void (*hot_add_cpu)(const int64_t id, Error **errp);
+    int (*kvm_type)(const char *arg);
+
+    BlockInterfaceType block_default_type;
+    int max_cpus;
+    unsigned int no_serial:1,
+        no_parallel:1,
+        use_virtcon:1,
+        use_sclp:1,
+        no_floppy:1,
+        no_cdrom:1,
+        no_sdcard:1;
+    int is_default;
+    const char *default_machine_opts;
+    const char *default_boot_order;
+    GlobalProperty *compat_props;
+    const char *hw_version;
 };
 
 /**
-- 
1.8.3.1

[Qemu-devel] [PATCH V2 2/5] machine: remove QEMUMachine indirection from MachineClass

[Marcel Apfelbaum]

No need to go through qemu_machine field. Use
MachineClass fields directly.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 device-hotplug.c |   2 +-
 qmp.c            |   4 +--
 vl.c             | 103 ++++++++++++++++++++++++++++++++-----------------------
 3 files changed, 63 insertions(+), 46 deletions(-)

diff --git a/device-hotplug.c b/device-hotplug.c
index ebfa6b1..eecb08e 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -40,7 +40,7 @@ DriveInfo *add_init_drive(const char *optstr)
         return NULL;
 
     mc = MACHINE_GET_CLASS(current_machine);
-    dinfo = drive_init(opts, mc->qemu_machine->block_default_type);
+    dinfo = drive_init(opts, mc->block_default_type);
     if (!dinfo) {
         qemu_opts_del(opts);
         return NULL;
diff --git a/qmp.c b/qmp.c
index 87a28f7..26eb589 100644
--- a/qmp.c
+++ b/qmp.c
@@ -117,8 +117,8 @@ void qmp_cpu_add(int64_t id, Error **errp)
     MachineClass *mc;
 
     mc = MACHINE_GET_CLASS(current_machine);
-    if (mc->qemu_machine->hot_add_cpu) {
-        mc->qemu_machine->hot_add_cpu(id, errp);
+    if (mc->hot_add_cpu) {
+        mc->hot_add_cpu(id, errp);
     } else {
         error_setg(errp, "Not supported");
     }
diff --git a/vl.c b/vl.c
index 9975e5a..96155ca 100644
--- a/vl.c
+++ b/vl.c
@@ -1583,8 +1583,29 @@ MachineState *current_machine;
 static void machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
-
-    mc->qemu_machine = data;
+    QEMUMachine *qm = data;
+
+    mc->name = qm->name;
+    mc->alias = qm->alias;
+    mc->desc = qm->desc;
+    mc->init = qm->init;
+    mc->reset = qm->reset;
+    mc->hot_add_cpu = qm->hot_add_cpu;
+    mc->kvm_type = qm->kvm_type;
+    mc->block_default_type = qm->block_default_type;
+    mc->max_cpus = qm->max_cpus;
+    mc->no_serial = qm->no_serial;
+    mc->no_parallel = qm->no_parallel;
+    mc->use_virtcon = qm->use_virtcon;
+    mc->use_sclp = qm->use_sclp;
+    mc->no_floppy = qm->no_floppy;
+    mc->no_cdrom = qm->no_cdrom;
+    mc->no_sdcard = qm->no_sdcard;
+    mc->is_default = qm->is_default;
+    mc->default_machine_opts = qm->default_machine_opts;
+    mc->default_boot_order = qm->default_boot_order;
+    mc->compat_props = qm->compat_props;
+    mc->hw_version = qm->hw_version;
 }
 
 int qemu_register_machine(QEMUMachine *m)
@@ -1611,12 +1632,12 @@ static MachineClass *find_machine(const char *name)
     for (el = machines; el; el = el->next) {
         MachineClass *temp = el->data;
 
-        if (!strcmp(temp->qemu_machine->name, name)) {
+        if (!strcmp(temp->name, name)) {
             mc = temp;
             break;
         }
-        if (temp->qemu_machine->alias &&
-            !strcmp(temp->qemu_machine->alias, name)) {
+        if (temp->alias &&
+            !strcmp(temp->alias, name)) {
             mc = temp;
             break;
         }
@@ -1634,7 +1655,7 @@ MachineClass *find_default_machine(void)
     for (el = machines; el; el = el->next) {
         MachineClass *temp = el->data;
 
-        if (temp->qemu_machine->is_default) {
+        if (temp->is_default) {
             mc = temp;
             break;
         }
@@ -1648,27 +1669,25 @@ MachineInfoList *qmp_query_machines(Error **errp)
 {
     GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
     MachineInfoList *mach_list = NULL;
-    QEMUMachine *m;
 
     for (el = machines; el; el = el->next) {
         MachineClass *mc = el->data;
         MachineInfoList *entry;
         MachineInfo *info;
 
-        m = mc->qemu_machine;
         info = g_malloc0(sizeof(*info));
-        if (m->is_default) {
+        if (mc->is_default) {
             info->has_is_default = true;
             info->is_default = true;
         }
 
-        if (m->alias) {
+        if (mc->alias) {
             info->has_alias = true;
-            info->alias = g_strdup(m->alias);
+            info->alias = g_strdup(mc->alias);
         }
 
-        info->name = g_strdup(m->name);
-        info->cpu_max = !m->max_cpus ? 1 : m->max_cpus;
+        info->name = g_strdup(mc->name);
+        info->cpu_max = !mc->max_cpus ? 1 : mc->max_cpus;
 
         entry = g_malloc0(sizeof(*entry));
         entry->value = info;
@@ -1874,8 +1893,8 @@ void qemu_system_reset(bool report)
 
     mc = current_machine ? MACHINE_GET_CLASS(current_machine) : NULL;
 
-    if (mc && mc->qemu_machine->reset) {
-        mc->qemu_machine->reset();
+    if (mc && mc->reset) {
+        mc->reset();
     } else {
         qemu_devices_reset();
     }
@@ -2684,12 +2703,11 @@ static MachineClass *machine_parse(const char *name)
         printf("Supported machines are:\n");
         for (el = machines; el; el = el->next) {
             MachineClass *mc = el->data;
-            QEMUMachine *m = mc->qemu_machine;
-            if (m->alias) {
-                printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->name);
+            if (mc->alias) {
+                printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
             }
-            printf("%-20s %s%s\n", m->name, m->desc,
-                   m->is_default ? " (default)" : "");
+            printf("%-20s %s%s\n", mc->name, mc->desc,
+                   mc->is_default ? " (default)" : "");
         }
     }
 
@@ -2943,7 +2961,7 @@ int main(int argc, char **argv, char **envp)
     const char *optarg;
     const char *loadvm = NULL;
     MachineClass *machine_class;
-    QEMUMachine *machine;
+    QEMUMachine *machine = NULL;
     const char *cpu_model;
     const char *vga_model = NULL;
     const char *qtest_chrdev = NULL;
@@ -3939,9 +3957,8 @@ int main(int argc, char **argv, char **envp)
     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);
+    if (machine_class->hw_version) {
+        qemu_set_version(machine_class->hw_version);
     }
 
     if (qemu_opts_foreach(qemu_find_opts("object"),
@@ -4001,11 +4018,11 @@ int main(int argc, char **argv, char **envp)
 
     smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
 
-    machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
-    if (smp_cpus > machine->max_cpus) {
+    machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
+    if (smp_cpus > machine_class->max_cpus) {
         fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
-                "supported by machine `%s' (%d)\n", smp_cpus,  machine->name,
-                machine->max_cpus);
+                "supported by machine `%s' (%d)\n", smp_cpus,
+                machine_class->name, machine_class->max_cpus);
         exit(1);
     }
 
@@ -4013,9 +4030,9 @@ int main(int argc, char **argv, char **envp)
      * Get the default machine options from the machine if it is not already
      * specified either by the configuration file or by the command line.
      */
-    if (machine->default_machine_opts) {
+    if (machine_class->default_machine_opts) {
         qemu_opts_set_defaults(qemu_find_opts("machine"),
-                               machine->default_machine_opts, 0);
+                               machine_class->default_machine_opts, 0);
     }
 
     qemu_opts_foreach(qemu_find_opts("device"), default_driver_check, NULL, 0);
@@ -4024,25 +4041,25 @@ int main(int argc, char **argv, char **envp)
     if (!vga_model && !default_vga) {
         vga_interface_type = VGA_DEVICE;
     }
-    if (!has_defaults || machine->no_serial) {
+    if (!has_defaults || machine_class->no_serial) {
         default_serial = 0;
     }
-    if (!has_defaults || machine->no_parallel) {
+    if (!has_defaults || machine_class->no_parallel) {
         default_parallel = 0;
     }
-    if (!has_defaults || !machine->use_virtcon) {
+    if (!has_defaults || !machine_class->use_virtcon) {
         default_virtcon = 0;
     }
-    if (!has_defaults || !machine->use_sclp) {
+    if (!has_defaults || !machine_class->use_sclp) {
         default_sclp = 0;
     }
-    if (!has_defaults || machine->no_floppy) {
+    if (!has_defaults || machine_class->no_floppy) {
         default_floppy = 0;
     }
-    if (!has_defaults || machine->no_cdrom) {
+    if (!has_defaults || machine_class->no_cdrom) {
         default_cdrom = 0;
     }
-    if (!has_defaults || machine->no_sdcard) {
+    if (!has_defaults || machine_class->no_sdcard) {
         default_sdcard = 0;
     }
     if (!has_defaults) {
@@ -4182,7 +4199,7 @@ int main(int argc, char **argv, char **envp)
     kernel_cmdline = qemu_opt_get(machine_opts, "append");
     bios_name = qemu_opt_get(machine_opts, "firmware");
 
-    boot_order = machine->default_boot_order;
+    boot_order = machine_class->default_boot_order;
     opts = qemu_opts_find(qemu_find_opts("boot-opts"), NULL);
     if (opts) {
         char *normal_boot_order;
@@ -4276,11 +4293,11 @@ int main(int argc, char **argv, char **envp)
     if (snapshot)
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);
     if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
-                          &machine->block_default_type, 1) != 0) {
+                          &machine_class->block_default_type, 1) != 0) {
         exit(1);
     }
 
-    default_drive(default_cdrom, snapshot, machine->block_default_type, 2,
+    default_drive(default_cdrom, snapshot, machine_class->block_default_type, 2,
                   CDROM_OPTS);
     default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
     default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
@@ -4364,8 +4381,8 @@ int main(int argc, char **argv, char **envp)
             exit (i == 1 ? 1 : 0);
     }
 
-    if (machine->compat_props) {
-        qdev_prop_register_global_list(machine->compat_props);
+    if (machine_class->compat_props) {
+        qdev_prop_register_global_list(machine_class->compat_props);
     }
     qemu_add_globals();
 
@@ -4380,7 +4397,7 @@ int main(int argc, char **argv, char **envp)
                                  .cpu_model = cpu_model };
 
     current_machine->init_args = args;
-    machine->init(&current_machine->init_args);
+    machine_class->init(&current_machine->init_args);
 
     audio_init();
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V2 2/5] machine: remove QEMUMachine indirection from MachineClass

[Andreas Färber]

Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
> No need to go through qemu_machine field. Use
> MachineClass fields directly.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  device-hotplug.c |   2 +-
>  qmp.c            |   4 +--
>  vl.c             | 103 ++++++++++++++++++++++++++++++++-----------------------
>  3 files changed, 63 insertions(+), 46 deletions(-)
[...]
> diff --git a/vl.c b/vl.c
> index 9975e5a..96155ca 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1583,8 +1583,29 @@ MachineState *current_machine;
>  static void machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> -
> -    mc->qemu_machine = data;

This looks incomplete. You should drop the qemu_machine field from
boards.h to assure that no one is using it - pseries still is, so 4/5
needs to be be squashed into this commit, otherwise the new fields below
remain empty for it.

Regards,
Andreas

> +    QEMUMachine *qm = data;
> +
> +    mc->name = qm->name;
> +    mc->alias = qm->alias;
> +    mc->desc = qm->desc;
> +    mc->init = qm->init;
> +    mc->reset = qm->reset;
> +    mc->hot_add_cpu = qm->hot_add_cpu;
> +    mc->kvm_type = qm->kvm_type;
> +    mc->block_default_type = qm->block_default_type;
> +    mc->max_cpus = qm->max_cpus;
> +    mc->no_serial = qm->no_serial;
> +    mc->no_parallel = qm->no_parallel;
> +    mc->use_virtcon = qm->use_virtcon;
> +    mc->use_sclp = qm->use_sclp;
> +    mc->no_floppy = qm->no_floppy;
> +    mc->no_cdrom = qm->no_cdrom;
> +    mc->no_sdcard = qm->no_sdcard;
> +    mc->is_default = qm->is_default;
> +    mc->default_machine_opts = qm->default_machine_opts;
> +    mc->default_boot_order = qm->default_boot_order;
> +    mc->compat_props = qm->compat_props;
> +    mc->hw_version = qm->hw_version;
>  }
>  
>  int qemu_register_machine(QEMUMachine *m)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH V2 2/5] machine: remove QEMUMachine indirection from MachineClass

[Marcel Apfelbaum]

On Thu, 2014-04-03 at 18:59 +0200, Andreas Färber wrote:
> Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
> > No need to go through qemu_machine field. Use
> > MachineClass fields directly.
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> >  device-hotplug.c |   2 +-
> >  qmp.c            |   4 +--
> >  vl.c             | 103 ++++++++++++++++++++++++++++++++-----------------------
> >  3 files changed, 63 insertions(+), 46 deletions(-)
> [...]
> > diff --git a/vl.c b/vl.c
> > index 9975e5a..96155ca 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1583,8 +1583,29 @@ MachineState *current_machine;
> >  static void machine_class_init(ObjectClass *oc, void *data)
> >  {
> >      MachineClass *mc = MACHINE_CLASS(oc);
> > -
> > -    mc->qemu_machine = data;
> 
> This looks incomplete. You should drop the qemu_machine field from
> boards.h to assure that no one is using it - pseries still is, so 4/5
> needs to be be squashed into this commit, otherwise the new fields below
> remain empty for it.
Yes, indeed, otherwise it will break bisection.
I was looking for a way to separate the changes for hw/ppc/spapr.c
into a separate patch.

Maybe I will leave both assignments (of the qemu_machine pointer and the fields)
in this patch and remove it on 5/5 (together with qemu_machine field from 
boards.h), leaving the 4/5 as it is?

Thanks,
Marcel 


> 
> Regards,
> Andreas
> 
> > +    QEMUMachine *qm = data;
> > +
> > +    mc->name = qm->name;
> > +    mc->alias = qm->alias;
> > +    mc->desc = qm->desc;
> > +    mc->init = qm->init;
> > +    mc->reset = qm->reset;
> > +    mc->hot_add_cpu = qm->hot_add_cpu;
> > +    mc->kvm_type = qm->kvm_type;
> > +    mc->block_default_type = qm->block_default_type;
> > +    mc->max_cpus = qm->max_cpus;
> > +    mc->no_serial = qm->no_serial;
> > +    mc->no_parallel = qm->no_parallel;
> > +    mc->use_virtcon = qm->use_virtcon;
> > +    mc->use_sclp = qm->use_sclp;
> > +    mc->no_floppy = qm->no_floppy;
> > +    mc->no_cdrom = qm->no_cdrom;
> > +    mc->no_sdcard = qm->no_sdcard;
> > +    mc->is_default = qm->is_default;
> > +    mc->default_machine_opts = qm->default_machine_opts;
> > +    mc->default_boot_order = qm->default_boot_order;
> > +    mc->compat_props = qm->compat_props;
> > +    mc->hw_version = qm->hw_version;
> >  }
> >  
> >  int qemu_register_machine(QEMUMachine *m)
> 

Re: [Qemu-devel] [PATCH V2 2/5] machine: remove QEMUMachine indirection from MachineClass

[Andreas Färber]

Am 03.04.2014 19:11, schrieb Marcel Apfelbaum:
> On Thu, 2014-04-03 at 18:59 +0200, Andreas Färber wrote:
>> Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
>>> diff --git a/vl.c b/vl.c
>>> index 9975e5a..96155ca 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -1583,8 +1583,29 @@ MachineState *current_machine;
>>>  static void machine_class_init(ObjectClass *oc, void *data)
>>>  {
>>>      MachineClass *mc = MACHINE_CLASS(oc);
>>> -
>>> -    mc->qemu_machine = data;
>>
>> This looks incomplete. You should drop the qemu_machine field from
>> boards.h to assure that no one is using it - pseries still is, so 4/5
>> needs to be be squashed into this commit, otherwise the new fields below
>> remain empty for it.
> Yes, indeed, otherwise it will break bisection.
> I was looking for a way to separate the changes for hw/ppc/spapr.c
> into a separate patch.
> 
> Maybe I will leave both assignments (of the qemu_machine pointer and the fields)
> in this patch and remove it on 5/5 (together with qemu_machine field from 
> boards.h), leaving the 4/5 as it is?

The trouble with 2/5 is that in generic code you start using mc->
fields, which do not get assigned by pseries. So the "mc->... = ...;"
parts of 4/5 need to go into 2/5, whether you leave qemu_machine
assigned or not.

Of course I understand your desire to not put everything into one huge
commit. Maybe reordering 3/5 before this patch using ->qemu_machine
helps? Next prepare QEMUMachineInitArgs for MachineClass (5/5). Then in
this step you can drop qemu_machine field as suggested and any
->qemu_machine gets dropped in one big but mechanical change.

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

Re: [Qemu-devel] [PATCH V2 2/5] machine: remove QEMUMachine indirection from MachineClass

[Marcel Apfelbaum]

On Thu, 2014-04-03 at 19:36 +0200, Andreas Färber wrote:
> Am 03.04.2014 19:11, schrieb Marcel Apfelbaum:
> > On Thu, 2014-04-03 at 18:59 +0200, Andreas Färber wrote:
> >> Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
> >>> diff --git a/vl.c b/vl.c
> >>> index 9975e5a..96155ca 100644
> >>> --- a/vl.c
> >>> +++ b/vl.c
> >>> @@ -1583,8 +1583,29 @@ MachineState *current_machine;
> >>>  static void machine_class_init(ObjectClass *oc, void *data)
> >>>  {
> >>>      MachineClass *mc = MACHINE_CLASS(oc);
> >>> -
> >>> -    mc->qemu_machine = data;
> >>
> >> This looks incomplete. You should drop the qemu_machine field from
> >> boards.h to assure that no one is using it - pseries still is, so 4/5
> >> needs to be be squashed into this commit, otherwise the new fields below
> >> remain empty for it.
> > Yes, indeed, otherwise it will break bisection.
> > I was looking for a way to separate the changes for hw/ppc/spapr.c
> > into a separate patch.
> > 
> > Maybe I will leave both assignments (of the qemu_machine pointer and the fields)
> > in this patch and remove it on 5/5 (together with qemu_machine field from 
> > boards.h), leaving the 4/5 as it is?
> 
> The trouble with 2/5 is that in generic code you start using mc->
> fields, which do not get assigned by pseries. So the "mc->... = ...;"
> parts of 4/5 need to go into 2/5, whether you leave qemu_machine
> assigned or not.
> 
> Of course I understand your desire to not put everything into one huge
> commit. Maybe reordering 3/5 before this patch using ->qemu_machine
> helps? Next prepare QEMUMachineInitArgs for MachineClass (5/5). Then in
> this step you can drop qemu_machine field as suggested and any
> ->qemu_machine gets dropped in one big but mechanical change.
Thanks Andreas,
I'll try to follow this path.

Marcel
> 
> Regards,
> Andreas
> 

[Qemu-devel] [PATCH V2 3/5] machine: replace QEMUMachine by MachineClass in accelerator configuration

[Marcel Apfelbaum]

This minimizes QEMUMachine usage, as part of machine QOM-ification.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 include/hw/xen/xen.h    |  2 +-
 include/qemu/typedefs.h |  1 +
 include/sysemu/kvm.h    |  2 +-
 include/sysemu/qtest.h  |  2 +-
 kvm-all.c               |  6 +++---
 kvm-stub.c              |  2 +-
 qtest.c                 |  2 +-
 vl.c                    | 10 +++++-----
 xen-all.c               |  2 +-
 xen-stub.c              |  2 +-
 10 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 9d549fc..85fda3d 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -36,7 +36,7 @@ void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
 
 qemu_irq *xen_interrupt_controller_init(void);
 
-int xen_init(QEMUMachine *machine);
+int xen_init(MachineClass *mc);
 int xen_hvm_init(MemoryRegion **ram_memory);
 void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
 
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index bf8daac..86bab12 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -31,6 +31,7 @@ typedef struct MemoryListener MemoryListener;
 typedef struct MemoryMappingList MemoryMappingList;
 
 typedef struct QEMUMachine QEMUMachine;
+typedef struct MachineClass MachineClass;
 typedef struct NICInfo NICInfo;
 typedef struct HCIInfo HCIInfo;
 typedef struct AudioState AudioState;
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0bee1e8..518655c 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -152,7 +152,7 @@ extern KVMState *kvm_state;
 
 /* external API */
 
-int kvm_init(QEMUMachine *machine);
+int kvm_init(MachineClass *mc);
 
 int kvm_has_sync_mmu(void);
 int kvm_has_vcpu_events(void);
diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index 224131f..95c9ade 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -26,7 +26,7 @@ static inline bool qtest_enabled(void)
 
 bool qtest_driver(void);
 
-int qtest_init_accel(QEMUMachine *machine);
+int qtest_init_accel(MachineClass *mc);
 void qtest_init(const char *qtest_chrdev, const char *qtest_log, Error **errp);
 
 static inline int qtest_available(void)
diff --git a/kvm-all.c b/kvm-all.c
index cd4111d..1bb979d 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1341,7 +1341,7 @@ static int kvm_max_vcpus(KVMState *s)
     return (ret) ? ret : kvm_recommended_vcpus(s);
 }
 
-int kvm_init(QEMUMachine *machine)
+int kvm_init(MachineClass *mc)
 {
     static const char upgrade_note[] =
         "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
@@ -1433,8 +1433,8 @@ int kvm_init(QEMUMachine *machine)
     }
 
     kvm_type = qemu_opt_get(qemu_get_machine_opts(), "kvm-type");
-    if (machine->kvm_type) {
-        type = machine->kvm_type(kvm_type);
+    if (mc->kvm_type) {
+        type = mc->kvm_type(kvm_type);
     } else if (kvm_type) {
         fprintf(stderr, "Invalid argument kvm-type=%s\n", kvm_type);
         goto err;
diff --git a/kvm-stub.c b/kvm-stub.c
index ccdba62..8acda86 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -34,7 +34,7 @@ int kvm_init_vcpu(CPUState *cpu)
     return -ENOSYS;
 }
 
-int kvm_init(QEMUMachine *machine)
+int kvm_init(MachineClass *mc)
 {
     return -ENOSYS;
 }
diff --git a/qtest.c b/qtest.c
index 0ac9f42..2aba20d 100644
--- a/qtest.c
+++ b/qtest.c
@@ -500,7 +500,7 @@ static void qtest_event(void *opaque, int event)
     }
 }
 
-int qtest_init_accel(QEMUMachine *machine)
+int qtest_init_accel(MachineClass *mc)
 {
     configure_icount("0");
 
diff --git a/vl.c b/vl.c
index 96155ca..a4f8a6b 100644
--- a/vl.c
+++ b/vl.c
@@ -2715,7 +2715,7 @@ static MachineClass *machine_parse(const char *name)
     exit(!name || !is_help_option(name));
 }
 
-static int tcg_init(QEMUMachine *machine)
+static int tcg_init(MachineClass *mc)
 {
     tcg_exec_init(tcg_tb_size * 1024 * 1024);
     return 0;
@@ -2725,7 +2725,7 @@ static struct {
     const char *opt_name;
     const char *name;
     int (*available)(void);
-    int (*init)(QEMUMachine *);
+    int (*init)(MachineClass *mc);
     bool *allowed;
 } accel_list[] = {
     { "tcg", "tcg", tcg_available, tcg_init, &tcg_allowed },
@@ -2734,7 +2734,7 @@ static struct {
     { "qtest", "QTest", qtest_available, qtest_init_accel, &qtest_allowed },
 };
 
-static int configure_accelerator(QEMUMachine *machine)
+static int configure_accelerator(MachineClass *mc)
 {
     const char *p;
     char buf[10];
@@ -2761,7 +2761,7 @@ static int configure_accelerator(QEMUMachine *machine)
                     continue;
                 }
                 *(accel_list[i].allowed) = true;
-                ret = accel_list[i].init(machine);
+                ret = accel_list[i].init(mc);
                 if (ret < 0) {
                     init_failed = true;
                     fprintf(stderr, "failed to initialize %s: %s\n",
@@ -4181,7 +4181,7 @@ int main(int argc, char **argv, char **envp)
         exit(0);
     }
 
-    configure_accelerator(machine);
+    configure_accelerator(machine_class);
 
     if (qtest_chrdev) {
         Error *local_err = NULL;
diff --git a/xen-all.c b/xen-all.c
index ba34739..a63b531 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -1001,7 +1001,7 @@ static void xen_exit_notifier(Notifier *n, void *data)
     xs_daemon_close(state->xenstore);
 }
 
-int xen_init(QEMUMachine *machine)
+int xen_init(MachineClass *mc)
 {
     xen_xc = xen_xc_interface_open(0, 0, 0);
     if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
diff --git a/xen-stub.c b/xen-stub.c
index 59927cb..de26583 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -47,7 +47,7 @@ qemu_irq *xen_interrupt_controller_init(void)
     return NULL;
 }
 
-int xen_init(QEMUMachine *machine)
+int xen_init(MachineClass *mc)
 {
     return -ENOSYS;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH V2 4/5] hw/ppc: remove QEMUMachine indirection

[Marcel Apfelbaum]

No need for QEMUMachine anymore because
its fields are passed to MachineClass.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 hw/ppc/spapr.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a11e121..b4ce950 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1419,19 +1419,6 @@ static int spapr_kvm_type(const char *vm_type)
     exit(1);
 }
 
-static QEMUMachine spapr_machine = {
-    .name = "pseries",
-    .desc = "pSeries Logical Partition (PAPR compliant)",
-    .is_default = 1,
-    .init = ppc_spapr_init,
-    .reset = ppc_spapr_reset,
-    .block_default_type = IF_SCSI,
-    .max_cpus = MAX_CPUS,
-    .no_parallel = 1,
-    .default_boot_order = NULL,
-    .kvm_type = spapr_kvm_type,
-};
-
 /*
  * Implementation of an interface to adjust firmware patch
  * for the bootindex property handling.
@@ -1494,7 +1481,17 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     MachineClass *mc = MACHINE_CLASS(oc);
     FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc);
 
-    mc->qemu_machine = data;
+    mc->name = "pseries";
+    mc->desc = "pSeries Logical Partition (PAPR compliant)";
+    mc->is_default = 1;
+    mc->init = ppc_spapr_init;
+    mc->reset = ppc_spapr_reset;
+    mc->block_default_type = IF_SCSI;
+    mc->max_cpus = MAX_CPUS;
+    mc->no_parallel = 1;
+    mc->default_boot_order = NULL;
+    mc->kvm_type = spapr_kvm_type;
+
     fwc->get_dev_path = spapr_get_fw_dev_path;
 }
 
@@ -1502,7 +1499,6 @@ static const TypeInfo spapr_machine_info = {
     .name          = TYPE_SPAPR_MACHINE,
     .parent        = TYPE_MACHINE,
     .class_init    = spapr_machine_class_init,
-    .class_data    = &spapr_machine,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_FW_PATH_PROVIDER },
         { }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V2 4/5] hw/ppc: remove QEMUMachine indirection

[Andreas Färber]

Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
> No need for QEMUMachine anymore because
> its fields are passed to MachineClass.

QEMUMachineInitArgs still has a QEMUMachine field that now becomes NULL?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH V2 4/5] hw/ppc: remove QEMUMachine indirection

[Marcel Apfelbaum]

On Thu, 2014-04-03 at 19:25 +0200, Andreas Färber wrote:
> Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
> > No need for QEMUMachine anymore because
> > its fields are passed to MachineClass.
> 
> QEMUMachineInitArgs still has a QEMUMachine field that now becomes NULL?
No... 
It is properly initiated right before the machine init: (No changes made)


   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 }; 
   
   current_machine->init_args = args; 
   machine->init(&current_machine->init_args);

I plan to get rid of this too, but it touches a lot of files,
I am looking for a clean way to get rid of it.

Thanks,
Marcel

> 
> Andreas
> 

Re: [Qemu-devel] [PATCH V2 4/5] hw/ppc: remove QEMUMachine indirection

[Andreas Färber]

Am 03.04.2014 19:46, schrieb Marcel Apfelbaum:
> On Thu, 2014-04-03 at 19:25 +0200, Andreas Färber wrote:
>> Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
>>> No need for QEMUMachine anymore because
>>> its fields are passed to MachineClass.
>>
>> QEMUMachineInitArgs still has a QEMUMachine field that now becomes NULL?
> No... 
> It is properly initiated right before the machine init: (No changes made)
> 
> 
>    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 }; 
>    
>    current_machine->init_args = args; 
>    machine->init(&current_machine->init_args);

You're dropping pseries QEMUMachine in this patch,
=> mc->qemu_machine = NULL
=> machine = NULL
=> .machine = NULL

Therefore my suggestion to start using MachineClass for .machine early
(because machine_class != NULL) and to do the ->qemu_machine and mc->
changes in one go (so that ->qemu_machine != NULL as long as it exists).

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

Re: [Qemu-devel] [PATCH V2 4/5] hw/ppc: remove QEMUMachine indirection

[Marcel Apfelbaum]

On Thu, 2014-04-03 at 19:57 +0200, Andreas Färber wrote:
> Am 03.04.2014 19:46, schrieb Marcel Apfelbaum:
> > On Thu, 2014-04-03 at 19:25 +0200, Andreas Färber wrote:
> >> Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
> >>> No need for QEMUMachine anymore because
> >>> its fields are passed to MachineClass.
> >>
> >> QEMUMachineInitArgs still has a QEMUMachine field that now becomes NULL?
> > No... 
> > It is properly initiated right before the machine init: (No changes made)
> > 
> > 
> >    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 }; 
> >    
> >    current_machine->init_args = args; 
> >    machine->init(&current_machine->init_args);
> 
> You're dropping pseries QEMUMachine in this patch,
> => mc->qemu_machine = NULL
> => machine = NULL
> => .machine = NULL
Right! My bad :(

> 
> Therefore my suggestion to start using MachineClass for .machine early
> (because machine_class != NULL) and to do the ->qemu_machine and mc->
> changes in one go (so that ->qemu_machine != NULL as long as it exists).
Going for it,

Thanks,
Marcel
> 
> Regards,
> Andreas
> 

[Qemu-devel] [PATCH V2 5/5] vl.c: Remove QEMUMachine usage

[Marcel Apfelbaum]

All the references of QEMUMachine are already
replaced by MachineClass.

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

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 7cf1f07..66ee98a 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -7,8 +7,10 @@
 #include "hw/qdev.h"
 #include "qom/object.h"
 
+typedef struct MachineClass MachineClass;
+
 typedef struct QEMUMachineInitArgs {
-    const QEMUMachine *machine;
+    const MachineClass *machine;
     ram_addr_t ram_size;
     const char *boot_order;
     const char *kernel_filename;
@@ -46,7 +48,6 @@ struct QEMUMachine {
     const char *default_machine_opts;
     const char *default_boot_order;
     GlobalProperty *compat_props;
-    struct QEMUMachine *next;
     const char *hw_version;
 };
 
@@ -63,7 +64,6 @@ int qemu_register_machine(QEMUMachine *m);
     OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
 
 typedef struct MachineState MachineState;
-typedef struct MachineClass MachineClass;
 
 MachineClass *find_default_machine(void);
 extern MachineState *current_machine;
@@ -77,7 +77,6 @@ struct MachineClass {
     ObjectClass parent_class;
     /*< public >*/
 
-    QEMUMachine *qemu_machine;
     const char *name;
     const char *alias;
     const char *desc;
diff --git a/vl.c b/vl.c
index a4f8a6b..190bd5d 100644
--- a/vl.c
+++ b/vl.c
@@ -2961,7 +2961,6 @@ int main(int argc, char **argv, char **envp)
     const char *optarg;
     const char *loadvm = NULL;
     MachineClass *machine_class;
-    QEMUMachine *machine = NULL;
     const char *cpu_model;
     const char *vga_model = NULL;
     const char *qtest_chrdev = NULL;
@@ -4388,7 +4387,7 @@ int main(int argc, char **argv, char **envp)
 
     qdev_machine_init();
 
-    QEMUMachineInitArgs args = { .machine = machine,
+    QEMUMachineInitArgs args = { .machine = machine_class,
                                  .ram_size = ram_size,
                                  .boot_order = boot_order,
                                  .kernel_filename = kernel_filename,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V2 5/5] vl.c: Remove QEMUMachine usage

[Andreas Färber]

Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
> All the references of QEMUMachine are already
> replaced by MachineClass.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
>  include/hw/boards.h | 7 +++----
>  vl.c                | 3 +--
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 7cf1f07..66ee98a 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -7,8 +7,10 @@
>  #include "hw/qdev.h"
>  #include "qom/object.h"
>  
> +typedef struct MachineClass MachineClass;
> +
>  typedef struct QEMUMachineInitArgs {
> -    const QEMUMachine *machine;
> +    const MachineClass *machine;
>      ram_addr_t ram_size;
>      const char *boot_order;
>      const char *kernel_filename;
> @@ -46,7 +48,6 @@ struct QEMUMachine {
>      const char *default_machine_opts;
>      const char *default_boot_order;
>      GlobalProperty *compat_props;
> -    struct QEMUMachine *next;

This field shouldn't be used in qemu.git any more since you adopted the
QOM way of iterating over the types. Could you drop this field as a very
first patch, pointing to the commit which obsoleted it?

Regards,
Andreas

>      const char *hw_version;
>  };
>  
> @@ -63,7 +64,6 @@ int qemu_register_machine(QEMUMachine *m);
>      OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
>  
>  typedef struct MachineState MachineState;
> -typedef struct MachineClass MachineClass;
>  
>  MachineClass *find_default_machine(void);
>  extern MachineState *current_machine;
> @@ -77,7 +77,6 @@ struct MachineClass {
>      ObjectClass parent_class;
>      /*< public >*/
>  
> -    QEMUMachine *qemu_machine;
>      const char *name;
>      const char *alias;
>      const char *desc;
> diff --git a/vl.c b/vl.c
> index a4f8a6b..190bd5d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2961,7 +2961,6 @@ int main(int argc, char **argv, char **envp)
>      const char *optarg;
>      const char *loadvm = NULL;
>      MachineClass *machine_class;
> -    QEMUMachine *machine = NULL;
>      const char *cpu_model;
>      const char *vga_model = NULL;
>      const char *qtest_chrdev = NULL;
> @@ -4388,7 +4387,7 @@ int main(int argc, char **argv, char **envp)
>  
>      qdev_machine_init();
>  
> -    QEMUMachineInitArgs args = { .machine = machine,
> +    QEMUMachineInitArgs args = { .machine = machine_class,
>                                   .ram_size = ram_size,
>                                   .boot_order = boot_order,
>                                   .kernel_filename = kernel_filename,
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH V2 5/5] vl.c: Remove QEMUMachine usage

[Marcel Apfelbaum]

On Thu, 2014-04-03 at 19:09 +0200, Andreas Färber wrote:
> Am 31.03.2014 11:26, schrieb Marcel Apfelbaum:
> > All the references of QEMUMachine are already
> > replaced by MachineClass.
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> >  include/hw/boards.h | 7 +++----
> >  vl.c                | 3 +--
> >  2 files changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 7cf1f07..66ee98a 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -7,8 +7,10 @@
> >  #include "hw/qdev.h"
> >  #include "qom/object.h"
> >  
> > +typedef struct MachineClass MachineClass;
> > +
> >  typedef struct QEMUMachineInitArgs {
> > -    const QEMUMachine *machine;
> > +    const MachineClass *machine;
> >      ram_addr_t ram_size;
> >      const char *boot_order;
> >      const char *kernel_filename;
> > @@ -46,7 +48,6 @@ struct QEMUMachine {
> >      const char *default_machine_opts;
> >      const char *default_boot_order;
> >      GlobalProperty *compat_props;
> > -    struct QEMUMachine *next;
> 
> This field shouldn't be used in qemu.git any more since you adopted the
> QOM way of iterating over the types. Could you drop this field as a very
> first patch, pointing to the commit which obsoleted it?
Sure,

Thanks,
Marcel

> 
> Regards,
> Andreas
> 
> >      const char *hw_version;
> >  };
> >  
> > @@ -63,7 +64,6 @@ int qemu_register_machine(QEMUMachine *m);
> >      OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
> >  
> >  typedef struct MachineState MachineState;
> > -typedef struct MachineClass MachineClass;
> >  
> >  MachineClass *find_default_machine(void);
> >  extern MachineState *current_machine;
> > @@ -77,7 +77,6 @@ struct MachineClass {
> >      ObjectClass parent_class;
> >      /*< public >*/
> >  
> > -    QEMUMachine *qemu_machine;
> >      const char *name;
> >      const char *alias;
> >      const char *desc;
> > diff --git a/vl.c b/vl.c
> > index a4f8a6b..190bd5d 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -2961,7 +2961,6 @@ int main(int argc, char **argv, char **envp)
> >      const char *optarg;
> >      const char *loadvm = NULL;
> >      MachineClass *machine_class;
> > -    QEMUMachine *machine = NULL;
> >      const char *cpu_model;
> >      const char *vga_model = NULL;
> >      const char *qtest_chrdev = NULL;
> > @@ -4388,7 +4387,7 @@ int main(int argc, char **argv, char **envp)
> >  
> >      qdev_machine_init();
> >  
> > -    QEMUMachineInitArgs args = { .machine = machine,
> > +    QEMUMachineInitArgs args = { .machine = machine_class,
> >                                   .ram_size = ram_size,
> >                                   .boot_order = boot_order,
> >                                   .kernel_filename = kernel_filename,
> > 
> 
>