From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53663) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPIng-0003H6-1i for qemu-devel@nongnu.org; Tue, 01 May 2012 15:31:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SPInd-0005eW-IQ for qemu-devel@nongnu.org; Tue, 01 May 2012 15:31:51 -0400 Received: from cantor2.suse.de ([195.135.220.15]:37535 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPInd-0005eQ-8c for qemu-devel@nongnu.org; Tue, 01 May 2012 15:31:49 -0400 Message-ID: <4FA03A22.9050406@suse.de> Date: Tue, 01 May 2012 21:31:46 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1335896294-9530-1-git-send-email-aliguori@us.ibm.com> <1335896294-9530-9-git-send-email-aliguori@us.ibm.com> In-Reply-To: <1335896294-9530-9-git-send-email-aliguori@us.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 08/14] qdev: convert busses to QEMU Object Model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Wanpeng Li , Peter Maydell , qemu-devel@nongnu.org, Paolo Bonzini Am 01.05.2012 20:18, schrieb Anthony Liguori: > This is far less interesting than it sounds. We simply add an Object t= o each > BusInfo and then register the types appropriately. Most of the interes= ting > refactoring will follow in the next patches. >=20 > Since we're changing fundamental type names (BusInfo -> BusClass), it a= ll needs > to convert at once. Fortunately, not a lot of code is affected. >=20 > Signed-off-by: Anthony Liguori > --- [...] > diff --git a/hw/intel-hda.c b/hw/intel-hda.c > index 9405f54..58497e5 100644 > --- a/hw/intel-hda.c > +++ b/hw/intel-hda.c > @@ -29,16 +29,19 @@ > /* -------------------------------------------------------------------= -- */ > /* hda bus = */ > =20 > -static struct BusInfo hda_codec_bus_info =3D { > - .name =3D "HDA", > - .size =3D sizeof(HDACodecBus), > +#define TYPE_HDA_BUS "HDA" I stumbled over this being a pretty generic term for a type name. "HDA-codec-bus" maybe? > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c > index f5ee166..97673dd 100644 > --- a/hw/qdev-monitor.c > +++ b/hw/qdev-monitor.c [...] > @@ -432,16 +433,16 @@ DeviceState *qdev_device_add(QemuOpts *opts) > if (!bus) { > return NULL; > } > - if (bus->info !=3D k->bus_info) { > + if (strcmp(object_get_typename(OBJECT(bus)), k->bus_type) !=3D= 0){ Space before {. > diff --git a/hw/qdev.h b/hw/qdev.h > index 6d2261f..8ac703e 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h [...] > @@ -79,28 +79,30 @@ struct DeviceState { > int alias_required_for_version; > }; > =20 > -typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int in= dent); > -typedef char *(*bus_get_dev_path)(DeviceState *dev); > /* > * This callback is used to create Open Firmware device path in accord= ance with > * OF spec http://forthworks.com/standards/of1275.pdf. Indicidual bus = bindings > * can be found here http://playground.sun.com/1275/bindings/. > */ > -typedef char *(*bus_get_fw_dev_path)(DeviceState *dev); > -typedef int (qbus_resetfn)(BusState *bus); > =20 > -struct BusInfo { > - const char *name; > - size_t size; > - bus_dev_printfn print_dev; > - bus_get_dev_path get_dev_path; > - bus_get_fw_dev_path get_fw_dev_path; > - qbus_resetfn *reset; > +#define TYPE_BUS "bus" > +#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS) > +#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BU= S) > +#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS) > + > +struct BusClass { > + ObjectClass parent_class; > + > + /* FIXME first arg should be BusState */ > + void (*print_dev)(Monitor *mon, DeviceState *dev, int indent); > + char *(*get_dev_path)(DeviceState *dev); > + char *(*get_fw_dev_path)(DeviceState *dev); > + int (*reset)(BusState *bus); > }; > =20 > struct BusState { > + Object obj; > DeviceState *parent; > - BusInfo *info; > const char *name; > int allow_hotplug; > int qdev_allocated; Indicate what is /*< private >*/ and what not? > @@ -321,7 +323,7 @@ void error_set_from_qdev_prop_error(Error **errp, i= nt ret, DeviceState *dev, > char *qdev_get_fw_dev_path(DeviceState *dev); > =20 > /* This is a nasty hack to allow passing a NULL bus to qdev_create. *= / > -extern struct BusInfo system_bus_info; > +#define TYPE_SYSTEM_BUS "System" "System-bus"? Regarding the bus names, these have been merely moved here but I'm raising the topic because they are now in the global name space where they will potentially conflict with devices and other types. Otherwise looks okay, thanks for your work on this. Andreas --=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