* [Qemu-devel] [PATCH v3 1/2] qdev: Fold state enum into bool realized
2013-01-09 2:58 [Qemu-devel] [PATCH v3 0/2] QOM realize, device-only Andreas Färber
@ 2013-01-09 2:58 ` Andreas Färber
2013-01-09 2:58 ` [Qemu-devel] [PATCH v3 2/2] qdev: Prepare "realized" property Andreas Färber
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Andreas Färber @ 2013-01-09 2:58 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost, anthony, Andreas Färber
Whether the device was initialized or not is QOM-level information and
currently unused. Drop it from device. This leaves the boolean state of
whether or not DeviceClass::init was called or not, a.k.a. "realized".
Suggested-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
hw/qdev-addr.c | 2 +-
hw/qdev-core.h | 18 ++++++++++--------
hw/qdev-properties-system.c | 4 ++--
hw/qdev-properties.c | 24 ++++++++++++------------
hw/qdev.c | 12 ++++++------
5 Dateien geändert, 31 Zeilen hinzugefügt(+), 29 Zeilen entfernt(-)
diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index 3bfe101..b4388f6 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -40,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index fdf14ec..494902d 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -8,11 +8,6 @@
#include "hw/irq.h"
#include "qapi/error.h"
-enum DevState {
- DEV_STATE_CREATED = 1,
- DEV_STATE_INITIALIZED,
-};
-
enum {
DEV_NVECTORS_UNSPECIFIED = -1,
};
@@ -49,13 +44,20 @@ typedef struct DeviceClass {
const char *bus_type;
} DeviceClass;
-/* This structure should not be accessed directly. We declare it here
- so that it can be embedded in individual device state structures. */
+/**
+ * DeviceState:
+ * @realized: Indicates whether the device has been fully constructed.
+ *
+ * This structure should not be accessed directly. We declare it here
+ * so that it can be embedded in individual device state structures.
+ */
struct DeviceState {
+ /*< private >*/
Object parent_obj;
+ /*< public >*/
const char *id;
- enum DevState state;
+ bool realized;
QemuOpts *opts;
int hotplugged;
BusState *parent_bus;
diff --git a/hw/qdev-properties-system.c b/hw/qdev-properties-system.c
index c73c713..ce0f793 100644
--- a/hw/qdev-properties-system.c
+++ b/hw/qdev-properties-system.c
@@ -42,7 +42,7 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop,
char *str;
int ret;
- if (dev->state != DEV_STATE_CREATED) {
+ if (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -254,7 +254,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
int32_t id;
NetClientState *hubport;
- if (dev->state != DEV_STATE_CREATED) {
+ if (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index f724357..a8a31f5 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -32,7 +32,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -85,7 +85,7 @@ static void set_bit(Object *obj, Visitor *v, void *opaque,
Error *local_err = NULL;
bool value;
- if (dev->state != DEV_STATE_CREATED) {
+ if (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -125,7 +125,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -192,7 +192,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -225,7 +225,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -250,7 +250,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -323,7 +323,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -413,7 +413,7 @@ static void set_string(Object *obj, Visitor *v, void *opaque,
Error *local_err = NULL;
char *str;
- if (dev->state != DEV_STATE_CREATED) {
+ if (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -477,7 +477,7 @@ static void set_mac(Object *obj, Visitor *v, void *opaque,
int i, pos;
char *str, *p;
- if (dev->state != DEV_STATE_CREATED) {
+ if (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -569,7 +569,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -640,7 +640,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -708,7 +708,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, void *opaque,
unsigned long dom = 0, bus = 0;
unsigned int slot = 0, func = 0;
- if (dev->state != DEV_STATE_CREATED) {
+ if (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
diff --git a/hw/qdev.c b/hw/qdev.c
index e2a5c57..07eef5c 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -151,7 +151,7 @@ int qdev_init(DeviceState *dev)
DeviceClass *dc = DEVICE_GET_CLASS(dev);
int rc;
- assert(dev->state == DEV_STATE_CREATED);
+ assert(!dev->realized);
rc = dc->init(dev);
if (rc < 0) {
@@ -174,7 +174,7 @@ int qdev_init(DeviceState *dev)
dev->instance_id_alias,
dev->alias_required_for_version);
}
- dev->state = DEV_STATE_INITIALIZED;
+ dev->realized = true;
if (dev->hotplugged) {
device_reset(dev);
}
@@ -184,7 +184,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(!dev->realized);
dev->instance_id_alias = alias_id;
dev->alias_required_for_version = required_for_version;
}
@@ -541,7 +541,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 (dev->realized) {
error_set(errp, QERR_PERMISSION_DENIED);
return;
}
@@ -648,7 +648,7 @@ static void device_initfn(Object *obj)
}
dev->instance_id_alias = -1;
- dev->state = DEV_STATE_CREATED;
+ dev->realized = false;
class = object_get_class(OBJECT(dev));
do {
@@ -671,7 +671,7 @@ static void device_finalize(Object *obj)
BusState *bus;
DeviceClass *dc = DEVICE_GET_CLASS(dev);
- if (dev->state == DEV_STATE_INITIALIZED) {
+ if (dev->realized) {
while (dev->num_child_bus) {
bus = QLIST_FIRST(&dev->child_bus);
qbus_free(bus);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH v3 2/2] qdev: Prepare "realized" property
2013-01-09 2:58 [Qemu-devel] [PATCH v3 0/2] QOM realize, device-only Andreas Färber
2013-01-09 2:58 ` [Qemu-devel] [PATCH v3 1/2] qdev: Fold state enum into bool realized Andreas Färber
@ 2013-01-09 2:58 ` Andreas Färber
2013-01-14 14:35 ` [Qemu-devel] [PATCH v3 0/2] QOM realize, device-only Igor Mammedov
2013-01-16 1:43 ` Anthony Liguori
3 siblings, 0 replies; 5+ messages in thread
From: Andreas Färber @ 2013-01-09 2:58 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, ehabkost, anthony, Andreas Färber
Introduce the QOM realizefn suggested by Anthony.
Detailed documentation is supplied in the qdev header.
For now this implements a default DeviceClass::realize callback that
just wraps DeviceClass::init, which it deprecates.
Once all devices have been converted to DeviceClass::realize,
DeviceClass::init is to be removed.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <anthony@codemonkey.ws>
---
hw/qdev-core.h | 52 +++++++++++++++++++++++++++++-
hw/qdev.c | 96 ++++++++++++++++++++++++++++++++++++++++++--------------
2 Dateien geändert, 123 Zeilen hinzugefügt(+), 25 Zeilen entfernt(-)
diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index 494902d..487a2bf 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -20,11 +20,59 @@ enum {
typedef int (*qdev_initfn)(DeviceState *dev);
typedef int (*qdev_event)(DeviceState *dev);
typedef void (*qdev_resetfn)(DeviceState *dev);
+typedef void (*DeviceRealize)(DeviceState *dev, Error **errp);
+typedef void (*DeviceUnrealize)(DeviceState *dev, Error **errp);
struct VMStateDescription;
+/**
+ * DeviceClass:
+ * @props: Properties accessing state fields.
+ * @realize: Callback function invoked when the #DeviceState:realized
+ * property is changed to %true. The default invokes @init if not %NULL.
+ * @unrealize: Callback function invoked when the #DeviceState:realized
+ * property is changed to %false.
+ * @init: Callback function invoked when the #DeviceState::realized property
+ * is changed to %true. Deprecated, new types inheriting directly from
+ * TYPE_DEVICE should use @realize instead, new leaf types should consult
+ * their respective parent type.
+ *
+ * # Realization #
+ * Devices are constructed in two stages,
+ * 1) object instantiation via object_initialize() and
+ * 2) device realization via #DeviceState:realized property.
+ * The former may not fail (it might assert or exit), the latter may return
+ * error information to the caller and must be re-entrant.
+ * Trivial field initializations should go into #TypeInfo.instance_init.
+ * Operations depending on @props static properties should go into @realize.
+ * After successful realization, setting static properties will fail.
+ *
+ * As an interim step, the #DeviceState:realized property is set by deprecated
+ * functions qdev_init() and qdev_init_nofail().
+ * In the future, devices will propagate this state change to their children
+ * and along busses they expose.
+ * The point in time will be deferred to machine creation, so that values
+ * set in @realize will not be introspectable beforehand. Therefore devices
+ * must not create children during @realize; they should initialize them via
+ * object_initialize() in their own #TypeInfo.instance_init and forward the
+ * realization events appropriately.
+ *
+ * The @init callback is considered private to a particular bus implementation
+ * (immediate abstract child types of TYPE_DEVICE). Derived leaf types set an
+ * "init" callback on their parent class instead.
+ * Any type may override the @realize and/or @unrealize callbacks but needs
+ * to call (and thus save) the parent type's implementation if so desired.
+ * Usually this means storing the previous value of, e.g., @realized inside
+ * the type's class structure and overwriting it with a function that first
+ * invokes the stored callback, then performs any additional steps.
+ * If a type derived directly from TYPE_DEVICE implements @realize, it does
+ * not need to implement @init and therefore does not need to store and call
+ * #DeviceClass' default @realize callback.
+ */
typedef struct DeviceClass {
+ /*< private >*/
ObjectClass parent_class;
+ /*< public >*/
const char *fw_name;
const char *desc;
@@ -33,12 +81,14 @@ typedef struct DeviceClass {
/* callbacks */
void (*reset)(DeviceState *dev);
+ DeviceRealize realize;
+ DeviceUnrealize unrealize;
/* device state */
const struct VMStateDescription *vmsd;
/* Private to qdev / bus. */
- qdev_initfn init;
+ qdev_initfn init; /* TODO remove, once users are converted to realize */
qdev_event unplug;
qdev_event exit;
const char *bus_type;
diff --git a/hw/qdev.c b/hw/qdev.c
index 07eef5c..13697c8 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -148,37 +148,30 @@ DeviceState *qdev_try_create(BusState *bus, const char *type)
Return 0 on success. */
int qdev_init(DeviceState *dev)
{
- DeviceClass *dc = DEVICE_GET_CLASS(dev);
- int rc;
+ Error *local_err = NULL;
assert(!dev->realized);
- rc = dc->init(dev);
- if (rc < 0) {
+ object_property_set_bool(OBJECT(dev), true, "realized", &local_err);
+ if (local_err != NULL) {
+ error_free(local_err);
qdev_free(dev);
- return rc;
+ return -1;
}
+ return 0;
+}
- if (!OBJECT(dev)->parent) {
- static int unattached_count = 0;
- gchar *name = g_strdup_printf("device[%d]", unattached_count++);
-
- object_property_add_child(container_get(qdev_get_machine(),
- "/unattached"),
- name, OBJECT(dev), NULL);
- g_free(name);
- }
+static void device_realize(DeviceState *dev, Error **err)
+{
+ DeviceClass *dc = DEVICE_GET_CLASS(dev);
- if (qdev_get_vmsd(dev)) {
- vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
- dev->instance_id_alias,
- dev->alias_required_for_version);
- }
- dev->realized = true;
- if (dev->hotplugged) {
- device_reset(dev);
+ if (dc->init) {
+ int rc = dc->init(dev);
+ if (rc < 0) {
+ error_setg(err, "Device initialization failed.");
+ return;
+ }
}
- return 0;
}
void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
@@ -636,6 +629,55 @@ void qdev_property_add_static(DeviceState *dev, Property *prop,
assert_no_error(local_err);
}
+static bool device_get_realized(Object *obj, Error **err)
+{
+ DeviceState *dev = DEVICE(obj);
+ return dev->realized;
+}
+
+static void device_set_realized(Object *obj, bool value, Error **err)
+{
+ DeviceState *dev = DEVICE(obj);
+ DeviceClass *dc = DEVICE_GET_CLASS(dev);
+ Error *local_err = NULL;
+
+ if (value && !dev->realized) {
+ if (dc->realize) {
+ dc->realize(dev, &local_err);
+ }
+
+ if (!obj->parent && local_err == NULL) {
+ static int unattached_count;
+ gchar *name = g_strdup_printf("device[%d]", unattached_count++);
+
+ object_property_add_child(container_get(qdev_get_machine(),
+ "/unattached"),
+ name, obj, &local_err);
+ g_free(name);
+ }
+
+ if (qdev_get_vmsd(dev) && local_err == NULL) {
+ vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
+ dev->instance_id_alias,
+ dev->alias_required_for_version);
+ }
+ if (dev->hotplugged && local_err == NULL) {
+ device_reset(dev);
+ }
+ } else if (!value && dev->realized) {
+ if (dc->unrealize) {
+ dc->unrealize(dev, &local_err);
+ }
+ }
+
+ if (local_err != NULL) {
+ error_propagate(err, local_err);
+ return;
+ }
+
+ dev->realized = value;
+}
+
static void device_initfn(Object *obj)
{
DeviceState *dev = DEVICE(obj);
@@ -650,6 +692,9 @@ static void device_initfn(Object *obj)
dev->instance_id_alias = -1;
dev->realized = false;
+ object_property_add_bool(obj, "realized",
+ device_get_realized, device_set_realized, NULL);
+
class = object_get_class(OBJECT(dev));
do {
for (prop = DEVICE_CLASS(class)->props; prop && prop->name; prop++) {
@@ -709,7 +754,10 @@ static void device_unparent(Object *obj)
static void device_class_init(ObjectClass *class, void *data)
{
+ DeviceClass *dc = DEVICE_CLASS(class);
+
class->unparent = device_unparent;
+ dc->realize = device_realize;
}
void device_reset(DeviceState *dev)
@@ -732,7 +780,7 @@ Object *qdev_get_machine(void)
return dev;
}
-static TypeInfo device_type_info = {
+static const TypeInfo device_type_info = {
.name = TYPE_DEVICE,
.parent = TYPE_OBJECT,
.instance_size = sizeof(DeviceState),
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread