| Message ID | 1429280557-8887-5-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 17/04/2015 16:22, Daniel P. Berrange wrote: > + > +Object *object_new_proplist(const char *typename, > + const char *id, > + Error **errp, > + va_list vargs) > +{ > + Object *obj; > + const char *propname; > + > + obj = object_new(typename); > + > + if (object_class_is_abstract(object_get_class(obj))) { > + error_setg(errp, "object type '%s' is abstract", typename); > + goto error; > + } > + > + propname = va_arg(vargs, char *); > + while (propname != NULL) { > + const char *value = va_arg(vargs, char *); > + > + object_property_parse(obj, value, propname, errp); > + if (*errp) { > + goto error; > + } > + propname = va_arg(vargs, char *); > + } > + > + object_property_add_child(container_get(object_get_root(), "/objects"), > + id, obj, errp); I would pass the parent as an additional argument to object_new_proplist(). Otherwise looks good. Paolo
On Fri, Apr 17, 2015 at 04:55:26PM +0200, Paolo Bonzini wrote: > > > On 17/04/2015 16:22, Daniel P. Berrange wrote: > > + > > +Object *object_new_proplist(const char *typename, > > + const char *id, > > + Error **errp, > > + va_list vargs) > > +{ > > + Object *obj; > > + const char *propname; > > + > > + obj = object_new(typename); > > + > > + if (object_class_is_abstract(object_get_class(obj))) { > > + error_setg(errp, "object type '%s' is abstract", typename); > > + goto error; > > + } > > + > > + propname = va_arg(vargs, char *); > > + while (propname != NULL) { > > + const char *value = va_arg(vargs, char *); > > + > > + object_property_parse(obj, value, propname, errp); > > + if (*errp) { > > + goto error; > > + } > > + propname = va_arg(vargs, char *); > > + } > > + > > + object_property_add_child(container_get(object_get_root(), "/objects"), > > + id, obj, errp); > > I would pass the parent as an additional argument to > object_new_proplist(). Otherwise looks good. Yep, that does rather make sense for flexibility. Regards, Daniel
On 04/17/2015 08:22 AM, Daniel P. Berrange wrote: > It is reasonably common to want to create an object, set a > number of properties, register it in the hierarchy and then > mark it as complete (if a user creatable type). This requires > quite alot of error prone, verbose, boilerplate code to achieve. s/alot/a lot/ > > The object_new_propv / object_new_proplist constructors will > simplify this task by performing all required steps in one go, > accepting the property names/values as variadic args. > > Usage would be: > > Error *err = NULL; > Object *obj; > obj = object_new_propv(TYPE_MEMORY_BACKEND_FILE, > "hostmem0", > &err, > "share", "yes", > "mem-path", "/dev/shm/somefile", > "prealloc", "yes", > "size": "1048576", s/:/,/ > NULL); > > Note all property values are passed in string form and will > be parsed into their required data types. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > include/qom/object.h | 58 +++++++++++++++++++++++++++++++++++++++++++++++ > qom/object.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/Makefile | 2 +- > 3 files changed, 123 insertions(+), 1 deletion(-) > > diff --git a/include/qom/object.h b/include/qom/object.h > index d2d7748..223b577 100644 > --- a/include/qom/object.h > + * > + * obj = object_new_propv(TYPE_MEMORY_BACKEND_FILE, > + * "hostmem0", > + * &err, > + * "share", "yes", > + * "mem-path", "/dev/shm/somefile", > + * "prealloc", "yes", > + * "size": "1048576", and again > + * NULL); > + * > + * if (!obj) { > + * g_printerr("Cannot create memory backend: %s\n", > + * error_get_pretty(err)); > + * } > + * > + * Returns: The newly allocated, instantiated & initialized object. > + */ > +Object *object_new_propv(const char *typename, > + const char *id, > + Error **errp, > + ...); You probably want to use the gcc __attribute__((__sentinel__)), so that the compiler can ensure that the caller NULL-terminates their list. > +Object *object_new_proplist(const char *typename, > + const char *id, > + Error **errp, > + va_list vargs) > +{ > + Object *obj; > + const char *propname; > + > + obj = object_new(typename); > + > + if (object_class_is_abstract(object_get_class(obj))) { > + error_setg(errp, "object type '%s' is abstract", typename); > + goto error; > + } > + > + propname = va_arg(vargs, char *); > + while (propname != NULL) { > + const char *value = va_arg(vargs, char *); > + > + object_property_parse(obj, value, propname, errp); Is it worth a sanity check of assert(value) prior to calling object_property_parse(), as I have the suspicion that it doesn't handle NULL very well?
diff --git a/include/qom/object.h b/include/qom/object.h index d2d7748..223b577 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -607,6 +607,64 @@ Object *object_new(const char *typename); Object *object_new_with_type(Type type); /** + * object_new_propv: + * @typename: The name of the type of the object to instantiate. + * @id: The unique ID of the object + * @errp: pointer to error object + * @...: list of property names and values + * + * This function with initialize a new object using heap allocated memory. + * The returned object has a reference count of 1, and will be freed when + * the last reference is dropped. + * + * The @id parameter will be used to register the object in the /objects + * hierarchy. + * + * The variadic parameters are a list of pairs of (propname, propvalue) + * strings. The propname of NULL indicates the end of the property + * list. If the object implements the user creatable interface, the + * object will be marked complete once all the properties have been + * processed. + * + * Error *err = NULL; + * Object *obj; + * + * obj = object_new_propv(TYPE_MEMORY_BACKEND_FILE, + * "hostmem0", + * &err, + * "share", "yes", + * "mem-path", "/dev/shm/somefile", + * "prealloc", "yes", + * "size": "1048576", + * NULL); + * + * if (!obj) { + * g_printerr("Cannot create memory backend: %s\n", + * error_get_pretty(err)); + * } + * + * Returns: The newly allocated, instantiated & initialized object. + */ +Object *object_new_propv(const char *typename, + const char *id, + Error **errp, + ...); + +/** + * object_new_proplist: + * @typename: The name of the type of the object to instantiate. + * @id: The unique ID of the object + * @errp: pointer to error object + * @vargs: list of property names and values + * + * See object_new_propv for documentation. + */ +Object *object_new_proplist(const char *typename, + const char *id, + Error **errp, + va_list vargs); + +/** * object_initialize_with_type: * @data: A pointer to the memory to be used for the object. * @size: The maximum size available at @data for the object. diff --git a/qom/object.c b/qom/object.c index b8dff43..29c7aea 100644 --- a/qom/object.c +++ b/qom/object.c @@ -11,6 +11,7 @@ */ #include "qom/object.h" +#include "qom/object_interfaces.h" #include "qemu-common.h" #include "qapi/visitor.h" #include "qapi-visit.h" @@ -439,6 +440,69 @@ Object *object_new(const char *typename) return object_new_with_type(ti); } +Object *object_new_propv(const char *typename, + const char *id, + Error **errp, + ...) +{ + va_list vargs; + Object *obj; + + va_start(vargs, errp); + obj = object_new_proplist(typename, id, errp, vargs); + va_end(vargs); + + return obj; +} + +Object *object_new_proplist(const char *typename, + const char *id, + Error **errp, + va_list vargs) +{ + Object *obj; + const char *propname; + + obj = object_new(typename); + + if (object_class_is_abstract(object_get_class(obj))) { + error_setg(errp, "object type '%s' is abstract", typename); + goto error; + } + + propname = va_arg(vargs, char *); + while (propname != NULL) { + const char *value = va_arg(vargs, char *); + + object_property_parse(obj, value, propname, errp); + if (*errp) { + goto error; + } + propname = va_arg(vargs, char *); + } + + object_property_add_child(container_get(object_get_root(), "/objects"), + id, obj, errp); + if (*errp) { + goto error; + } + + if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) { + user_creatable_complete(obj, errp); + if (*errp) { + object_property_del(container_get(object_get_root(), "/objects"), + id, &error_abort); + goto error; + } + } + + return obj; + + error: + object_unref(obj); + return NULL; +} + Object *object_dynamic_cast(Object *obj, const char *typename) { if (obj && object_class_dynamic_cast(object_get_class(obj), typename)) { diff --git a/tests/Makefile b/tests/Makefile index 55aa745..e212ba9 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -240,7 +240,7 @@ test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \ $(test-obj-y): QEMU_INCLUDES += -Itests QEMU_CFLAGS += -I$(SRC_PATH)/tests -qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o +qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o qom/object_interfaces.o tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a tests/check-qstring$(EXESUF): tests/check-qstring.o libqemuutil.a
It is reasonably common to want to create an object, set a number of properties, register it in the hierarchy and then mark it as complete (if a user creatable type). This requires quite alot of error prone, verbose, boilerplate code to achieve. The object_new_propv / object_new_proplist constructors will simplify this task by performing all required steps in one go, accepting the property names/values as variadic args. Usage would be: Error *err = NULL; Object *obj; obj = object_new_propv(TYPE_MEMORY_BACKEND_FILE, "hostmem0", &err, "share", "yes", "mem-path", "/dev/shm/somefile", "prealloc", "yes", "size": "1048576", NULL); Note all property values are passed in string form and will be parsed into their required data types. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- include/qom/object.h | 58 +++++++++++++++++++++++++++++++++++++++++++++++ qom/object.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/Makefile | 2 +- 3 files changed, 123 insertions(+), 1 deletion(-)