From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49335) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScnrE-0000zG-Hu for qemu-devel@nongnu.org; Thu, 07 Jun 2012 21:19:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ScnrC-0000eH-Bh for qemu-devel@nongnu.org; Thu, 07 Jun 2012 21:19:20 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:42867) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScnrC-0000e6-2S for qemu-devel@nongnu.org; Thu, 07 Jun 2012 21:19:18 -0400 Received: by pbbro12 with SMTP id ro12so2013523pbb.4 for ; Thu, 07 Jun 2012 18:19:16 -0700 (PDT) Message-ID: <4FD1530D.4010706@codemonkey.ws> Date: Fri, 08 Jun 2012 09:19:09 +0800 From: Anthony Liguori MIME-Version: 1.0 References: <1339097465-22977-1-git-send-email-afaerber@suse.de> <1339097465-22977-2-git-send-email-afaerber@suse.de> In-Reply-To: <1339097465-22977-2-git-send-email-afaerber@suse.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH qom-next 1/7] qdev: Push state up to Object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: pbonzini@redhat.com, qemu-devel@nongnu.org On 06/08/2012 03:30 AM, Andreas Färber wrote: > From: Paolo Bonzini > > qdev properties use the state member (an embryo of the "realized" > property) in order to disable setting them after a device has been > initialized. So, in order to push qdev properties up to Object > we need to push this bit there too. > > Signed-off-by: Paolo Bonzini > [AF: Rename to OBJECT_STATE_INITIALIZED and set it after instance_init.] > Signed-off-by: Andreas Färber > --- > hw/qdev-addr.c | 3 ++- > hw/qdev-properties.c | 26 +++++++++++++------------- > hw/qdev.c | 11 +++++------ > hw/qdev.h | 6 ------ > include/qemu/object.h | 14 ++++++++++++++ > qom/object.c | 7 +++++++ > 6 files changed, 41 insertions(+), 26 deletions(-) > > diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c > index b711b6b..a3796bd 100644 > --- a/hw/qdev-addr.c > +++ b/hw/qdev-addr.c > @@ -1,3 +1,4 @@ > +#include "qemu/object.h" > #include "qdev.h" > #include "qdev-addr.h" > #include "targphys.h" > @@ -39,7 +40,7 @@ static void set_taddr(Object *obj, Visitor *v, void *opaque, > Error *local_err = NULL; > int64_t value; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index 099a7aa..fcc0bed 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -53,7 +53,7 @@ static void set_bit(Object *obj, Visitor *v, void *opaque, > Error *local_err = NULL; > bool value; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -93,7 +93,7 @@ static void set_uint8(Object *obj, Visitor *v, void *opaque, > Property *prop = opaque; > uint8_t *ptr = qdev_get_prop_ptr(dev, prop); > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -160,7 +160,7 @@ static void set_uint16(Object *obj, Visitor *v, void *opaque, > Property *prop = opaque; > uint16_t *ptr = qdev_get_prop_ptr(dev, prop); > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -193,7 +193,7 @@ static void set_uint32(Object *obj, Visitor *v, void *opaque, > Property *prop = opaque; > uint32_t *ptr = qdev_get_prop_ptr(dev, prop); > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -218,7 +218,7 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, > Property *prop = opaque; > int32_t *ptr = qdev_get_prop_ptr(dev, prop); > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -291,7 +291,7 @@ static void set_uint64(Object *obj, Visitor *v, void *opaque, > Property *prop = opaque; > uint64_t *ptr = qdev_get_prop_ptr(dev, prop); > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -379,7 +379,7 @@ static void set_string(Object *obj, Visitor *v, void *opaque, > Error *local_err = NULL; > char *str; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -457,7 +457,7 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop, > char *str; > int ret; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -626,7 +626,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque, > int64_t id; > VLANState *vlan; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -696,7 +696,7 @@ static void set_mac(Object *obj, Visitor *v, void *opaque, > int i, pos; > char *str, *p; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -766,7 +766,7 @@ static void set_enum(Object *obj, Visitor *v, void *opaque, > Property *prop = opaque; > int *ptr = qdev_get_prop_ptr(dev, prop); > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -797,7 +797,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque, > Error *local_err = NULL; > char *str; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -867,7 +867,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque, > const int64_t min = 512; > const int64_t max = 32768; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > diff --git a/hw/qdev.c b/hw/qdev.c > index b20b34d..c12e151 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -155,7 +155,7 @@ int qdev_init(DeviceState *dev) > DeviceClass *dc = DEVICE_GET_CLASS(dev); > int rc; > > - assert(dev->state == DEV_STATE_CREATED); > + assert(!object_is_realized(OBJECT(dev))); > > rc = dc->init(dev); > if (rc< 0) { > @@ -178,7 +178,7 @@ int qdev_init(DeviceState *dev) > dev->instance_id_alias, > dev->alias_required_for_version); > } > - dev->state = DEV_STATE_INITIALIZED; > + OBJECT(dev)->state = OBJECT_STATE_REALIZED; > if (dev->hotplugged) { > device_reset(dev); > } > @@ -188,7 +188,7 @@ int qdev_init(DeviceState *dev) > void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > int required_for_version) > { > - assert(dev->state == DEV_STATE_CREATED); > + assert(!object_is_realized(OBJECT(dev))); > dev->instance_id_alias = alias_id; > dev->alias_required_for_version = required_for_version; > } > @@ -567,7 +567,7 @@ static void qdev_set_legacy_property(Object *obj, Visitor *v, void *opaque, > char *ptr = NULL; > int ret; > > - if (dev->state != DEV_STATE_CREATED) { > + if (object_is_realized(obj)) { > error_set(errp, QERR_PERMISSION_DENIED); > return; > } > @@ -674,7 +674,6 @@ static void device_initfn(Object *obj) > } > > dev->instance_id_alias = -1; > - dev->state = DEV_STATE_CREATED; > > class = object_get_class(OBJECT(dev)); > do { > @@ -697,7 +696,7 @@ static void device_finalize(Object *obj) > BusState *bus; > DeviceClass *dc = DEVICE_GET_CLASS(dev); > > - if (dev->state == DEV_STATE_INITIALIZED) { > + if (object_is_realized(obj)) { > while (dev->num_child_bus) { > bus = QLIST_FIRST(&dev->child_bus); > qbus_free(bus); > diff --git a/hw/qdev.h b/hw/qdev.h > index ae1d281..a1e40c5 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h > @@ -19,11 +19,6 @@ typedef struct BusState BusState; > > typedef struct BusClass BusClass; > > -enum DevState { > - DEV_STATE_CREATED = 1, > - DEV_STATE_INITIALIZED, > -}; > - > enum { > DEV_NVECTORS_UNSPECIFIED = -1, > }; > @@ -64,7 +59,6 @@ struct DeviceState { > Object parent_obj; > > const char *id; > - enum DevState state; > QemuOpts *opts; > int hotplugged; > BusState *parent_bus; > diff --git a/include/qemu/object.h b/include/qemu/object.h > index 8b17776..1606777 100644 > --- a/include/qemu/object.h > +++ b/include/qemu/object.h > @@ -241,6 +241,11 @@ struct ObjectClass > Type type; > }; > > +typedef enum ObjectState { > + OBJECT_STATE_INITIALIZED = 1, > + OBJECT_STATE_REALIZED, > +} ObjectState; I think using a bool would be better since it reduces the temptation to add additional states. Regards, Anthony Liguori