From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57414) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoW4o-0005XD-2r for qemu-devel@nongnu.org; Tue, 10 Jul 2012 04:45:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SoW4l-0003bJ-RY for qemu-devel@nongnu.org; Tue, 10 Jul 2012 04:45:45 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:54130) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoW4l-0003at-E8 for qemu-devel@nongnu.org; Tue, 10 Jul 2012 04:45:43 -0400 Received: by pbbro12 with SMTP id ro12so22250146pbb.4 for ; Tue, 10 Jul 2012 01:45:41 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4FFBEBAE.7080200@redhat.com> Date: Tue, 10 Jul 2012 10:45:34 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1341900967-4344-1-git-send-email-qemulist@gmail.com> <1341900967-4344-2-git-send-email-qemulist@gmail.com> In-Reply-To: <1341900967-4344-2-git-send-email-qemulist@gmail.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/5] qdev: introduce qdev_create_kid(Object *parent, const char *type) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liu Ping Fan Cc: Blue Swirl , Jan Kiszka , qemu-devel@nongnu.org, Anthony Liguori , =?ISO-8859-15?Q?Andreas_F=E4rber?= Il 10/07/2012 08:16, Liu Ping Fan ha scritto: > DeviceState can be created as kid of DeviceState/CPUState, not neccesary > attached to bus. This will be helpful to simulate the real hardware > submodule which sits inside package. > > Signed-off-by: Liu Ping Fan > --- > hw/qdev.c | 28 ++++++++++++++++++++++++++++ > hw/qdev.h | 1 + > 2 files changed, 29 insertions(+), 0 deletions(-) > > diff --git a/hw/qdev.c b/hw/qdev.c > index af54467..d2100a1 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -26,6 +26,7 @@ > this API directly. */ > > #include "net.h" > +#include "qemu/cpu.h" > #include "qdev.h" > #include "sysemu.h" > #include "error.h" > @@ -145,6 +146,33 @@ DeviceState *qdev_try_create(BusState *bus, const char *type) > return dev; > } > > +DeviceState *qdev_create_kid(Object *parent, const char *type) > +{ > + DeviceState *dev; > + assert(parent); > + > + if (object_class_by_name(type) == NULL) { > + return NULL; > + } > + > + if (object_is_type_str(parent, TYPE_BUS)) { > + return qdev_create(BUS(parent), type); > + } > + > + if (!object_is_type_str(parent, TYPE_DEVICE) > + || !object_is_type_str(parent, TYPE_CPU)) { > + return NULL; > + } > + > + dev = DEVICE(object_new(type)); > + if (!dev) { > + return NULL; > + } > + object_property_add_child(OBJECT(parent), type, OBJECT(dev), NULL); I don't like this. The only additional functionality is "magic" dispatching between qdev_create for buses and object_property_add_child for devices. This should be done with a method that is implemented in both objects (e.g. an interface), not with type checks like this. However, you're not even using the functionality, and designing APIs without an effective need usually makes for bad APIs. Instead, you can just move APIC creation in the CPU, and use object_property_add_child there. Paolo > + return dev; > +} > + > /* Initialize a device. Device properties should be set before calling > this function. IRQs and MMIO regions should be connected/mapped after > calling this function. > diff --git a/hw/qdev.h b/hw/qdev.h > index f4683dc..aecc69e 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h > @@ -154,6 +154,7 @@ typedef struct GlobalProperty { > > DeviceState *qdev_create(BusState *bus, const char *name); > DeviceState *qdev_try_create(BusState *bus, const char *name); > +DeviceState *qdev_create_kid(Object *parent, const char *type); > bool qdev_exists(const char *name); > int qdev_device_help(QemuOpts *opts); > DeviceState *qdev_device_add(QemuOpts *opts); >