From 1ed5b918ceda9a92dd00a2d432aa431bea2da66a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 3 Feb 2012 11:48:11 +0100 Subject: [PATCH 01/25] qom: clean up cast macros Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- include/qemu/object.h | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/include/qemu/object.h b/include/qemu/object.h index 9d0251d56a..ab1c48ca3c 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -258,6 +258,16 @@ struct TypeInfo #define OBJECT(obj) \ ((Object *)(obj)) +/** + * OBJECT_CLASS: + * @class: A derivative of #ObjectClas. + * + * Converts a class to an #ObjectClass. Since all objects are #Objects, + * this function will always succeed. + */ +#define OBJECT_CLASS(class) \ + ((ObjectClass *)(class)) + /** * OBJECT_CHECK: * @type: The C type to use for the return value. @@ -272,7 +282,7 @@ struct TypeInfo * generated. */ #define OBJECT_CHECK(type, obj, name) \ - ((type *)object_dynamic_cast_assert((Object *)(obj), (name))) + ((type *)object_dynamic_cast_assert(OBJECT(obj), (name))) /** * OBJECT_CLASS_CHECK: @@ -280,11 +290,12 @@ struct TypeInfo * @obj: A derivative of @type to cast. * @name: the QOM typename of @class. * - * A type safe version of @object_check_class. This macro is typically wrapped - * by each type to perform type safe casts of a class to a specific class type. + * A type safe version of @object_class_dynamic_cast_assert. This macro is + * typically wrapped by each type to perform type safe casts of a class to a + * specific class type. */ #define OBJECT_CLASS_CHECK(class, obj, name) \ - ((class *)object_class_dynamic_cast_assert((ObjectClass *)(obj), (name))) + ((class *)object_class_dynamic_cast_assert(OBJECT_CLASS(obj), (name))) /** * OBJECT_GET_CLASS: @@ -299,9 +310,6 @@ struct TypeInfo #define OBJECT_GET_CLASS(class, obj, name) \ OBJECT_CLASS_CHECK(class, object_get_class(OBJECT(obj)), name) -#define OBJECT_CLASS(class) \ - ((ObjectClass *)(class)) - /** * InterfaceClass: * @parent_class: the base class From 0815a859492e87e120efa7feaa836ecf6ecffaf7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 3 Feb 2012 12:51:53 +0100 Subject: [PATCH 02/25] qom: more documentation on subclassing Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- include/qemu/object.h | 76 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/include/qemu/object.h b/include/qemu/object.h index ab1c48ca3c..ad7d32dae2 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -55,6 +55,9 @@ typedef struct InterfaceInfo InterfaceInfo; * * #define TYPE_MY_DEVICE "my-device" * + * // No new virtual functions: we can reuse the typedef for the + * // superclass. + * typedef DeviceClass MyDeviceClass; * typedef struct MyDevice * { * DeviceState parent; @@ -88,8 +91,21 @@ typedef struct InterfaceInfo InterfaceInfo; * * Using object_new(), a new #Object derivative will be instantiated. You can * cast an #Object to a subclass (or base-class) type using - * object_dynamic_cast(). You typically want to define a macro wrapper around - * object_dynamic_cast_assert() to make it easier to convert to a specific type. + * object_dynamic_cast(). You typically want to define macro wrappers around + * OBJECT_CHECK() and OBJECT_CLASS_CHECK() to make it easier to convert to a + * specific type: + * + * + * Typecasting macros + * + * #define MY_DEVICE_GET_CLASS(obj) \ + * OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE) + * #define MY_DEVICE_CLASS(klass) \ + * OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE) + * #define MY_DEVICE(obj) \ + * OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE) + * + * * * # Class Initialization # * @@ -108,7 +124,61 @@ typedef struct InterfaceInfo InterfaceInfo; * * Once all of the parent classes have been initialized, #TypeInfo::class_init * is called to let the class being instantiated provide default initialize for - * it's virtual functions. + * it's virtual functions. Here is how the above example might be modified + * to introduce an overridden virtual function: + * + * + * Overriding a virtual function + * + * #include "qdev.h" + * + * void my_device_class_init(ObjectClass *klass, void *class_data) + * { + * DeviceClass *dc = DEVICE_CLASS(klass); + * dc->reset = my_device_reset; + * } + * + * static TypeInfo my_device_info = { + * .name = TYPE_MY_DEVICE, + * .parent = TYPE_DEVICE, + * .instance_size = sizeof(MyDevice), + * .class_init = my_device_class_init, + * }; + * + * + * + * Introducing new virtual functions requires a class to define its own + * struct and to add a .class_size member to the TypeInfo. Each function + * will also have a wrapper to call it easily: + * + * + * Defining an abstract class + * + * #include "qdev.h" + * + * typedef struct MyDeviceClass + * { + * DeviceClass parent; + * + * void (*frobnicate) (MyDevice *obj); + * } MyDeviceClass; + * + * static TypeInfo my_device_info = { + * .name = TYPE_MY_DEVICE, + * .parent = TYPE_DEVICE, + * .instance_size = sizeof(MyDevice), + * .abstract = true, // or set a default in my_device_class_init + * .class_size = sizeof(MyDeviceClass), + * }; + * + * void my_device_frobnicate(MyDevice *obj) + * { + * MyDeviceClass *klass = MY_DEVICE_GET_CLASS(obj); + * + * klass->frobnicate(obj); + * } + * + * * * # Interfaces # * From acc4af3fec335bb0778456f72bfb2c3591c11da4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 3 Feb 2012 11:57:23 +0100 Subject: [PATCH 03/25] qom: clean up/optimize object_dynamic_cast The interface loop can be performed only on the parent object. It does not need to be done on each interface. Similarly, we can simplify the code by switching early from the implementation object to the parent object. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- qom/object.c | 64 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/qom/object.c b/qom/object.c index 4261944849..669acc5225 100644 --- a/qom/object.c +++ b/qom/object.c @@ -368,11 +368,9 @@ void object_delete(Object *obj) g_free(obj); } -static bool object_is_type(Object *obj, const char *typename) +static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type) { - TypeImpl *target_type = type_get_by_name(typename); - TypeImpl *type = obj->class->type; - GSList *i; + assert(target_type); /* Check if typename is a direct ancestor of type */ while (type) { @@ -383,24 +381,50 @@ static bool object_is_type(Object *obj, const char *typename) type = type_get_parent(type); } - /* Check if obj has an interface of typename */ - for (i = obj->interfaces; i; i = i->next) { - Interface *iface = i->data; - - if (object_is_type(OBJECT(iface), typename)) { - return true; - } - } - return false; } +static bool object_is_type(Object *obj, const char *typename) +{ + TypeImpl *target_type; + + if (typename == TYPE_OBJECT) { + return true; + } + target_type = type_get_by_name(typename); + return type_is_ancestor(obj->class->type, target_type); +} + Object *object_dynamic_cast(Object *obj, const char *typename) { GSList *i; - /* Check if typename is a direct ancestor */ - if (object_is_type(obj, typename)) { + /* Check if typename is a direct ancestor. Special-case TYPE_OBJECT, + * we want to go back from interfaces to the parent. + */ + if (typename && object_is_type(obj, typename)) { + return obj; + } + + /* Check if obj is an interface and its containing object is a direct + * ancestor of typename. In principle we could do this test at the very + * beginning of object_dynamic_cast, avoiding a second call to + * object_is_type. However, casting between interfaces is relatively + * rare, and object_is_type(obj, TYPE_INTERFACE) would fail almost always. + * + * Perhaps we could add a magic value to the object header for increased + * (run-time) type safety and to speed up tests like this one. If we ever + * do that we can revisit the order here. + */ + if (object_is_type(obj, TYPE_INTERFACE)) { + assert(!obj->interfaces); + obj = INTERFACE(obj)->obj; + if (object_is_type(obj, typename)) { + return obj; + } + } + + if (typename == TYPE_OBJECT) { return obj; } @@ -413,16 +437,6 @@ Object *object_dynamic_cast(Object *obj, const char *typename) } } - /* Check if obj is an interface and its containing object is a direct - * ancestor of typename */ - if (object_is_type(obj, TYPE_INTERFACE)) { - Interface *iface = INTERFACE(obj); - - if (object_is_type(iface->obj, typename)) { - return iface->obj; - } - } - return NULL; } From 9970bd887d7b6ec17899fa337e58547430a99e7d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 3 Feb 2012 11:51:39 +0100 Subject: [PATCH 04/25] qom: avoid useless conversions from string to type Signed-off-by: Paolo Bonzini --- qom/object.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/qom/object.c b/qom/object.c index 669acc5225..ffd50a35c0 100644 --- a/qom/object.c +++ b/qom/object.c @@ -63,6 +63,8 @@ typedef struct Interface #define INTERFACE(obj) OBJECT_CHECK(Interface, obj, TYPE_INTERFACE) +static Type type_interface; + static GHashTable *type_table_get(void) { static GHashTable *type_table; @@ -384,25 +386,20 @@ static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type) return false; } -static bool object_is_type(Object *obj, const char *typename) +static bool object_is_type(Object *obj, TypeImpl *target_type) { - TypeImpl *target_type; - - if (typename == TYPE_OBJECT) { - return true; - } - target_type = type_get_by_name(typename); - return type_is_ancestor(obj->class->type, target_type); + return !target_type || type_is_ancestor(obj->class->type, target_type); } Object *object_dynamic_cast(Object *obj, const char *typename) { + TypeImpl *target_type = type_get_by_name(typename); GSList *i; /* Check if typename is a direct ancestor. Special-case TYPE_OBJECT, * we want to go back from interfaces to the parent. */ - if (typename && object_is_type(obj, typename)) { + if (target_type && object_is_type(obj, target_type)) { return obj; } @@ -410,21 +407,21 @@ Object *object_dynamic_cast(Object *obj, const char *typename) * ancestor of typename. In principle we could do this test at the very * beginning of object_dynamic_cast, avoiding a second call to * object_is_type. However, casting between interfaces is relatively - * rare, and object_is_type(obj, TYPE_INTERFACE) would fail almost always. + * rare, and object_is_type(obj, type_interface) would fail almost always. * * Perhaps we could add a magic value to the object header for increased * (run-time) type safety and to speed up tests like this one. If we ever * do that we can revisit the order here. */ - if (object_is_type(obj, TYPE_INTERFACE)) { + if (object_is_type(obj, type_interface)) { assert(!obj->interfaces); obj = INTERFACE(obj)->obj; - if (object_is_type(obj, typename)) { + if (object_is_type(obj, target_type)) { return obj; } } - if (typename == TYPE_OBJECT) { + if (!target_type) { return obj; } @@ -432,7 +429,7 @@ Object *object_dynamic_cast(Object *obj, const char *typename) for (i = obj->interfaces; i; i = i->next) { Interface *iface = i->data; - if (object_is_type(OBJECT(iface), typename)) { + if (object_is_type(OBJECT(iface), target_type)) { return OBJECT(iface); } } @@ -449,7 +446,7 @@ static void register_interface(void) .abstract = true, }; - type_register_static(&interface_info); + type_interface = type_register_static(&interface_info); } device_init(register_interface); From b46d9b1082054cba5af5ccab584f0e22a5264057 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 15:14:32 +0100 Subject: [PATCH 05/25] qom: do not include qdev header file Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- qom/object.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/qom/object.c b/qom/object.c index ffd50a35c0..98b8ad5f7d 100644 --- a/qom/object.c +++ b/qom/object.c @@ -13,8 +13,6 @@ #include "qemu/object.h" #include "qemu-common.h" #include "qapi/qapi-visit-core.h" -#include "hw/qdev.h" -// FIXME remove above #define MAX_INTERFACES 32 From 9f5f135058e22a6f7b3d8ca02e388e502c0d161f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 1 Feb 2012 16:58:47 +0100 Subject: [PATCH 06/25] qom: add QObject-based property get/set wrappers Move the creation of QmpInputVisitor and QmpOutputVisitor from qmp.c to qom/object.c, since it's the only practical way to access object properties. Keep this isolated such that it's easy to remove. At some point, we need to remove all usage of QObject in the tree and replace it with GVariant. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- include/qemu/qom-qobject.h | 42 ++++++++++++++++++++++++++++++++++++ qmp.c | 18 +++------------- qom/Makefile | 2 +- qom/qom-qobject.c | 44 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 16 deletions(-) create mode 100644 include/qemu/qom-qobject.h create mode 100644 qom/qom-qobject.c diff --git a/include/qemu/qom-qobject.h b/include/qemu/qom-qobject.h new file mode 100644 index 0000000000..f9dff12f11 --- /dev/null +++ b/include/qemu/qom-qobject.h @@ -0,0 +1,42 @@ +/* + * QEMU Object Model - QObject wrappers + * + * Copyright (C) 2012 Red Hat, Inc. + * + * Author: Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#ifndef QEMU_QOM_QOBJECT_H +#define QEMU_QOM_QOBJECT_H + +#include "qemu/object.h" + +/* + * object_property_get_qobject: + * @obj: the object + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Returns: the value of the property, converted to QObject, or NULL if + * an error occurs. + */ +struct QObject *object_property_get_qobject(Object *obj, const char *name, + struct Error **errp); + +/** + * object_property_set_qobject: + * @obj: the object + * @ret: The value that will be written to the property. + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Writes a property to a object. + */ +void object_property_set_qobject(Object *obj, struct QObject *qobj, + const char *name, struct Error **errp); + +#endif diff --git a/qmp.c b/qmp.c index 45052cc978..1f64844095 100644 --- a/qmp.c +++ b/qmp.c @@ -21,9 +21,8 @@ #include "kvm.h" #include "arch_init.h" #include "hw/qdev.h" -#include "qapi/qmp-input-visitor.h" -#include "qapi/qmp-output-visitor.h" #include "blockdev.h" +#include "qemu/qom-qobject.h" NameInfo *qmp_query_name(Error **errp) { @@ -198,7 +197,6 @@ int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret) const char *property = qdict_get_str(qdict, "property"); QObject *value = qdict_get(qdict, "value"); Error *local_err = NULL; - QmpInputVisitor *mi; Object *obj; obj = object_resolve_path(path, NULL); @@ -207,10 +205,7 @@ int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret) goto out; } - mi = qmp_input_visitor_new(value); - object_property_set(obj, qmp_input_get_visitor(mi), property, &local_err); - - qmp_input_visitor_cleanup(mi); + object_property_set_qobject(obj, value, property, &local_err); out: if (local_err) { @@ -227,7 +222,6 @@ int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret) const char *path = qdict_get_str(qdict, "path"); const char *property = qdict_get_str(qdict, "property"); Error *local_err = NULL; - QmpOutputVisitor *mo; Object *obj; obj = object_resolve_path(path, NULL); @@ -236,13 +230,7 @@ int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret) goto out; } - mo = qmp_output_visitor_new(); - object_property_get(obj, qmp_output_get_visitor(mo), property, &local_err); - if (!local_err) { - *ret = qmp_output_get_qobject(mo); - } - - qmp_output_visitor_cleanup(mo); + *ret = object_property_get_qobject(obj, property, &local_err); out: if (local_err) { diff --git a/qom/Makefile b/qom/Makefile index f33f0be8c9..885a2631bb 100644 --- a/qom/Makefile +++ b/qom/Makefile @@ -1 +1 @@ -qom-y = object.o container.o +qom-y = object.o container.o qom-qobject.o diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c new file mode 100644 index 0000000000..0689914e15 --- /dev/null +++ b/qom/qom-qobject.c @@ -0,0 +1,44 @@ +/* + * QEMU Object Model - QObject wrappers + * + * Copyright (C) 2012 Red Hat, Inc. + * + * Author: Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu-common.h" +#include "qemu/object.h" +#include "qemu/qom-qobject.h" +#include "qapi/qapi-visit-core.h" +#include "qapi/qmp-input-visitor.h" +#include "qapi/qmp-output-visitor.h" + +void object_property_set_qobject(Object *obj, QObject *value, + const char *name, Error **errp) +{ + QmpInputVisitor *mi; + mi = qmp_input_visitor_new(value); + object_property_set(obj, qmp_input_get_visitor(mi), name, errp); + + qmp_input_visitor_cleanup(mi); +} + +QObject *object_property_get_qobject(Object *obj, const char *name, + Error **errp) +{ + QObject *ret = NULL; + Error *local_err = NULL; + QmpOutputVisitor *mo; + + mo = qmp_output_visitor_new(); + object_property_get(obj, qmp_output_get_visitor(mo), name, &local_err); + if (!local_err) { + ret = qmp_output_get_qobject(mo); + } + error_propagate(errp, local_err); + qmp_output_visitor_cleanup(mo); + return ret; +} From 7b7b7d18e40c158134b7588e629be7dd35b468fa Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 1 Feb 2012 17:16:22 +0100 Subject: [PATCH 07/25] qom: add property get/set wrappers for C types Add wrappers that let you get/set properties using normal C data types. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- include/qemu/object.h | 70 +++++++++++++++++++++++++ qom/object.c | 119 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 180 insertions(+), 9 deletions(-) diff --git a/include/qemu/object.h b/include/qemu/object.h index ad7d32dae2..b27f80e0bd 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -621,6 +621,76 @@ void object_unparent(Object *obj); void object_property_get(Object *obj, struct Visitor *v, const char *name, struct Error **errp); +/** + * object_property_set_str: + * @value: the value to be written to the property + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Writes a string value to a property. + */ +void object_property_set_str(Object *obj, const char *value, + const char *name, struct Error **errp); + +/** + * object_property_get_str: + * @obj: the object + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Returns: the value of the property, converted to a C string, or NULL if + * an error occurs (including when the property value is not a string). + * The caller should free the string. + */ +char *object_property_get_str(Object *obj, const char *name, + struct Error **errp); + +/** + * object_property_set_bool: + * @value: the value to be written to the property + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Writes a bool value to a property. + */ +void object_property_set_bool(Object *obj, bool value, + const char *name, struct Error **errp); + +/** + * object_property_get_bool: + * @obj: the object + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Returns: the value of the property, converted to a boolean, or NULL if + * an error occurs (including when the property value is not a bool). + */ +bool object_property_get_bool(Object *obj, const char *name, + struct Error **errp); + +/** + * object_property_set_int: + * @value: the value to be written to the property + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Writes an integer value to a property. + */ +void object_property_set_int(Object *obj, int64_t value, + const char *name, struct Error **errp); + +/** + * object_property_get_int: + * @obj: the object + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Returns: the value of the property, converted to an integer, or NULL if + * an error occurs (including when the property value is not an integer). + */ +int64_t object_property_get_int(Object *obj, const char *name, + struct Error **errp); + /** * object_property_set: * @obj: the object diff --git a/qom/object.c b/qom/object.c index 98b8ad5f7d..212629defd 100644 --- a/qom/object.c +++ b/qom/object.c @@ -14,6 +14,14 @@ #include "qemu-common.h" #include "qapi/qapi-visit-core.h" +/* TODO: replace QObject with a simpler visitor to avoid a dependency + * of the QOM core on QObject? */ +#include "qemu/qom-qobject.h" +#include "qobject.h" +#include "qbool.h" +#include "qint.h" +#include "qstring.h" + #define MAX_INTERFACES 32 typedef struct InterfaceImpl InterfaceImpl; @@ -657,6 +665,99 @@ void object_property_set(Object *obj, Visitor *v, const char *name, } } +void object_property_set_str(Object *obj, const char *value, + const char *name, Error **errp) +{ + QString *qstr = qstring_from_str(value); + object_property_set_qobject(obj, QOBJECT(qstr), name, errp); + + QDECREF(qstr); +} + +char *object_property_get_str(Object *obj, const char *name, + Error **errp) +{ + QObject *ret = object_property_get_qobject(obj, name, errp); + QString *qstring; + char *retval; + + if (!ret) { + return NULL; + } + qstring = qobject_to_qstring(ret); + if (!qstring) { + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, "string"); + retval = NULL; + } else { + retval = g_strdup(qstring_get_str(qstring)); + } + + QDECREF(qstring); + return retval; +} + +void object_property_set_bool(Object *obj, bool value, + const char *name, Error **errp) +{ + QBool *qbool = qbool_from_int(value); + object_property_set_qobject(obj, QOBJECT(qbool), name, errp); + + QDECREF(qbool); +} + +bool object_property_get_bool(Object *obj, const char *name, + Error **errp) +{ + QObject *ret = object_property_get_qobject(obj, name, errp); + QBool *qbool; + bool retval; + + if (!ret) { + return false; + } + qbool = qobject_to_qbool(ret); + if (!qbool) { + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, "boolean"); + retval = false; + } else { + retval = qbool_get_int(qbool); + } + + QDECREF(qbool); + return retval; +} + +void object_property_set_int(Object *obj, int64_t value, + const char *name, Error **errp) +{ + QInt *qint = qint_from_int(value); + object_property_set_qobject(obj, QOBJECT(qint), name, errp); + + QDECREF(qint); +} + +int64_t object_property_get_int(Object *obj, const char *name, + Error **errp) +{ + QObject *ret = object_property_get_qobject(obj, name, errp); + QInt *qint; + int64_t retval; + + if (!ret) { + return -1; + } + qint = qobject_to_qint(ret); + if (!qint) { + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, "int"); + retval = -1; + } else { + retval = qint_get_int(qint); + } + + QDECREF(qint); + return retval; +} + const char *object_property_get_type(Object *obj, const char *name, Error **errp) { ObjectProperty *prop = object_property_find(obj, name); @@ -938,8 +1039,8 @@ typedef struct StringProperty void (*set)(Object *, const char *, Error **); } StringProperty; -static void object_property_get_str(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static void property_get_str(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { StringProperty *prop = opaque; char *value; @@ -951,8 +1052,8 @@ static void object_property_get_str(Object *obj, Visitor *v, void *opaque, } } -static void object_property_set_str(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static void property_set_str(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { StringProperty *prop = opaque; char *value; @@ -968,8 +1069,8 @@ static void object_property_set_str(Object *obj, Visitor *v, void *opaque, g_free(value); } -static void object_property_release_str(Object *obj, const char *name, - void *opaque) +static void property_release_str(Object *obj, const char *name, + void *opaque) { StringProperty *prop = opaque; g_free(prop); @@ -986,8 +1087,8 @@ void object_property_add_str(Object *obj, const char *name, prop->set = set; object_property_add(obj, name, "string", - get ? object_property_get_str : NULL, - set ? object_property_set_str : NULL, - object_property_release_str, + get ? property_get_str : NULL, + set ? property_set_str : NULL, + property_release_str, prop, errp); } From 8f770d39056c797a0a3de7a9a1a00befddfb088a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 3 Feb 2012 15:41:13 +0100 Subject: [PATCH 08/25] qom: fix off-by-one Signed-off-by: Paolo Bonzini --- qom/object.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/qom/object.c b/qom/object.c index 212629defd..7fd37cb879 100644 --- a/qom/object.c +++ b/qom/object.c @@ -854,11 +854,8 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, target = object_resolve_path(path, &ambiguous); if (target) { - gchar *target_type; - - target_type = g_strdup(&type[5]); - target_type[strlen(target_type) - 2] = 0; - + /* Go from link to FOO. */ + gchar *target_type = g_strndup(&type[5], strlen(type) - 6); if (object_dynamic_cast(target, target_type)) { object_ref(target); *child = target; From 02fe2db6312ff894be9aa0b862b383cc9d94505a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 3 Feb 2012 11:21:01 +0100 Subject: [PATCH 09/25] qom: add object_resolve_path_type Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- include/qemu/object.h | 25 +++++++++++++++++++++++-- qom/object.c | 26 ++++++++++++++++++-------- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/include/qemu/object.h b/include/qemu/object.h index b27f80e0bd..6c36345814 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -749,13 +749,34 @@ gchar *object_get_canonical_path(Object *obj); * specifying objects easy. At each level of the composition tree, the partial * path is matched as an absolute path. The first match is not returned. At * least two matches are searched for. A successful result is only returned if - * only one match is founded. If more than one match is found, a flag is - * return to indicate that the match was ambiguous. + * only one match is found. If more than one match is found, a flag is + * returned to indicate that the match was ambiguous. * * Returns: The matched object or NULL on path lookup failure. */ Object *object_resolve_path(const char *path, bool *ambiguous); +/** + * object_resolve_path_type: + * @path: the path to resolve + * @typename: the type to look for. + * @ambiguous: returns true if the path resolution failed because of an + * ambiguous match + * + * This is similar to object_resolve_path. However, when looking for a + * partial path only matches that implement the given type are considered. + * This restricts the search and avoids spuriously flagging matches as + * ambiguous. + * + * For both partial and absolute paths, the return value goes through + * a dynamic cast to @typename. This is important if either the link, + * or the typename itself are of interface types. + * + * Returns: The matched object or NULL on path lookup failure. + */ +Object *object_resolve_path_type(const char *path, const char *typename, + bool *ambiguous); + /** * object_property_add_child: * @obj: the object to add a property to diff --git a/qom/object.c b/qom/object.c index 7fd37cb879..ea0efc662a 100644 --- a/qom/object.c +++ b/qom/object.c @@ -930,17 +930,18 @@ gchar *object_get_canonical_path(Object *obj) static Object *object_resolve_abs_path(Object *parent, gchar **parts, + const char *typename, int index) { ObjectProperty *prop; Object *child; if (parts[index] == NULL) { - return parent; + return object_dynamic_cast(parent, typename); } if (strcmp(parts[index], "") == 0) { - return object_resolve_abs_path(parent, parts, index + 1); + return object_resolve_abs_path(parent, parts, typename, index + 1); } prop = object_property_find(parent, parts[index]); @@ -962,17 +963,18 @@ static Object *object_resolve_abs_path(Object *parent, return NULL; } - return object_resolve_abs_path(child, parts, index + 1); + return object_resolve_abs_path(child, parts, typename, index + 1); } static Object *object_resolve_partial_path(Object *parent, gchar **parts, + const char *typename, bool *ambiguous) { Object *obj; ObjectProperty *prop; - obj = object_resolve_abs_path(parent, parts, 0); + obj = object_resolve_abs_path(parent, parts, typename, 0); QTAILQ_FOREACH(prop, &parent->properties, node) { Object *found; @@ -981,7 +983,8 @@ static Object *object_resolve_partial_path(Object *parent, continue; } - found = object_resolve_partial_path(prop->opaque, parts, ambiguous); + found = object_resolve_partial_path(prop->opaque, parts, + typename, ambiguous); if (found) { if (obj) { if (ambiguous) { @@ -1000,7 +1003,8 @@ static Object *object_resolve_partial_path(Object *parent, return obj; } -Object *object_resolve_path(const char *path, bool *ambiguous) +Object *object_resolve_path_type(const char *path, const char *typename, + bool *ambiguous) { bool partial_path = true; Object *obj; @@ -1020,9 +1024,10 @@ Object *object_resolve_path(const char *path, bool *ambiguous) if (ambiguous) { *ambiguous = false; } - obj = object_resolve_partial_path(object_get_root(), parts, ambiguous); + obj = object_resolve_partial_path(object_get_root(), parts, + typename, ambiguous); } else { - obj = object_resolve_abs_path(object_get_root(), parts, 1); + obj = object_resolve_abs_path(object_get_root(), parts, typename, 1); } g_strfreev(parts); @@ -1030,6 +1035,11 @@ Object *object_resolve_path(const char *path, bool *ambiguous) return obj; } +Object *object_resolve_path(const char *path, bool *ambiguous) +{ + return object_resolve_path_type(path, TYPE_OBJECT, ambiguous); +} + typedef struct StringProperty { char *(*get)(Object *, Error **); From 11e35bfdc7f030f65844b34239bdde16e68b2468 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 12:37:53 +0100 Subject: [PATCH 10/25] qom: use object_resolve_path_type for links This allows to restrict partial matches to objects of the expected type. It will let people use bare names to reference drives even though their name might be the same as a device's (e.g. -drive id=hd0,if=none,... -device ...,drive=hd0,id=hd0). As a useful byproduct, this fixes a problem with links of interface type. When a link property's type is an interface, the code expects the implementation object (not the parent object) to be stored in the variable. The parent object does not contain the right vtable. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- qerror.c | 4 ++++ qerror.h | 3 +++ qom/object.c | 31 +++++++++++++++++-------------- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/qerror.c b/qerror.c index 3d179c87ea..8e6efafe00 100644 --- a/qerror.c +++ b/qerror.c @@ -47,6 +47,10 @@ static const QErrorStringTable qerror_table[] = { .error_fmt = QERR_ADD_CLIENT_FAILED, .desc = "Could not add client", }, + { + .error_fmt = QERR_AMBIGUOUS_PATH, + .desc = "Path '%(path)' does not uniquely identify a %(object)" + }, { .error_fmt = QERR_BAD_BUS_FOR_DEVICE, .desc = "Device '%(device)' can't go on a %(bad_bus_type) bus", diff --git a/qerror.h b/qerror.h index 8c36ddb7e1..e8718bfbab 100644 --- a/qerror.h +++ b/qerror.h @@ -54,6 +54,9 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_ADD_CLIENT_FAILED \ "{ 'class': 'AddClientFailed', 'data': {} }" +#define QERR_AMBIGUOUS_PATH \ + "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }" + #define QERR_BAD_BUS_FOR_DEVICE \ "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }" diff --git a/qom/object.c b/qom/object.c index ea0efc662a..2bd15b81b2 100644 --- a/qom/object.c +++ b/qom/object.c @@ -840,6 +840,7 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, bool ambiguous = false; const char *type; char *path; + gchar *target_type; type = object_property_get_type(obj, name, NULL); @@ -847,28 +848,30 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, if (*child) { object_unref(*child); + *child = NULL; } if (strcmp(path, "") != 0) { Object *target; - target = object_resolve_path(path, &ambiguous); - if (target) { - /* Go from link to FOO. */ - gchar *target_type = g_strndup(&type[5], strlen(type) - 6); - if (object_dynamic_cast(target, target_type)) { - object_ref(target); - *child = target; - } else { - error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, type); - } + /* Go from link to FOO. */ + target_type = g_strndup(&type[5], strlen(type) - 6); + target = object_resolve_path_type(path, target_type, &ambiguous); - g_free(target_type); + if (ambiguous) { + error_set(errp, QERR_AMBIGUOUS_PATH, path); + } else if (target) { + object_ref(target); + *child = target; } else { - error_set(errp, QERR_DEVICE_NOT_FOUND, path); + target = object_resolve_path(path, &ambiguous); + if (target || ambiguous) { + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, target_type); + } else { + error_set(errp, QERR_DEVICE_NOT_FOUND, path); + } } - } else { - *child = NULL; + g_free(target_type); } g_free(path); From a1e7efdcef38f7cba4a46e836f433c73d45d926f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 3 Feb 2012 15:59:53 +0100 Subject: [PATCH 11/25] qom: fix canonical paths vs. interfaces Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- qom/object.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/qom/object.c b/qom/object.c index 2bd15b81b2..686cca00b2 100644 --- a/qom/object.c +++ b/qom/object.c @@ -805,6 +805,12 @@ void object_property_add_child(Object *obj, const char *name, { gchar *type; + /* Registering an interface object in the composition tree will mightily + * confuse object_get_canonical_path (which, on the other hand, knows how + * to get the canonical path of an interface object). + */ + assert(!object_is_type(obj, type_interface)); + type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child))); object_property_add(obj, name, type, object_get_child_property, @@ -898,6 +904,10 @@ gchar *object_get_canonical_path(Object *obj) Object *root = object_get_root(); char *newpath = NULL, *path = NULL; + if (object_is_type(obj, type_interface)) { + obj = INTERFACE(obj)->obj; + } + while (obj != root) { ObjectProperty *prop = NULL; From 1d9c5a12cefcd914d99c32de9aac72966a4788ae Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 10:51:57 +0100 Subject: [PATCH 12/25] qom: add property get/set wrappers for links These can set a link to any object, as long as it is included in the composition tree. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- include/qemu/object.h | 24 ++++++++++++++++++++++++ qom/object.c | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/include/qemu/object.h b/include/qemu/object.h index 6c36345814..7d50da9078 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -645,6 +645,30 @@ void object_property_set_str(Object *obj, const char *value, char *object_property_get_str(Object *obj, const char *name, struct Error **errp); +/** + * object_property_set_link: + * @value: the value to be written to the property + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Writes an object's canonical path to a property. + */ +void object_property_set_link(Object *obj, Object *value, + const char *name, struct Error **errp); + +/** + * object_property_get_link: + * @obj: the object + * @name: the name of the property + * @errp: returns an error if this function fails + * + * Returns: the value of the property, resolved from a path to an Object, + * or NULL if an error occurs (including when the property value is not a + * string or not a valid object path). + */ +Object *object_property_get_link(Object *obj, const char *name, + struct Error **errp); + /** * object_property_set_bool: * @value: the value to be written to the property diff --git a/qom/object.c b/qom/object.c index 686cca00b2..5e5b261103 100644 --- a/qom/object.c +++ b/qom/object.c @@ -696,6 +696,30 @@ char *object_property_get_str(Object *obj, const char *name, return retval; } +void object_property_set_link(Object *obj, Object *value, + const char *name, Error **errp) +{ + object_property_set_str(obj, object_get_canonical_path(value), + name, errp); +} + +Object *object_property_get_link(Object *obj, const char *name, + Error **errp) +{ + char *str = object_property_get_str(obj, name, errp); + Object *target = NULL; + + if (str && *str) { + target = object_resolve_path(str, NULL); + if (!target) { + error_set(errp, QERR_DEVICE_NOT_FOUND, str); + } + } + + g_free(str); + return target; +} + void object_property_set_bool(Object *obj, bool value, const char *name, Error **errp) { From d822979bdf80c3ea9752615af15f231b4c4ce547 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 09:47:13 +0100 Subject: [PATCH 13/25] qdev: remove direct calls to print/parse There's no need to call into ->parse and ->print manually. The QOM legacy properties do that for us. Furthermore, in some cases legacy and static properties have exactly the same behavior, and we could drop the legacy properties right away. Add an appropriate fallback to prepare for this. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-monitor.c | 30 +++++++++++++++++------------- hw/qdev-properties.c | 26 ++++++++++---------------- hw/qdev.c | 9 +++++++++ 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c index 135c2bf237..49f13ca087 100644 --- a/hw/qdev-monitor.c +++ b/hw/qdev-monitor.c @@ -485,22 +485,26 @@ static void qbus_print(Monitor *mon, BusState *bus, int indent); static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props, const char *prefix, int indent) { - char buf[64]; - if (!props) return; - while (props->name) { - /* - * TODO Properties without a print method are just for dirty - * hacks. qdev_prop_ptr is the only such PropertyInfo. It's - * marked for removal. The test props->info->print should be - * removed along with it. - */ - if (props->info->print) { - props->info->print(dev, props, buf, sizeof(buf)); - qdev_printf("%s-prop: %s = %s\n", prefix, props->name, buf); + for (; props->name; props++) { + Error *err = NULL; + char *value; + char *legacy_name = g_strdup_printf("legacy-%s", props->name); + if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) { + value = object_property_get_str(OBJECT(dev), legacy_name, &err); + } else { + value = object_property_get_str(OBJECT(dev), props->name, &err); } - props++; + g_free(legacy_name); + + if (err) { + error_free(err); + continue; + } + qdev_printf("%s-prop: %s = %s\n", prefix, props->name, + value && *value ? value : ""); + g_free(value); } } diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index c4583a14d7..5e19ec8be3 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -1073,24 +1073,18 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev, int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) { - Property *prop; - int ret; + char *legacy_name; + Error *err = NULL; - prop = qdev_prop_find(dev, name); - /* - * TODO Properties without a parse method are just for dirty - * hacks. qdev_prop_ptr is the only such PropertyInfo. It's - * marked for removal. The test !prop->info->parse should be - * removed along with it. - */ - if (!prop || !prop->info->parse) { - qerror_report(QERR_PROPERTY_NOT_FOUND, object_get_typename(OBJECT(dev)), name); - return -1; + legacy_name = g_strdup_printf("legacy-%s", name); + if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) { + object_property_set_str(OBJECT(dev), value, legacy_name, &err); + } else { + object_property_set_str(OBJECT(dev), value, name, &err); } - ret = prop->info->parse(dev, prop, value); - if (ret < 0) { - Error *err; - error_set_from_qdev_prop_error(&err, ret, dev, prop, value); + g_free(legacy_name); + + if (err) { qerror_report_err(err); error_free(err); return -1; diff --git a/hw/qdev.c b/hw/qdev.c index e3b53b7f62..acb7829b5d 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -581,6 +581,15 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop, void qdev_property_add_static(DeviceState *dev, Property *prop, Error **errp) { + /* + * TODO qdev_prop_ptr does not have getters or setters. It must + * go now that it can be replaced with links. The test should be + * removed along with it: all static properties are read/write. + */ + if (!prop->info->get && !prop->info->set) { + return; + } + object_property_add(OBJECT(dev), prop->name, prop->info->name, prop->info->get, prop->info->set, NULL, From 68ee356941801d0a17fdc43b11ac3e6b72fcd597 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 10:17:19 +0100 Subject: [PATCH 14/25] qdev: allow reusing get/set for legacy property In some cases, a legacy property does need a special print method but not a special parse method. In this case, we can reuse the get/set from the static (non-legacy) property. If neither parse nor print is needed, though, do not register the legacy property at all. The previous patch ensures that the right fallback will be used. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/hw/qdev.c b/hw/qdev.c index acb7829b5d..0205fe7d7d 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -550,21 +550,24 @@ static void qdev_set_legacy_property(Object *obj, Visitor *v, void *opaque, * Do not use this is new code! Properties added through this interface will * be given names and types in the "legacy" namespace. * - * Legacy properties are always processed as strings. The format of the string - * depends on the property type. + * Legacy properties are string versions of other OOM properties. The format + * of the string depends on the property type. */ void qdev_property_add_legacy(DeviceState *dev, Property *prop, Error **errp) { gchar *name, *type; + if (!prop->info->print && !prop->info->parse) { + return; + } name = g_strdup_printf("legacy-%s", prop->name); type = g_strdup_printf("legacy<%s>", prop->info->legacy_name ?: prop->info->name); object_property_add(OBJECT(dev), name, type, - prop->info->print ? qdev_get_legacy_property : NULL, - prop->info->parse ? qdev_set_legacy_property : NULL, + prop->info->print ? qdev_get_legacy_property : prop->info->get, + prop->info->parse ? qdev_set_legacy_property : prop->info->set, NULL, prop, errp); From acbac4a1dc974a3915760b4551336544c4360241 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 13:04:45 +0100 Subject: [PATCH 15/25] qdev: remove parse method for string properties We need the print method to put double quotes, but parsing is not special. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 5e19ec8be3..1fc77b5979 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -510,16 +510,6 @@ PropertyInfo qdev_prop_hex64 = { /* --- string --- */ -static int parse_string(DeviceState *dev, Property *prop, const char *str) -{ - char **ptr = qdev_get_prop_ptr(dev, prop); - - if (*ptr) - g_free(*ptr); - *ptr = g_strdup(str); - return 0; -} - static void free_string(DeviceState *dev, Property *prop) { g_free(*(char **)qdev_get_prop_ptr(dev, prop)); @@ -581,7 +571,6 @@ PropertyInfo qdev_prop_string = { .name = "string", .type = PROP_TYPE_STRING, .size = sizeof(char*), - .parse = parse_string, .print = print_string, .free = free_string, .get = get_string, From 1ce05125571e9dbbd16e1de0f8924fe45bef2bf0 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 22:09:44 +0100 Subject: [PATCH 16/25] qdev: remove print/parse methods from LostTickPolicy properties Also generalize the code so that we can have more enum properties in the future. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 64 ++++++++++++++++++++++---------------------- hw/qdev.h | 1 + qemu-common.h | 1 + 3 files changed, 34 insertions(+), 32 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 1fc77b5979..dea287adf0 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -893,50 +893,50 @@ PropertyInfo qdev_prop_macaddr = { /* --- lost tick policy --- */ -static const struct { - const char *name; - LostTickPolicy code; -} lost_tick_policy_table[] = { - { .name = "discard", .code = LOST_TICK_DISCARD }, - { .name = "delay", .code = LOST_TICK_DELAY }, - { .name = "merge", .code = LOST_TICK_MERGE }, - { .name = "slew", .code = LOST_TICK_SLEW }, +static const char *lost_tick_policy_table[LOST_TICK_MAX+1] = { + [LOST_TICK_DISCARD] = "discard", + [LOST_TICK_DELAY] = "delay", + [LOST_TICK_MERGE] = "merge", + [LOST_TICK_SLEW] = "slew", + [LOST_TICK_MAX] = NULL, }; -static int parse_lost_tick_policy(DeviceState *dev, Property *prop, - const char *str) -{ - LostTickPolicy *ptr = qdev_get_prop_ptr(dev, prop); - int i; +QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int)); - for (i = 0; i < ARRAY_SIZE(lost_tick_policy_table); i++) { - if (!strcasecmp(str, lost_tick_policy_table[i].name)) { - *ptr = lost_tick_policy_table[i].code; - break; - } - } - if (i == ARRAY_SIZE(lost_tick_policy_table)) { - return -EINVAL; - } - return 0; +static void get_enum(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + int *ptr = qdev_get_prop_ptr(dev, prop); + + visit_type_enum(v, ptr, prop->info->enum_table, + prop->info->name, prop->name, errp); } -static int print_lost_tick_policy(DeviceState *dev, Property *prop, char *dest, - size_t len) +static void set_enum(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { - LostTickPolicy *ptr = qdev_get_prop_ptr(dev, prop); + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + int *ptr = qdev_get_prop_ptr(dev, prop); - return snprintf(dest, len, "%s", lost_tick_policy_table[*ptr].name); + if (dev->state != DEV_STATE_CREATED) { + error_set(errp, QERR_PERMISSION_DENIED); + return; + } + + visit_type_enum(v, ptr, prop->info->enum_table, + prop->info->name, prop->name, errp); } PropertyInfo qdev_prop_losttickpolicy = { - .name = "lost_tick_policy", + .name = "LostTickPolicy", .type = PROP_TYPE_LOSTTICKPOLICY, .size = sizeof(LostTickPolicy), - .parse = parse_lost_tick_policy, - .print = print_lost_tick_policy, - .get = get_generic, - .set = set_generic, + .enum_table = lost_tick_policy_table, + .get = get_enum, + .set = set_enum, }; /* --- pci address --- */ diff --git a/hw/qdev.h b/hw/qdev.h index ab53273632..c31dc4ea97 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -140,6 +140,7 @@ struct PropertyInfo { const char *legacy_name; size_t size; enum PropertyType type; + const char **enum_table; int64_t min; int64_t max; int (*parse)(DeviceState *dev, Property *prop, const char *str); diff --git a/qemu-common.h b/qemu-common.h index 8b69a9eea5..9b997f8838 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -255,6 +255,7 @@ typedef enum LostTickPolicy { LOST_TICK_DELAY, LOST_TICK_MERGE, LOST_TICK_SLEW, + LOST_TICK_MAX } LostTickPolicy; void tcg_exec_init(unsigned long tb_size); From e39e5d60c9a65f36946454ddf8c9be9c582ffef8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 17:08:47 +0100 Subject: [PATCH 17/25] qdev: remove parse/print methods for mac properties Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 61 ++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index dea287adf0..42b9b2402f 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -848,46 +848,69 @@ PropertyInfo qdev_prop_ptr = { * 01:02:03:04:05:06 * 01-02-03-04-05-06 */ -static int parse_mac(DeviceState *dev, Property *prop, const char *str) +static void get_mac(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) { + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; MACAddr *mac = qdev_get_prop_ptr(dev, prop); + char buffer[2 * 6 + 5 + 1]; + char *p = buffer; + + snprintf(buffer, sizeof(buffer), "%02x:%02x:%02x:%02x:%02x:%02x", + mac->a[0], mac->a[1], mac->a[2], + mac->a[3], mac->a[4], mac->a[5]); + + visit_type_str(v, &p, name, errp); +} + +static void set_mac(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + MACAddr *mac = qdev_get_prop_ptr(dev, prop); + Error *local_err = NULL; int i, pos; - char *p; + char *str, *p; + + if (dev->state != DEV_STATE_CREATED) { + error_set(errp, QERR_PERMISSION_DENIED); + return; + } + + visit_type_str(v, &str, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } for (i = 0, pos = 0; i < 6; i++, pos += 3) { if (!qemu_isxdigit(str[pos])) - return -EINVAL; + goto inval; if (!qemu_isxdigit(str[pos+1])) - return -EINVAL; + goto inval; if (i == 5) { if (str[pos+2] != '\0') - return -EINVAL; + goto inval; } else { if (str[pos+2] != ':' && str[pos+2] != '-') - return -EINVAL; + goto inval; } mac->a[i] = strtol(str+pos, &p, 16); } - return 0; -} + return; -static int print_mac(DeviceState *dev, Property *prop, char *dest, size_t len) -{ - MACAddr *mac = qdev_get_prop_ptr(dev, prop); - - return snprintf(dest, len, "%02x:%02x:%02x:%02x:%02x:%02x", - mac->a[0], mac->a[1], mac->a[2], - mac->a[3], mac->a[4], mac->a[5]); +inval: + error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str); } PropertyInfo qdev_prop_macaddr = { .name = "macaddr", .type = PROP_TYPE_MACADDR, .size = sizeof(MACAddr), - .parse = parse_mac, - .print = print_mac, - .get = get_generic, - .set = set_generic, + .get = get_mac, + .set = set_mac, }; From b403298adb54ca683b895ebddd412dcb092cb780 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 17:12:19 +0100 Subject: [PATCH 18/25] qdev: make the non-legacy pci address property accept an integer PCI addresses are set with qdev_prop_uint32. Thus we make the QOM property accept a device and function encoded in an 8-bit integer, instead of the magic dd.f hex string. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 42b9b2402f..9a67cc59dc 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -999,30 +999,20 @@ static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t } } -static void get_pci_devfn(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) -{ - DeviceState *dev = DEVICE(obj); - Property *prop = opaque; - uint32_t *ptr = qdev_get_prop_ptr(dev, prop); - char buffer[32]; - char *p = buffer; - - buffer[0] = 0; - if (*ptr != -1) { - snprintf(buffer, sizeof(buffer), "%02x.%x", *ptr >> 3, *ptr & 7); - } - visit_type_str(v, &p, name, errp); -} - PropertyInfo qdev_prop_pci_devfn = { - .name = "pci-devfn", + .name = "int32", + .legacy_name = "pci-devfn", .type = PROP_TYPE_UINT32, .size = sizeof(uint32_t), .parse = parse_pci_devfn, .print = print_pci_devfn, - .get = get_pci_devfn, - .set = set_generic, + .get = get_int32, + .set = set_int32, + /* FIXME: this should be -1...255, but the address is stored + * into an uint32_t rather than int32_t. + */ + .min = 0, + .max = 0xFFFFFFFFULL, }; /* --- public helpers --- */ From 7b009e5d09ee7786a4fd848327b6a8d8d7c01f0d Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 13:01:40 +0100 Subject: [PATCH 19/25] qdev: remove parse/print methods for pointer properties Pointer properties (except for PROP_PTR of course) should not need a legacy counterpart. In the future, relative paths will ensure that QEMU will support the same syntax as now for drives etc.. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 142 ++++++++++++++++++++++++------------------- 1 file changed, 79 insertions(+), 63 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 9a67cc59dc..67995a32dd 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -579,9 +579,8 @@ PropertyInfo qdev_prop_string = { /* --- drive --- */ -static int parse_drive(DeviceState *dev, Property *prop, const char *str) +static int parse_drive(DeviceState *dev, const char *str, void **ptr) { - BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop); BlockDriverState *bs; bs = bdrv_find(str); @@ -603,35 +602,30 @@ static void free_drive(DeviceState *dev, Property *prop) } } -static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t len) +static const char *print_drive(void *ptr) { - BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop); - return snprintf(dest, len, "%s", - *ptr ? bdrv_get_device_name(*ptr) : ""); + return bdrv_get_device_name(ptr); } -static void get_generic(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) -{ - DeviceState *dev = DEVICE(obj); - Property *prop = opaque; - void **ptr = qdev_get_prop_ptr(dev, prop); - char buffer[1024]; - char *p = buffer; - - buffer[0] = 0; - if (*ptr) { - prop->info->print(dev, prop, buffer, sizeof(buffer)); - } - visit_type_str(v, &p, name, errp); -} - -static void set_generic(Object *obj, Visitor *v, void *opaque, +static void get_pointer(Object *obj, Visitor *v, Property *prop, + const char *(*print)(void *ptr), + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + void **ptr = qdev_get_prop_ptr(dev, prop); + char *p; + + p = (char *) (*ptr ? print(*ptr) : ""); + visit_type_str(v, &p, name, errp); +} + +static void set_pointer(Object *obj, Visitor *v, Property *prop, + int (*parse)(DeviceState *dev, const char *str, void **ptr), const char *name, Error **errp) { DeviceState *dev = DEVICE(obj); - Property *prop = opaque; Error *local_err = NULL; + void **ptr = qdev_get_prop_ptr(dev, prop); char *str; int ret; @@ -650,36 +644,45 @@ static void set_generic(Object *obj, Visitor *v, void *opaque, error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str); return; } - ret = prop->info->parse(dev, prop, str); + ret = parse(dev, str, ptr); error_set_from_qdev_prop_error(errp, ret, dev, prop, str); g_free(str); } +static void get_drive(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + get_pointer(obj, v, opaque, print_drive, name, errp); +} + +static void set_drive(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + set_pointer(obj, v, opaque, parse_drive, name, errp); +} + PropertyInfo qdev_prop_drive = { .name = "drive", .type = PROP_TYPE_DRIVE, .size = sizeof(BlockDriverState *), - .parse = parse_drive, - .print = print_drive, - .get = get_generic, - .set = set_generic, + .get = get_drive, + .set = set_drive, .free = free_drive, }; /* --- character device --- */ -static int parse_chr(DeviceState *dev, Property *prop, const char *str) +static int parse_chr(DeviceState *dev, const char *str, void **ptr) { - CharDriverState **ptr = qdev_get_prop_ptr(dev, prop); - - *ptr = qemu_chr_find(str); - if (*ptr == NULL) { + CharDriverState *chr = qemu_chr_find(str); + if (chr == NULL) { return -ENOENT; } - if ((*ptr)->avail_connections < 1) { + if (chr->avail_connections < 1) { return -EEXIST; } - --(*ptr)->avail_connections; + *ptr = chr; + --chr->avail_connections; return 0; } @@ -693,62 +696,75 @@ static void free_chr(DeviceState *dev, Property *prop) } -static int print_chr(DeviceState *dev, Property *prop, char *dest, size_t len) +static const char *print_chr(void *ptr) { - CharDriverState **ptr = qdev_get_prop_ptr(dev, prop); + CharDriverState *chr = ptr; - if (*ptr && (*ptr)->label) { - return snprintf(dest, len, "%s", (*ptr)->label); - } else { - return snprintf(dest, len, ""); - } + return chr->label ? chr->label : ""; +} + +static void get_chr(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + get_pointer(obj, v, opaque, print_chr, name, errp); +} + +static void set_chr(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + set_pointer(obj, v, opaque, parse_chr, name, errp); } PropertyInfo qdev_prop_chr = { .name = "chr", .type = PROP_TYPE_CHR, .size = sizeof(CharDriverState*), - .parse = parse_chr, - .print = print_chr, - .get = get_generic, - .set = set_generic, + .get = get_chr, + .set = set_chr, .free = free_chr, }; /* --- netdev device --- */ -static int parse_netdev(DeviceState *dev, Property *prop, const char *str) +static int parse_netdev(DeviceState *dev, const char *str, void **ptr) { - VLANClientState **ptr = qdev_get_prop_ptr(dev, prop); + VLANClientState *netdev = qemu_find_netdev(str); - *ptr = qemu_find_netdev(str); - if (*ptr == NULL) + if (netdev == NULL) { return -ENOENT; - if ((*ptr)->peer) { + } + if (netdev->peer) { return -EEXIST; } + *ptr = netdev; return 0; } -static int print_netdev(DeviceState *dev, Property *prop, char *dest, size_t len) +static const char *print_netdev(void *ptr) { - VLANClientState **ptr = qdev_get_prop_ptr(dev, prop); + VLANClientState *netdev = ptr; - if (*ptr && (*ptr)->name) { - return snprintf(dest, len, "%s", (*ptr)->name); - } else { - return snprintf(dest, len, ""); - } + return netdev->name ? netdev->name : ""; +} + +static void get_netdev(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + get_pointer(obj, v, opaque, print_netdev, name, errp); +} + +static void set_netdev(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + set_pointer(obj, v, opaque, parse_netdev, name, errp); } PropertyInfo qdev_prop_netdev = { .name = "netdev", .type = PROP_TYPE_NETDEV, .size = sizeof(VLANClientState*), - .parse = parse_netdev, - .print = print_netdev, - .get = get_generic, - .set = set_generic, + .get = get_netdev, + .set = set_netdev, }; /* --- vlan --- */ From dd0ba250ca83ed915ea192ab7539cdbb4e868c14 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 13:08:48 +0100 Subject: [PATCH 20/25] qdev: let QOM free properties Drop the special free callback. Instead, register a "regular" release method in the non-legacy property. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 19 ++++++++++++------- hw/qdev.c | 8 +------- hw/qdev.h | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 67995a32dd..d69a987b86 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -510,9 +510,10 @@ PropertyInfo qdev_prop_hex64 = { /* --- string --- */ -static void free_string(DeviceState *dev, Property *prop) +static void release_string(Object *obj, const char *name, void *opaque) { - g_free(*(char **)qdev_get_prop_ptr(dev, prop)); + Property *prop = opaque; + g_free(*(char **)qdev_get_prop_ptr(DEVICE(obj), prop)); } static int print_string(DeviceState *dev, Property *prop, char *dest, size_t len) @@ -572,7 +573,7 @@ PropertyInfo qdev_prop_string = { .type = PROP_TYPE_STRING, .size = sizeof(char*), .print = print_string, - .free = free_string, + .release = release_string, .get = get_string, .set = set_string, }; @@ -592,8 +593,10 @@ static int parse_drive(DeviceState *dev, const char *str, void **ptr) return 0; } -static void free_drive(DeviceState *dev, Property *prop) +static void release_drive(Object *obj, const char *name, void *opaque) { + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop); if (*ptr) { @@ -667,7 +670,7 @@ PropertyInfo qdev_prop_drive = { .size = sizeof(BlockDriverState *), .get = get_drive, .set = set_drive, - .free = free_drive, + .release = release_drive, }; /* --- character device --- */ @@ -686,8 +689,10 @@ static int parse_chr(DeviceState *dev, const char *str, void **ptr) return 0; } -static void free_chr(DeviceState *dev, Property *prop) +static void release_chr(Object *obj, const char *name, void *opaque) { + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; CharDriverState **ptr = qdev_get_prop_ptr(dev, prop); if (*ptr) { @@ -721,7 +726,7 @@ PropertyInfo qdev_prop_chr = { .size = sizeof(CharDriverState*), .get = get_chr, .set = set_chr, - .free = free_chr, + .release = release_chr, }; /* --- netdev device --- */ diff --git a/hw/qdev.c b/hw/qdev.c index 0205fe7d7d..487ca5d189 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -595,7 +595,7 @@ void qdev_property_add_static(DeviceState *dev, Property *prop, object_property_add(OBJECT(dev), prop->name, prop->info->name, prop->info->get, prop->info->set, - NULL, + prop->info->release, prop, errp); } @@ -626,7 +626,6 @@ static void device_finalize(Object *obj) { DeviceState *dev = DEVICE(obj); BusState *bus; - Property *prop; DeviceClass *dc = DEVICE_GET_CLASS(dev); if (dev->state == DEV_STATE_INITIALIZED) { @@ -645,11 +644,6 @@ static void device_finalize(Object *obj) } } QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling); - for (prop = qdev_get_props(dev); prop && prop->name; prop++) { - if (prop->info->free) { - prop->info->free(dev, prop); - } - } } void device_reset(DeviceState *dev) diff --git a/hw/qdev.h b/hw/qdev.h index c31dc4ea97..acccf264a5 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -145,9 +145,9 @@ struct PropertyInfo { int64_t max; int (*parse)(DeviceState *dev, Property *prop, const char *str); int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len); - void (*free)(DeviceState *dev, Property *prop); ObjectPropertyAccessor *get; ObjectPropertyAccessor *set; + ObjectPropertyRelease *release; }; typedef struct GlobalProperty { From 6350b0904615cc0531cc3059ea34db5c009c88aa Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 16:19:21 +0100 Subject: [PATCH 21/25] qdev: fix off-by-one Integer properties did not work. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index d69a987b86..debb37f4fb 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -151,7 +151,7 @@ static void set_int8(Object *obj, Visitor *v, void *opaque, error_propagate(errp, local_err); return; } - if (value > prop->info->min && value <= prop->info->max) { + if (value >= prop->info->min && value <= prop->info->max) { *ptr = value; } else { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, @@ -259,7 +259,7 @@ static void set_int16(Object *obj, Visitor *v, void *opaque, error_propagate(errp, local_err); return; } - if (value > prop->info->min && value <= prop->info->max) { + if (value >= prop->info->min && value <= prop->info->max) { *ptr = value; } else { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, @@ -333,7 +333,7 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, error_propagate(errp, local_err); return; } - if (value > prop->info->min && value <= prop->info->max) { + if (value >= prop->info->min && value <= prop->info->max) { *ptr = value; } else { error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, From 9b170e60adc6dc01564128cf09f96ec923ed6526 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 12:51:44 +0100 Subject: [PATCH 22/25] qdev: access properties via QOM Do not poke anymore in the struct when accessing qdev properties. Instead, ask the object to set the right value. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-addr.c | 5 ++- hw/qdev-properties.c | 78 ++++++++++++++++++++++++++++++-------------- hw/qdev.h | 4 +-- 3 files changed, 59 insertions(+), 28 deletions(-) diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c index 5976dcdf47..8daa73339f 100644 --- a/hw/qdev-addr.c +++ b/hw/qdev-addr.c @@ -71,5 +71,8 @@ PropertyInfo qdev_prop_taddr = { void qdev_prop_set_taddr(DeviceState *dev, const char *name, target_phys_addr_t value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_TADDR); + Error *errp = NULL; + object_property_set_int(OBJECT(dev), value, name, &errp); + assert(!errp); + } diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index debb37f4fb..5a11676165 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -1115,7 +1115,7 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) return 0; } -void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type) +static void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type) { Property *prop; @@ -1135,52 +1135,63 @@ void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyT void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_BIT); + Error *errp = NULL; + object_property_set_bool(OBJECT(dev), value, name, &errp); + assert(!errp); } void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_UINT8); + Error *errp = NULL; + object_property_set_int(OBJECT(dev), value, name, &errp); + assert(!errp); } void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_UINT16); + Error *errp = NULL; + object_property_set_int(OBJECT(dev), value, name, &errp); + assert(!errp); } void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_UINT32); + Error *errp = NULL; + object_property_set_int(OBJECT(dev), value, name, &errp); + assert(!errp); } void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_INT32); + Error *errp = NULL; + object_property_set_int(OBJECT(dev), value, name, &errp); + assert(!errp); } void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_UINT64); + Error *errp = NULL; + object_property_set_int(OBJECT(dev), value, name, &errp); + assert(!errp); } void qdev_prop_set_string(DeviceState *dev, const char *name, char *value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_STRING); + Error *errp = NULL; + object_property_set_str(OBJECT(dev), value, name, &errp); + assert(!errp); } int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value) { - int res; - - res = bdrv_attach_dev(value, dev); - if (res < 0) { - error_report("Can't attach drive %s to %s.%s: %s", - bdrv_get_device_name(value), - dev->id ? dev->id : object_get_typename(OBJECT(dev)), - name, strerror(-res)); + Error *errp = NULL; + object_property_set_str(OBJECT(dev), bdrv_get_device_name(value), + name, &errp); + if (errp) { + qerror_report_err(errp); + error_free(errp); return -1; } - qdev_prop_set(dev, name, &value, PROP_TYPE_DRIVE); return 0; } @@ -1192,28 +1203,47 @@ void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverS } void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_CHR); + Error *errp = NULL; + assert(value->label); + object_property_set_str(OBJECT(dev), value->label, name, &errp); + assert(!errp); } void qdev_prop_set_netdev(DeviceState *dev, const char *name, VLANClientState *value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_NETDEV); + Error *errp = NULL; + assert(value->name); + object_property_set_str(OBJECT(dev), value->name, name, &errp); + assert(!errp); } void qdev_prop_set_vlan(DeviceState *dev, const char *name, VLANState *value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_VLAN); + Error *errp = NULL; + object_property_set_int(OBJECT(dev), value ? value->id : -1, name, &errp); + assert(!errp); } void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value) { - qdev_prop_set(dev, name, value, PROP_TYPE_MACADDR); + Error *errp = NULL; + char str[2 * 6 + 5 + 1]; + snprintf(str, sizeof(str), "%02x:%02x:%02x:%02x:%02x:%02x", + value[0], value[1], value[2], value[3], value[4], value[5]); + + object_property_set_str(OBJECT(dev), str, name, &errp); + assert(!errp); } -void qdev_prop_set_losttickpolicy(DeviceState *dev, const char *name, - LostTickPolicy *value) +void qdev_prop_set_enum(DeviceState *dev, const char *name, int value) { - qdev_prop_set(dev, name, value, PROP_TYPE_LOSTTICKPOLICY); + Property *prop; + Error *errp = NULL; + + prop = qdev_prop_find(dev, name); + object_property_set_str(OBJECT(dev), prop->info->enum_table[value], + name, &errp); + assert(!errp); } void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value) diff --git a/hw/qdev.h b/hw/qdev.h index acccf264a5..9ccd5c3db5 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -310,7 +310,6 @@ extern PropertyInfo qdev_prop_pci_devfn; void *qdev_get_prop_ptr(DeviceState *dev, Property *prop); int qdev_prop_exists(DeviceState *dev, const char *name); int qdev_prop_parse(DeviceState *dev, const char *name, const char *value); -void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type); void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value); void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value); void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value); @@ -324,8 +323,7 @@ void qdev_prop_set_vlan(DeviceState *dev, const char *name, VLANState *value); int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value) QEMU_WARN_UNUSED_RESULT; void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value); void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value); -void qdev_prop_set_losttickpolicy(DeviceState *dev, const char *name, - LostTickPolicy *value); +void qdev_prop_set_enum(DeviceState *dev, const char *name, int value); /* FIXME: Remove opaque pointer properties. */ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value); void qdev_prop_set_defaults(DeviceState *dev, Property *props); From 7a7aae21ccab06606cee9aba846d2e30cb616763 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 16:58:31 +0100 Subject: [PATCH 23/25] qdev: inline qdev_prop_set into qdev_prop_set_ptr qdev_prop_set is not needed anymore except for hacks, simplify it and inline it. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 5a11676165..b3cd2a8921 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -1115,24 +1115,6 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) return 0; } -static void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type) -{ - Property *prop; - - prop = qdev_prop_find(dev, name); - if (!prop) { - fprintf(stderr, "%s: property \"%s.%s\" not found\n", - __FUNCTION__, object_get_typename(OBJECT(dev)), name); - abort(); - } - if (prop->info->type != type) { - fprintf(stderr, "%s: property \"%s.%s\" type mismatch\n", - __FUNCTION__, object_get_typename(OBJECT(dev)), name); - abort(); - } - qdev_prop_cpy(dev, prop, src); -} - void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value) { Error *errp = NULL; @@ -1248,7 +1230,13 @@ void qdev_prop_set_enum(DeviceState *dev, const char *name, int value) void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value) { - qdev_prop_set(dev, name, &value, PROP_TYPE_PTR); + Property *prop; + void **ptr; + + prop = qdev_prop_find(dev, name); + assert(prop && prop->info == &qdev_prop_ptr); + ptr = qdev_get_prop_ptr(dev, prop); + *ptr = value; } void qdev_prop_set_defaults(DeviceState *dev, Property *props) From 4f2d3d705c1ae7dce29254e2c4645c84e77a74d4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 09:43:02 +0100 Subject: [PATCH 24/25] qdev: initialize properties via QOM Similarly, use the object properties also to set the default values of the qdev properties. This requires reordering registration and initialization. Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-properties.c | 29 ++++++++++++++--------------- hw/qdev.c | 4 ++-- hw/qdev.h | 11 +++++++---- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index b3cd2a8921..49bed302d4 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -26,17 +26,6 @@ static void bit_prop_set(DeviceState *dev, Property *props, bool val) *p &= ~mask; } -static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src) -{ - if (props->info->type == PROP_TYPE_BIT) { - bool *defval = src; - bit_prop_set(dev, props, *defval); - } else { - char *dst = qdev_get_prop_ptr(dev, props); - memcpy(dst, src, props->info->size); - } -} - /* Bit */ static int parse_bit(DeviceState *dev, Property *prop, const char *str) { @@ -1241,13 +1230,23 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value) void qdev_prop_set_defaults(DeviceState *dev, Property *props) { + Object *obj = OBJECT(dev); if (!props) return; - while (props->name) { - if (props->defval) { - qdev_prop_cpy(dev, props, props->defval); + for (; props->name; props++) { + Error *errp = NULL; + if (props->qtype == QTYPE_NONE) { + continue; } - props++; + if (props->qtype == QTYPE_QBOOL) { + object_property_set_bool(obj, props->defval, props->name, &errp); + } else if (props->info->enum_table) { + object_property_set_str(obj, props->info->enum_table[props->defval], + props->name, &errp); + } else if (props->qtype == QTYPE_QINT) { + object_property_set_int(obj, props->defval, props->name, &errp); + } + assert(!errp); } } diff --git a/hw/qdev.c b/hw/qdev.c index 487ca5d189..8a413ef0ed 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -86,11 +86,11 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) dev->parent_bus = bus; QTAILQ_INSERT_HEAD(&bus->children, dev, sibling); - qdev_prop_set_defaults(dev, dev->parent_bus->info->props); for (prop = qdev_get_bus_info(dev)->props; prop && prop->name; prop++) { qdev_property_add_legacy(dev, prop, NULL); qdev_property_add_static(dev, prop, NULL); } + qdev_prop_set_defaults(dev, dev->parent_bus->info->props); } /* Create a new device. This only initializes the device state structure @@ -612,13 +612,13 @@ static void device_initfn(Object *obj) dev->instance_id_alias = -1; dev->state = DEV_STATE_CREATED; - qdev_prop_set_defaults(dev, qdev_get_props(dev)); for (prop = qdev_get_props(dev); prop && prop->name; prop++) { qdev_property_add_legacy(dev, prop, NULL); qdev_property_add_static(dev, prop, NULL); } object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL); + qdev_prop_set_defaults(dev, qdev_get_props(dev)); } /* Unlink device from bus and free the structure. */ diff --git a/hw/qdev.h b/hw/qdev.h index 9ccd5c3db5..a3bcf0b11b 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -112,8 +112,9 @@ struct Property { const char *name; PropertyInfo *info; int offset; - int bitnr; - void *defval; + uint8_t bitnr; + uint8_t qtype; + int64_t defval; }; enum PropertyType { @@ -255,7 +256,8 @@ extern PropertyInfo qdev_prop_pci_devfn; .info = &(_prop), \ .offset = offsetof(_state, _field) \ + type_check(_type,typeof_field(_state, _field)), \ - .defval = (_type[]) { _defval }, \ + .qtype = QTYPE_QINT, \ + .defval = (_type)_defval, \ } #define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \ .name = (_name), \ @@ -263,7 +265,8 @@ extern PropertyInfo qdev_prop_pci_devfn; .bitnr = (_bit), \ .offset = offsetof(_state, _field) \ + type_check(uint32_t,typeof_field(_state, _field)), \ - .defval = (bool[]) { (_defval) }, \ + .qtype = QTYPE_QBOOL, \ + .defval = (bool)_defval, \ } #define DEFINE_PROP_UINT8(_n, _s, _f, _d) \ From a3d4a1b0479414fc4d59368a0635640979a9e4d2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 2 Feb 2012 22:51:09 +0100 Subject: [PATCH 25/25] qdev: remove unused fields from PropertyInfo Reviewed-by: Anthony Liguori Signed-off-by: Paolo Bonzini --- hw/qdev-addr.c | 2 -- hw/qdev-properties.c | 38 +------------------------------------- hw/qdev.h | 21 --------------------- 3 files changed, 1 insertion(+), 60 deletions(-) diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c index 8daa73339f..0bb16c7eb3 100644 --- a/hw/qdev-addr.c +++ b/hw/qdev-addr.c @@ -61,8 +61,6 @@ static void set_taddr(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_taddr = { .name = "taddr", - .type = PROP_TYPE_TADDR, - .size = sizeof(target_phys_addr_t), .parse = parse_taddr, .print = print_taddr, .get = get_taddr, diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 49bed302d4..b6d6fcff01 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -12,7 +12,7 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop) static uint32_t qdev_get_prop_mask(Property *prop) { - assert(prop->info->type == PROP_TYPE_BIT); + assert(prop->info == &qdev_prop_bit); return 0x1 << prop->bitnr; } @@ -79,8 +79,6 @@ static void set_bit(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_bit = { .name = "boolean", .legacy_name = "on/off", - .type = PROP_TYPE_BIT, - .size = sizeof(uint32_t), .parse = parse_bit, .print = print_bit, .get = get_bit, @@ -151,8 +149,6 @@ static void set_int8(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_uint8 = { .name = "uint8", - .type = PROP_TYPE_UINT8, - .size = sizeof(uint8_t), .parse = parse_uint8, .print = print_uint8, .get = get_int8, @@ -185,8 +181,6 @@ static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len) PropertyInfo qdev_prop_hex8 = { .name = "uint8", .legacy_name = "hex8", - .type = PROP_TYPE_UINT8, - .size = sizeof(uint8_t), .parse = parse_hex8, .print = print_hex8, .get = get_int8, @@ -259,8 +253,6 @@ static void set_int16(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_uint16 = { .name = "uint16", - .type = PROP_TYPE_UINT16, - .size = sizeof(uint16_t), .parse = parse_uint16, .print = print_uint16, .get = get_int16, @@ -333,8 +325,6 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_uint32 = { .name = "uint32", - .type = PROP_TYPE_UINT32, - .size = sizeof(uint32_t), .parse = parse_uint32, .print = print_uint32, .get = get_int32, @@ -364,8 +354,6 @@ static int print_int32(DeviceState *dev, Property *prop, char *dest, size_t len) PropertyInfo qdev_prop_int32 = { .name = "int32", - .type = PROP_TYPE_INT32, - .size = sizeof(int32_t), .parse = parse_int32, .print = print_int32, .get = get_int32, @@ -398,8 +386,6 @@ static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len) PropertyInfo qdev_prop_hex32 = { .name = "uint32", .legacy_name = "hex32", - .type = PROP_TYPE_UINT32, - .size = sizeof(uint32_t), .parse = parse_hex32, .print = print_hex32, .get = get_int32, @@ -457,8 +443,6 @@ static void set_int64(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_uint64 = { .name = "uint64", - .type = PROP_TYPE_UINT64, - .size = sizeof(uint64_t), .parse = parse_uint64, .print = print_uint64, .get = get_int64, @@ -489,8 +473,6 @@ static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len) PropertyInfo qdev_prop_hex64 = { .name = "uint64", .legacy_name = "hex64", - .type = PROP_TYPE_UINT64, - .size = sizeof(uint64_t), .parse = parse_hex64, .print = print_hex64, .get = get_int64, @@ -559,8 +541,6 @@ static void set_string(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_string = { .name = "string", - .type = PROP_TYPE_STRING, - .size = sizeof(char*), .print = print_string, .release = release_string, .get = get_string, @@ -655,8 +635,6 @@ static void set_drive(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_drive = { .name = "drive", - .type = PROP_TYPE_DRIVE, - .size = sizeof(BlockDriverState *), .get = get_drive, .set = set_drive, .release = release_drive, @@ -711,8 +689,6 @@ static void set_chr(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_chr = { .name = "chr", - .type = PROP_TYPE_CHR, - .size = sizeof(CharDriverState*), .get = get_chr, .set = set_chr, .release = release_chr, @@ -755,8 +731,6 @@ static void set_netdev(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_netdev = { .name = "netdev", - .type = PROP_TYPE_NETDEV, - .size = sizeof(VLANClientState*), .get = get_netdev, .set = set_netdev, }; @@ -834,8 +808,6 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_vlan = { .name = "vlan", - .type = PROP_TYPE_VLAN, - .size = sizeof(VLANClientState*), .parse = parse_vlan, .print = print_vlan, .get = get_vlan, @@ -847,8 +819,6 @@ PropertyInfo qdev_prop_vlan = { /* Not a proper property, just for dirty hacks. TODO Remove it! */ PropertyInfo qdev_prop_ptr = { .name = "ptr", - .type = PROP_TYPE_PTR, - .size = sizeof(void*), }; /* --- mac address --- */ @@ -917,8 +887,6 @@ inval: PropertyInfo qdev_prop_macaddr = { .name = "macaddr", - .type = PROP_TYPE_MACADDR, - .size = sizeof(MACAddr), .get = get_mac, .set = set_mac, }; @@ -965,8 +933,6 @@ static void set_enum(Object *obj, Visitor *v, void *opaque, PropertyInfo qdev_prop_losttickpolicy = { .name = "LostTickPolicy", - .type = PROP_TYPE_LOSTTICKPOLICY, - .size = sizeof(LostTickPolicy), .enum_table = lost_tick_policy_table, .get = get_enum, .set = set_enum, @@ -1012,8 +978,6 @@ static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t PropertyInfo qdev_prop_pci_devfn = { .name = "int32", .legacy_name = "pci-devfn", - .type = PROP_TYPE_UINT32, - .size = sizeof(uint32_t), .parse = parse_pci_devfn, .print = print_pci_devfn, .get = get_int32, diff --git a/hw/qdev.h b/hw/qdev.h index a3bcf0b11b..9cc3f984b2 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -117,30 +117,9 @@ struct Property { int64_t defval; }; -enum PropertyType { - PROP_TYPE_UNSPEC = 0, - PROP_TYPE_UINT8, - PROP_TYPE_UINT16, - PROP_TYPE_UINT32, - PROP_TYPE_INT32, - PROP_TYPE_UINT64, - PROP_TYPE_TADDR, - PROP_TYPE_MACADDR, - PROP_TYPE_LOSTTICKPOLICY, - PROP_TYPE_DRIVE, - PROP_TYPE_CHR, - PROP_TYPE_STRING, - PROP_TYPE_NETDEV, - PROP_TYPE_VLAN, - PROP_TYPE_PTR, - PROP_TYPE_BIT, -}; - struct PropertyInfo { const char *name; const char *legacy_name; - size_t size; - enum PropertyType type; const char **enum_table; int64_t min; int64_t max;