diff mbox

[v5,5/8] qom: add object_new_with_props / object_new_withpropv constructors

Message ID 1432739276-10452-6-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé May 27, 2015, 3:07 p.m. UTC
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 a lot of error prone, verbose, boilerplate code to achieve.

First a pair of methods object_set_props / object_set_propv
are added which allow for a list of objects to be set in
one single API call.

Then object_new_with_props / object_new_with_propv constructors
are added which simplify the sequence of calls to create an
object, populate properties, register in the object composition
tree and mark the object complete, into a single method call.

Usage would be:

   Error *err = NULL;
   Object *obj;
   obj = object_new_with_propv(TYPE_MEMORY_BACKEND_FILE,
                               object_get_objects_root(),
                               "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, using normal QOM
semantics for parsing from string format.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/compiler.h    |   6 ++
 include/qom/object.h       | 128 +++++++++++++++++++++++++++++++
 qom/object.c               | 109 ++++++++++++++++++++++++++
 tests/.gitignore           |   1 +
 tests/Makefile             |   5 +-
 tests/check-qom-proplist.c | 186 +++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 434 insertions(+), 1 deletion(-)
 create mode 100644 tests/check-qom-proplist.c

Comments

Eric Blake June 19, 2015, 4:04 p.m. UTC | #1
On 05/27/2015 09:07 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 a lot of error prone, verbose, boilerplate code to achieve.
> 

>  /**
> + * object_new_with_props:
> + * @typename:  The name of the type of the object to instantiate.
> + * @parent: the parent object
> + * @id: The unique ID of the object
> + * @errp: pointer to error object
> + * @...: list of property names and values
> + *
> + * This function will 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 when registering the object as a
> + * child of @parent in the objects composition tree.

s/objects/object's/
Andreas Färber June 19, 2015, 4:08 p.m. UTC | #2
Am 19.06.2015 um 18:04 schrieb Eric Blake:
> On 05/27/2015 09:07 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 a lot of error prone, verbose, boilerplate code to achieve.
>>
> 
>>  /**
>> + * object_new_with_props:
>> + * @typename:  The name of the type of the object to instantiate.
>> + * @parent: the parent object
>> + * @id: The unique ID of the object
>> + * @errp: pointer to error object
>> + * @...: list of property names and values
>> + *
>> + * This function will 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 when registering the object as a
>> + * child of @parent in the objects composition tree.
> 
> s/objects/object's/

Actually I think /objects is meant, so looks okay to me?

Please do check the version in my tree or in the two PULLs though, as I
edited most messages, including capitalization in subject, use of () for
functions and s/method/function/g.

Regards,
Andreas
Eric Blake June 19, 2015, 4:17 p.m. UTC | #3
On 06/19/2015 10:08 AM, Andreas Färber wrote:
> Am 19.06.2015 um 18:04 schrieb Eric Blake:
>> On 05/27/2015 09:07 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 a lot of error prone, verbose, boilerplate code to achieve.
>>>
>>
>>>  /**
>>> + * object_new_with_props:
>>> + * @typename:  The name of the type of the object to instantiate.
>>> + * @parent: the parent object
>>> + * @id: The unique ID of the object
>>> + * @errp: pointer to error object
>>> + * @...: list of property names and values
>>> + *
>>> + * This function will 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 when registering the object as a
>>> + * child of @parent in the objects composition tree.
>>
>> s/objects/object's/
> 
> Actually I think /objects is meant, so looks okay to me?

Then s,objects,/objects, :)

Yes, that reads okay as well (and now you see why adding a character,
whether / or ', makes a difference).

> 
> Please do check the version in my tree or in the two PULLs though, as I
> edited most messages, including capitalization in subject, use of () for
> functions and s/method/function/g.
> 
> Regards,
> Andreas
>
Andreas Färber June 19, 2015, 4:43 p.m. UTC | #4
Am 19.06.2015 um 18:17 schrieb Eric Blake:
> On 06/19/2015 10:08 AM, Andreas Färber wrote:
>> Am 19.06.2015 um 18:04 schrieb Eric Blake:
>>> On 05/27/2015 09:07 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 a lot of error prone, verbose, boilerplate code to achieve.
>>>>
>>>
>>>>  /**
>>>> + * object_new_with_props:
>>>> + * @typename:  The name of the type of the object to instantiate.
>>>> + * @parent: the parent object
>>>> + * @id: The unique ID of the object
>>>> + * @errp: pointer to error object
>>>> + * @...: list of property names and values
>>>> + *
>>>> + * This function will 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 when registering the object as a
>>>> + * child of @parent in the objects composition tree.
>>>
>>> s/objects/object's/
>>
>> Actually I think /objects is meant, so looks okay to me?
> 
> Then s,objects,/objects, :)
> 
> Yes, that reads okay as well (and now you see why adding a character,
> whether / or ', makes a difference).

Erm, on second thoughts "object composition tree" is meant. :) The
parent is specified by the caller. And since that file always just talks
of "composition tree" except for one case where it says "QOM composition
tree", let's just adapt to that.

Andreas
diff mbox

Patch

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index ac7c4c4..df9dd51 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -24,6 +24,12 @@ 
 #define QEMU_WARN_UNUSED_RESULT
 #endif
 
+#if QEMU_GNUC_PREREQ(4, 0)
+#define QEMU_SENTINEL __attribute__((sentinel))
+#else
+#define QEMU_SENTINEL
+#endif
+
 #if QEMU_GNUC_PREREQ(4, 3)
 #define QEMU_ARTIFICIAL __attribute__((always_inline, artificial))
 #else
diff --git a/include/qom/object.h b/include/qom/object.h
index d30c68c..fcacdb0 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -607,6 +607,134 @@  Object *object_new(const char *typename);
 Object *object_new_with_type(Type type);
 
 /**
+ * object_new_with_props:
+ * @typename:  The name of the type of the object to instantiate.
+ * @parent: the parent object
+ * @id: The unique ID of the object
+ * @errp: pointer to error object
+ * @...: list of property names and values
+ *
+ * This function will 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 when registering the object as a
+ * child of @parent in the objects composition tree.
+ *
+ * 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.
+ *
+ * <example>
+ *   <title>Creating an object with properties</title>
+ *   <programlisting>
+ *   Error *err = NULL;
+ *   Object *obj;
+ *
+ *   obj = object_new_with_props(TYPE_MEMORY_BACKEND_FILE,
+ *                               object_get_objects_root(),
+ *                               "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));
+ *   }
+ *   </programlisting>
+ * </example>
+ *
+ * The returned object will have one stable reference maintained
+ * for as long as it is present in the object hierarchy.
+ *
+ * Returns: The newly allocated, instantiated & initialized object.
+ */
+Object *object_new_with_props(const char *typename,
+                              Object *parent,
+                              const char *id,
+                              Error **errp,
+                              ...) QEMU_SENTINEL;
+
+/**
+ * object_new_with_propv:
+ * @typename:  The name of the type of the object to instantiate.
+ * @parent: the parent object
+ * @id: The unique ID of the object
+ * @errp: pointer to error object
+ * @vargs: list of property names and values
+ *
+ * See object_new_with_props() for documentation.
+ */
+Object *object_new_with_propv(const char *typename,
+                              Object *parent,
+                              const char *id,
+                              Error **errp,
+                              va_list vargs);
+
+/**
+ * object_set_props:
+ * @obj: the object instance to set properties on
+ * @errp: pointer to error object
+ * @...: list of property names and values
+ *
+ * This function will set a list of properties on an existing object
+ * instance.
+ *
+ * The variadic parameters are a list of pairs of (propname, propvalue)
+ * strings. The propname of %NULL indicates the end of the property
+ * list.
+ *
+ * <example>
+ *   <title>Update an object's properties</title>
+ *   <programlisting>
+ *   Error *err = NULL;
+ *   Object *obj = ...get / create object...;
+ *
+ *   obj = object_set_props(obj,
+ *                          &err,
+ *                          "share", "yes",
+ *                          "mem-path", "/dev/shm/somefile",
+ *                          "prealloc", "yes",
+ *                          "size", "1048576",
+ *                          NULL);
+ *
+ *   if (!obj) {
+ *     g_printerr("Cannot set properties: %s\n",
+ *                error_get_pretty(err));
+ *   }
+ *   </programlisting>
+ * </example>
+ *
+ * The returned object will have one stable reference maintained
+ * for as long as it is present in the object hierarchy.
+ *
+ * Returns: -1 on error, 0 on success
+ */
+int object_set_props(Object *obj,
+                     Error **errp,
+                     ...) QEMU_SENTINEL;
+
+/**
+ * object_set_propv:
+ * @obj: the object instance to set properties on
+ * @errp: pointer to error object
+ * @vargs: list of property names and values
+ *
+ * See object_set_props() for documentation.
+ *
+ * Returns: -1 on error, 0 on success
+ */
+int object_set_propv(Object *obj,
+                     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 baeece2..55ab081 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,114 @@  Object *object_new(const char *typename)
     return object_new_with_type(ti);
 }
 
+
+Object *object_new_with_props(const char *typename,
+                              Object *parent,
+                              const char *id,
+                              Error **errp,
+                              ...)
+{
+    va_list vargs;
+    Object *obj;
+
+    va_start(vargs, errp);
+    obj = object_new_with_propv(typename, parent, id, errp, vargs);
+    va_end(vargs);
+
+    return obj;
+}
+
+
+Object *object_new_with_propv(const char *typename,
+                              Object *parent,
+                              const char *id,
+                              Error **errp,
+                              va_list vargs)
+{
+    Object *obj;
+    ObjectClass *klass;
+    Error *local_err = NULL;
+
+    klass = object_class_by_name(typename);
+    if (!klass) {
+        error_setg(errp, "invalid object type: %s", typename);
+        return NULL;
+    }
+
+    if (object_class_is_abstract(klass)) {
+        error_setg(errp, "object type '%s' is abstract", typename);
+        return NULL;
+    }
+    obj = object_new(typename);
+
+    if (object_set_propv(obj, &local_err, vargs) < 0) {
+        goto error;
+    }
+
+    object_property_add_child(parent, id, obj, &local_err);
+    if (local_err) {
+        goto error;
+    }
+
+    if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
+        user_creatable_complete(obj, &local_err);
+        if (local_err) {
+            object_unparent(obj);
+            goto error;
+        }
+    }
+
+    object_unref(OBJECT(obj));
+    return obj;
+
+ error:
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+    object_unref(obj);
+    return NULL;
+}
+
+
+int object_set_props(Object *obj,
+                     Error **errp,
+                     ...)
+{
+    va_list vargs;
+    int ret;
+
+    va_start(vargs, errp);
+    ret = object_set_propv(obj, errp, vargs);
+    va_end(vargs);
+
+    return ret;
+}
+
+
+int object_set_propv(Object *obj,
+                     Error **errp,
+                     va_list vargs)
+{
+    const char *propname;
+    Error *local_err = NULL;
+
+    propname = va_arg(vargs, char *);
+    while (propname != NULL) {
+        const char *value = va_arg(vargs, char *);
+
+        g_assert(value != NULL);
+        object_property_parse(obj, value, propname, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return -1;
+        }
+        propname = va_arg(vargs, char *);
+    }
+
+    return 0;
+}
+
+
 Object *object_dynamic_cast(Object *obj, const char *typename)
 {
     if (obj && object_class_dynamic_cast(object_get_class(obj), typename)) {
diff --git a/tests/.gitignore b/tests/.gitignore
index 0dcb618..dc813c2 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -5,6 +5,7 @@  check-qjson
 check-qlist
 check-qstring
 check-qom-interface
+check-qom-proplist
 rcutorture
 test-aio
 test-bitops
diff --git a/tests/Makefile b/tests/Makefile
index 729b969..39c6e2c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -68,6 +68,8 @@  check-unit-y += tests/test-bitops$(EXESUF)
 check-unit-$(CONFIG_HAS_GLIB_SUBPROCESS_TESTS) += tests/test-qdev-global-props$(EXESUF)
 check-unit-y += tests/check-qom-interface$(EXESUF)
 gcov-files-check-qom-interface-y = qom/object.c
+check-unit-y += tests/check-qom-proplist$(EXESUF)
+gcov-files-check-qom-proplist-y = qom/object.c
 check-unit-y += tests/test-qemu-opts$(EXESUF)
 gcov-files-test-qemu-opts-y = qom/test-qemu-opts.c
 check-unit-y += tests/test-write-threshold$(EXESUF)
@@ -264,7 +266,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
@@ -273,6 +275,7 @@  tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
 tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
 tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a libqemustub.a
 tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(qom-core-obj) libqemuutil.a libqemustub.a
+tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(qom-core-obj) libqemuutil.a libqemustub.a
 tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) libqemuutil.a libqemustub.a
 tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a libqemustub.a
 tests/test-rfifolock$(EXESUF): tests/test-rfifolock.o libqemuutil.a libqemustub.a
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
new file mode 100644
index 0000000..e82532e
--- /dev/null
+++ b/tests/check-qom-proplist.c
@@ -0,0 +1,186 @@ 
+/*
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Daniel P. Berrange <berrange@redhat.com>
+ */
+
+#include <glib.h>
+
+#include "qom/object.h"
+#include "qemu/module.h"
+
+
+#define TYPE_DUMMY "qemu-dummy"
+
+typedef struct DummyObject DummyObject;
+typedef struct DummyObjectClass DummyObjectClass;
+
+#define DUMMY_OBJECT(obj)                               \
+    OBJECT_CHECK(DummyObject, (obj), TYPE_DUMMY)
+
+struct DummyObject {
+    Object parent_obj;
+
+    bool bv;
+    char *sv;
+};
+
+struct DummyObjectClass {
+    ObjectClass parent_class;
+};
+
+
+static void dummy_set_bv(Object *obj,
+                         bool value,
+                         Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    dobj->bv = value;
+}
+
+static bool dummy_get_bv(Object *obj,
+                         Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    return dobj->bv;
+}
+
+
+static void dummy_set_sv(Object *obj,
+                         const char *value,
+                         Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    g_free(dobj->sv);
+    dobj->sv = g_strdup(value);
+}
+
+static char *dummy_get_sv(Object *obj,
+                          Error **errp)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    return g_strdup(dobj->sv);
+}
+
+
+static void dummy_init(Object *obj)
+{
+    object_property_add_bool(obj, "bv",
+                             dummy_get_bv,
+                             dummy_set_bv,
+                             NULL);
+    object_property_add_str(obj, "sv",
+                            dummy_get_sv,
+                            dummy_set_sv,
+                            NULL);
+}
+
+static void dummy_finalize(Object *obj)
+{
+    DummyObject *dobj = DUMMY_OBJECT(obj);
+
+    g_free(dobj->sv);
+}
+
+
+static const TypeInfo dummy_info = {
+    .name          = TYPE_DUMMY,
+    .parent        = TYPE_OBJECT,
+    .instance_size = sizeof(DummyObject),
+    .instance_init = dummy_init,
+    .instance_finalize = dummy_finalize,
+    .class_size = sizeof(DummyObjectClass),
+};
+
+static void test_dummy_createv(void)
+{
+    Error *err = NULL;
+    Object *parent = object_get_objects_root();
+    DummyObject *dobj = DUMMY_OBJECT(
+        object_new_with_props(TYPE_DUMMY,
+                              parent,
+                              "dummy0",
+                              &err,
+                              "bv", "yes",
+                              "sv", "Hiss hiss hiss",
+                              NULL));
+
+    g_assert(err == NULL);
+    g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
+    g_assert(dobj->bv == true);
+
+    g_assert(object_resolve_path_component(parent, "dummy0")
+             == OBJECT(dobj));
+
+    object_unparent(OBJECT(dobj));
+}
+
+
+static Object *new_helper(Error **errp,
+                          Object *parent,
+                          ...)
+{
+    va_list vargs;
+    Object *obj;
+
+    va_start(vargs, parent);
+    obj = object_new_with_propv(TYPE_DUMMY,
+                                parent,
+                                "dummy0",
+                                errp,
+                                vargs);
+    va_end(vargs);
+    return obj;
+}
+
+static void test_dummy_createlist(void)
+{
+    Error *err = NULL;
+    Object *parent = object_get_objects_root();
+    DummyObject *dobj = DUMMY_OBJECT(
+        new_helper(&err,
+                   parent,
+                   "bv", "yes",
+                   "sv", "Hiss hiss hiss",
+                   NULL));
+
+    g_assert(err == NULL);
+    g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
+    g_assert(dobj->bv == true);
+
+    g_assert(object_resolve_path_component(parent, "dummy0")
+             == OBJECT(dobj));
+
+    object_unparent(OBJECT(dobj));
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    module_call_init(MODULE_INIT_QOM);
+    type_register_static(&dummy_info);
+
+    g_test_add_func("/qom/proplist/createlist", test_dummy_createlist);
+    g_test_add_func("/qom/proplist/createv", test_dummy_createv);
+
+    return g_test_run();
+}