From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60412) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKXM2-0002Cs-KM for qemu-devel@nongnu.org; Mon, 03 Mar 2014 13:12:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WKXLv-0005iZ-8v for qemu-devel@nongnu.org; Mon, 03 Mar 2014 13:12:42 -0500 Received: from cantor2.suse.de ([195.135.220.15]:42948 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKXLu-0005i5-VX for qemu-devel@nongnu.org; Mon, 03 Mar 2014 13:12:35 -0500 Message-ID: <5314C610.2090305@suse.de> Date: Mon, 03 Mar 2014 19:12:32 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1393765632-2753-1-git-send-email-marcel.a@redhat.com> <1393765632-2753-3-git-send-email-marcel.a@redhat.com> In-Reply-To: <1393765632-2753-3-git-send-email-marcel.a@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC V2 2/9] vl: use qemu machine QOM class instead of global machines list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, peter.crosthwaite@petalogix.com, ehabkost@redhat.com, mst@redhat.com, mtosatti@redhat.com, edgar.iglesias@gmail.com, mdroth@linux.vnet.ibm.com, armbru@redhat.com, blauwirbel@gmail.com, quintela@redhat.com, agraf@suse.de, aliguori@amazon.com, pbonzini@redhat.com, scottwood@freescale.com, imammedo@redhat.com, lcapitulino@redhat.com, rth@twiddle.net Am 02.03.2014 14:07, schrieb Marcel Apfelbaum: > The machine registration flow is refactored to use the QOM functionalit= y. > Instead of linking the machines into a list, each machine has a type > and the types can be traversed in the QOM way. >=20 > Signed-off-by: Marcel Apfelbaum > --- > include/hw/boards.h | 1 + > vl.c | 75 ++++++++++++++++++++++++++++++++++++++-------= -------- > 2 files changed, 55 insertions(+), 21 deletions(-) >=20 > 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; > }; > =20 > +#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); > =20 > 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 *qdi= ct) > /***********************************************************/ > /* machine registration */ > =20 > -static QEMUMachine *first_machine =3D NULL; > QEMUMachine *current_machine =3D NULL; > =20 > +static void qemu_machine_class_init(ObjectClass *klass, void *data) "oc" like in 1/9 please. > +{ > + QemuMachineClass *k =3D 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 =3D data; > +} > + > int qemu_register_machine(QEMUMachine *m) > { > - QEMUMachine **pm; > - pm =3D &first_machine; > - while (*pm !=3D NULL) > - pm =3D &(*pm)->next; > - m->next =3D NULL; > - *pm =3D m; > + TypeInfo ti =3D { > + .name =3D g_strconcat(TYPE_QEMU_MACHINE_PREFIX, m->name,= NULL), > + .parent =3D TYPE_QEMU_MACHINE, > + .class_init =3D qemu_machine_class_init, > + .class_data =3D (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; > } > =20 > static QEMUMachine *find_machine(const char *name) > { > - QEMUMachine *m; > + GSList *el, *machines =3D object_class_get_list(TYPE_QEMU_MACHINE,= false); > + QEMUMachine *m =3D NULL; > + > + for (el =3D machines; el; el =3D el->next) { > + QemuMachineClass *k =3D el->data; > =20 > - for(m =3D first_machine; m !=3D NULL; m =3D 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 =3D k->qemu_machine; > + break; > + } > + if (k->qemu_machine->alias && !strcmp(k->qemu_machine->alias, = name)) { > + m =3D k->qemu_machine; > + break; > + } > } > - return NULL; > + > + g_slist_free(machines); > + return m; > } > =20 > QEMUMachine *find_default_machine(void) > { > - QEMUMachine *m; > + GSList *el, *machines =3D object_class_get_list(TYPE_QEMU_MACHINE,= false); > + QEMUMachine *m =3D NULL; > =20 > - for(m =3D first_machine; m !=3D NULL; m =3D m->next) { > - if (m->is_default) { > - return m; > + for (el =3D machines; el; el =3D el->next) { > + QemuMachineClass *k =3D el->data; > + > + if (k->qemu_machine->is_default) { > + m =3D k->qemu_machine; > + break; > } > } > - return NULL; > + > + g_slist_free(machines); > + return m; > } > =20 > MachineInfoList *qmp_query_machines(Error **errp) > { > + GSList *el, *machines =3D object_class_get_list(TYPE_QEMU_MACHINE,= false); > MachineInfoList *mach_list =3D NULL; > QEMUMachine *m; > =20 > - for (m =3D first_machine; m; m =3D m->next) { > + for (el =3D machines; el; el =3D el->next) { > + QemuMachineClass *k =3D el->data; > MachineInfoList *entry; > MachineInfo *info; > =20 > + m =3D k->qemu_machine; > info =3D g_malloc0(sizeof(*info)); > if (m->is_default) { > info->has_is_default =3D true; > @@ -1597,6 +1624,7 @@ MachineInfoList *qmp_query_machines(Error **errp) > mach_list =3D entry; > } > =20 > + g_slist_free(machines); > return mach_list; > } > =20 > @@ -2540,6 +2568,7 @@ static int debugcon_parse(const char *devname) > static QEMUMachine *machine_parse(const char *name) > { > QEMUMachine *m, *machine =3D NULL; > + GSList *el, *machines =3D object_class_get_list(TYPE_QEMU_MACHINE,= false); > =20 > if (name) { > machine =3D find_machine(name); > @@ -2548,13 +2577,17 @@ static QEMUMachine *machine_parse(const char *n= ame) > return machine; > } > printf("Supported machines are:\n"); > - for (m =3D first_machine; m !=3D NULL; m =3D m->next) { > + for (el =3D machines; el; el =3D el->next) { > + QemuMachineClass *k =3D el->data; > + m =3D k->qemu_machine; > if (m->alias) { > printf("%-20s %s (alias of %s)\n", m->alias, m->desc, m->n= ame); > } > printf("%-20s %s%s\n", m->name, m->desc, > m->is_default ? " (default)" : ""); > } > + > + g_slist_free(machines); > exit(!name || !is_help_option(name)); > } > =20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg