| Message ID | 1341900967-4344-2-git-send-email-qemulist@gmail.com |
|---|---|
| State | New |
| Headers | show |
Am 10.07.2012 08:16, schrieb Liu Ping Fan: > 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 <pingfank@linux.vnet.ibm.com> > --- > 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)) { This is only introduced in patch 2, no? Andreas > + 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); > + > + 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); >
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 <pingfank@linux.vnet.ibm.com> > --- > 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); >
On Tue, Jul 10, 2012 at 4:45 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > 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 <pingfank@linux.vnet.ibm.com> >> --- >> 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. > OK, I will move the creation in the CPU. But I think as part of qom, DeviceState can have a DeviceState child, so there is need for wrapper for the function. Maybe just make the qdev_create_kid(Object*) -> qdev_create_kid(DeviceState*) ? regards, pingfan > 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); >> > >
Il 11/07/2012 03:17, liu ping fan ha scritto: > OK, I will move the creation in the CPU. But I think as part of qom, > DeviceState can have a DeviceState child, so there is need for wrapper > for the function. Maybe just make the qdev_create_kid(Object*) -> > qdev_create_kid(DeviceState*) ? You can just use object_property_add_child whenever you would use qdev_create_kid. Paolo
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); + + 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);
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 <pingfank@linux.vnet.ibm.com> --- hw/qdev.c | 28 ++++++++++++++++++++++++++++ hw/qdev.h | 1 + 2 files changed, 29 insertions(+), 0 deletions(-)