qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: "Andreas Färber" <afaerber@suse.de>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH qom-next 1/7] qdev: Push state up to Object
Date: Fri, 08 Jun 2012 09:19:09 +0800	[thread overview]
Message-ID: <4FD1530D.4010706@codemonkey.ws> (raw)
In-Reply-To: <1339097465-22977-2-git-send-email-afaerber@suse.de>

On 06/08/2012 03:30 AM, Andreas Färber wrote:
> From: Paolo Bonzini<pbonzini@redhat.com>
>
> 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<pbonzini@redhat.com>
> [AF: Rename to OBJECT_STATE_INITIALIZED and set it after instance_init.]
> Signed-off-by: Andreas Färber<afaerber@suse.de>
> ---
>   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

  reply	other threads:[~2012-06-08  1:19 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-07 19:30 [Qemu-devel] [PATCH qom-next 0/7] QOM realize, revised Andreas Färber
2012-06-07 19:30 ` [Qemu-devel] [PATCH qom-next 1/7] qdev: Push state up to Object Andreas Färber
2012-06-08  1:19   ` Anthony Liguori [this message]
2012-06-10 15:49     ` Paolo Bonzini
2012-06-10 17:35       ` Anthony Liguori
2012-06-10 17:38       ` Andreas Färber
2012-06-11  8:25         ` Kevin Wolf
2012-06-11 13:21           ` Anthony Liguori
2012-06-11 14:38             ` Kevin Wolf
2012-06-11 21:31             ` Andreas Färber
2012-06-11 21:43               ` Andreas Färber
2012-06-11 21:48               ` Anthony Liguori
2012-06-12  0:14                 ` Andreas Färber
2012-06-07 19:31 ` [Qemu-devel] [PATCH qom-next 2/7] qom: Add get_id Andreas Färber
2012-06-08  1:22   ` Anthony Liguori
2012-06-08  7:11     ` Andreas Färber
2012-06-08  7:44       ` Anthony Liguori
2012-06-08  8:17         ` Andreas Färber
2012-06-08 10:59           ` [Qemu-devel] [libvirt] " Daniel P. Berrange
2012-06-08 11:58         ` [Qemu-devel] " Eric Blake
2012-06-07 19:31 ` [Qemu-devel] [PATCH qom-next 3/7] qdev: Generalize properties to Objects Andreas Färber
2012-06-08  1:23   ` Anthony Liguori
2012-06-07 19:31 ` [Qemu-devel] [PATCH qom-next 4/7] qdev: Move bulk of qdev-properties.c to qom/object-properties.c Andreas Färber
2012-06-07 23:23   ` Paolo Bonzini
2012-06-08  1:26   ` Anthony Liguori
2012-06-07 19:31 ` [Qemu-devel] [PATCH qom-next 5/7] qom: Push static properties to Object Andreas Färber
2012-06-08  1:26   ` Anthony Liguori
2012-06-07 19:31 ` [Qemu-devel] [PATCH qom-next 6/7] qom: Add "realized" property Andreas Färber
2012-06-08  1:26   ` Anthony Liguori
2012-06-07 19:31 ` [Qemu-devel] [PATCH qom-next 7/7] qom: Add QERR_PROPERTY_SET_AFTER_REALIZE Andreas Färber
2012-06-07 19:56   ` Andreas Färber
2012-06-07 23:22 ` [Qemu-devel] [PATCH qom-next 0/7] QOM realize, revised Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FD1530D.4010706@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=afaerber@suse.de \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).