On Tue, Jul 10, 2012 at 4:45 PM, Paolo Bonzini <pbonz...@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 <pingf...@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);
>>
>
>
