diff mbox

[RFC,V2,2/9] vl: use qemu machine QOM class instead of global machines list

Message ID 1393765632-2753-3-git-send-email-marcel.a@redhat.com
State New
Headers show

Commit Message

Marcel Apfelbaum March 2, 2014, 1:07 p.m. UTC
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>
---
 include/hw/boards.h |  1 +
 vl.c                | 75 ++++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 55 insertions(+), 21 deletions(-)

Comments

Paolo Bonzini March 3, 2014, 12:57 p.m. UTC | #1
Il 03/03/2014 13:58, Michael S. Tsirkin ha scritto:
> On Sun, Mar 02, 2014 at 03:07:05PM +0200, Marcel Apfelbaum wrote:
>> > 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>
> Nice.
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
> This patch already looks like a nice cleanup. How about queueing
> this and the preceding patch straight away?
> Which tree is appropriate? I'm guessing QOM?
>

Fine by me.

Paolo
Michael S. Tsirkin March 3, 2014, 12:58 p.m. UTC | #2
On Sun, Mar 02, 2014 at 03:07:05PM +0200, Marcel Apfelbaum wrote:
> 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>

Nice.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

This patch already looks like a nice cleanup. How about queueing
this and the preceding patch straight away?
Which tree is appropriate? I'm guessing QOM?

> ---
>  include/hw/boards.h |  1 +
>  vl.c                | 75 ++++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 55 insertions(+), 21 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 7b4708d..65e1e03 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -49,6 +49,7 @@ struct QEMUMachine {
>      const char *hw_version;
>  };
>  
> +#define TYPE_QEMU_MACHINE_PREFIX "machine-"
>  int qemu_register_machine(QEMUMachine *m);
>  QEMUMachine *find_default_machine(void);
>  
> diff --git a/vl.c b/vl.c
> index 9379d33..50c880f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1529,54 +1529,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       = g_strconcat(TYPE_QEMU_MACHINE_PREFIX, m->name, NULL),
> +        .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;
> @@ -1597,6 +1624,7 @@ MachineInfoList *qmp_query_machines(Error **errp)
>          mach_list = entry;
>      }
>  
> +    g_slist_free(machines);
>      return mach_list;
>  }
>  
> @@ -2540,6 +2568,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);
> @@ -2548,13 +2577,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
Marcel Apfelbaum March 3, 2014, 1:03 p.m. UTC | #3
On Mon, 2014-03-03 at 13:57 +0100, Paolo Bonzini wrote:
> Il 03/03/2014 13:58, Michael S. Tsirkin ha scritto:
> > On Sun, Mar 02, 2014 at 03:07:05PM +0200, Marcel Apfelbaum wrote:
> >> > 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>
> > Nice.
Thanks!

> >
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > This patch already looks like a nice cleanup. How about queueing
> > this and the preceding patch straight away?
> > Which tree is appropriate? I'm guessing QOM?
> >
> 
> Fine by me.
Thanks!

Where can I find the QOM tree? I ask because I will rebase the series on this tree...
Marcel
 
> 
> Paolo
Andreas Färber March 3, 2014, 2:52 p.m. UTC | #4
Am 03.03.2014 14:03, schrieb Marcel Apfelbaum:
> On Mon, 2014-03-03 at 13:57 +0100, Paolo Bonzini wrote:
>> Il 03/03/2014 13:58, Michael S. Tsirkin ha scritto:
>>> On Sun, Mar 02, 2014 at 03:07:05PM +0200, Marcel Apfelbaum wrote:
>>>>> 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>
>>> Nice.
> Thanks!
> 
>>>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>>
>>> This patch already looks like a nice cleanup. How about queueing
>>> this and the preceding patch straight away?
>>> Which tree is appropriate? I'm guessing QOM?
>>>
>>
>> Fine by me.
> Thanks!
> 
> Where can I find the QOM tree? I ask because I will rebase the series on this tree...

github.com/afaerber/qemu-cpu qom-next

but I hope rebasing won't be necessary, just give me some more time to
look at the series.

Thanks a lot for your efforts already, it sounds very promising!

