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

["Michael S. Tsirkin" <mst@redhat.com>]

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