Cheers,
Andreas
Marcel Apfelbaum March 3, 2014, 3:05 p.m. UTC | #5
On Mon, 2014-03-03 at 15:52 +0100, Andreas Färber wrote:
> Am 03.03.2014 14:03, schrieb Marcel Apfelbaum:
> > On Mon, 2014-03-03 at 13:57 +0100, Paolo Bonzini wrote:
> >> Il 03/03/2014 13:58, Michael S. Tsirkin ha scritto:
> >>> On Sun, Mar 02, 2014 at 03:07:05PM +0200, Marcel Apfelbaum wrote:
> >>>>> 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>
> >>> Nice.
> > Thanks!
> > 
> >>>
> >>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> >>>
> >>> This patch already looks like a nice cleanup. How about queueing
> >>> this and the preceding patch straight away?
> >>> Which tree is appropriate? I'm guessing QOM?
> >>>
> >>
> >> Fine by me.
> > Thanks!
> > 
> > Where can I find the QOM tree? I ask because I will rebase the series on this tree...
> 
> github.com/afaerber/qemu-cpu qom-next
> 
> but I hope rebasing won't be necessary, just give me some more time to
> look at the series.
Sure, I appreciate you are going over it.

> 
> Thanks a lot for your efforts already, it sounds very promising!
Thanks! 
Marcel

> 
> Cheers,
> Andreas
>
Andreas Färber March 3, 2014, 6:12 p.m. UTC | #6
Am 02.03.2014 14:07, schrieb 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>
> ---
>  include/hw/boards.h |  1 +
>  vl.c                | 75 ++++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 55 insertions(+), 21 deletions(-)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 7b4708d..65e1e03 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -49,6 +49,7 @@ struct QEMUMachine {
>      const char *hw_version;
>  };
>  
> +#define TYPE_QEMU_MACHINE_PREFIX "machine-"

Would you mind turning that into a "-machine" suffix?

>  int qemu_register_machine(QEMUMachine *m);
>  QEMUMachine *find_default_machine(void);
>  
> diff --git a/vl.c b/vl.c
> index 9379d33..50c880f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1529,54 +1529,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)

"oc" like in 1/9 please.

> +{
> +    QemuMachineClass *k = QEMU_MACHINE_CLASS(klass);

"mc" for [QEMU]MachineClass maybe? Applies elsewhere as well.

On that matter, might we simply choose MachineState and MachineClass, or
is there some name conflict?

> +
> +    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       = g_strconcat(TYPE_QEMU_MACHINE_PREFIX, m->name, NULL),
> +        .parent     = TYPE_QEMU_MACHINE,
> +        .class_init = qemu_machine_class_init,
> +        .class_data = (void *)m,
> +    };
> +
> +    type_register(&ti);

Cute idea as minimally invasive solution!

I do wonder whether doing type_register() as part of machine_init()
callbacks could pose any timing problems - have you checked on that in
vl.c? I.e. in the worst case we might need to sweep
s/machine_init/type_init/ for all machines in this early patch already
while still calling qemu_register_machine().

Otherwise the use of QOM infrastructure looks really nice.

Cheers,
Andreas

> +
>      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;
> @@ -1597,6 +1624,7 @@ MachineInfoList *qmp_query_machines(Error **errp)
>          mach_list = entry;
>      }
>  
> +    g_slist_free(machines);
>      return mach_list;
>  }
>  
> @@ -2540,6 +2568,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);
> @@ -2548,13 +2577,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));
>  }
>
Marcel Apfelbaum March 3, 2014, 7:54 p.m. UTC | #7
On Mon, 2014-03-03 at 19:12 +0100, Andreas Färber wrote:
> Am 02.03.2014 14:07, schrieb 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>
> > ---
> >  include/hw/boards.h |  1 +
> >  vl.c                | 75 ++++++++++++++++++++++++++++++++++++++---------------
> >  2 files changed, 55 insertions(+), 21 deletions(-)
> > 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 7b4708d..65e1e03 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -49,6 +49,7 @@ struct QEMUMachine {
> >      const char *hw_version;
> >  };
> >  
> > +#define TYPE_QEMU_MACHINE_PREFIX "machine-"
> 
> Would you mind turning that into a "-machine" suffix?
Not at all

> 
> >  int qemu_register_machine(QEMUMachine *m);
> >  QEMUMachine *find_default_machine(void);
> >  
> > diff --git a/vl.c b/vl.c
> > index 9379d33..50c880f 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1529,54 +1529,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)
> 
> "oc" like in 1/9 please.
Sure
> 
> > +{
> > +    QemuMachineClass *k = QEMU_MACHINE_CLASS(klass);
> 
> "mc" for [QEMU]MachineClass maybe? Applies elsewhere as well.
As above, I couldn't follow a unified naming scheme,
your comment really help, thanks!

> 
> On that matter, might we simply choose MachineState and MachineClass, or
> is there some name conflict?
As stated in your 1/9 patch review, I find your choice great,
indeed, there is no need for the "Qemu" prefix. It mostly helped *me*
to associate old-new names, but there is no naming conflict, I'll change.

> 
> > +
> > +    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       = g_strconcat(TYPE_QEMU_MACHINE_PREFIX, m->name, NULL),
> > +        .parent     = TYPE_QEMU_MACHINE,
> > +        .class_init = qemu_machine_class_init,
> > +        .class_data = (void *)m,
> > +    };
> > +
> > +    type_register(&ti);
> 
> Cute idea as minimally invasive solution!
Thanks!

> 
> I do wonder whether doing type_register() as part of machine_init()
> callbacks could pose any timing problems - have you checked on that in
> vl.c? I.e. in the worst case we might need to sweep
> s/machine_init/type_init/ for all machines in this early patch already
> while still calling qemu_register_machine().
I encountered no problems here (meaning it works), at least I see no race:
The [Qemu]MachineClasses are created dynamically after
'module_call_init(MODULE_INIT_QOM)' is called with no connection with what happened
there, beside the creation of the ObjectClass of course, which we can count it (can we?).
Other than that, we can say that a side effect of 'module_call_init(MODULE_INIT_MACHINE)'
is the creation of the new Machine QOM classes.

Please let me know if I am missing something, in that case I'll switch as you suggested.
For the moment is how I see things: MODULE_INIT_QOM should be before MODULE_INIT_MACHINE.
 

> Otherwise the use of QOM infrastructure looks really nice.
Thanks!

If I am not exaggerating, can you please go over the patch 3/9 ?
([PATCH RFC V2 3/9] hw/boards: converted current_machine to be an instance of QemuMachineCLass)

I have a minor change to do there (add the machine to the QOM tree instead
of the /machine container) and combined with the first two, they can be a
nice feature start :) - and let me concentrate on the next phase.

Thanks again or your time!
Marcel



> 
> Cheers,
> Andreas
> 
> > +
> >      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;
> > @@ -1597,6 +1624,7 @@ MachineInfoList *qmp_query_machines(Error **errp)
> >          mach_list = entry;
> >      }
> >  
> > +    g_slist_free(machines);
> >      return mach_list;
> >  }
> >  
> > @@ -2540,6 +2568,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);
> > @@ -2548,13 +2577,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));
> >  }
> >  
>
diff mbox

Patch

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 7b4708d..65e1e03 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -49,6 +49,7 @@  struct QEMUMachine {
     const char *hw_version;
 };
 
+#define TYPE_QEMU_MACHINE_PREFIX "machine-"
 int qemu_register_machine(QEMUMachine *m);
 QEMUMachine *find_default_machine(void);
 
diff --git a/vl.c b/vl.c
index 9379d33..50c880f 100644
--- a/vl.c
+++ b/vl.c
@@ -1529,54 +1529,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       = g_strconcat(TYPE_QEMU_MACHINE_PREFIX, m->name, NULL),
+        .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;
@@ -1597,6 +1624,7 @@  MachineInfoList *qmp_query_machines(Error **errp)
         mach_list = entry;
     }
 
+    g_slist_free(machines);
     return mach_list;
 }
 
@@ -2540,6 +2568,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);
@@ -2548,13 +2577,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));
 }