From 1590d266d96b3f9b42443d6388dfc38f527ac2d8 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 9 Apr 2015 16:57:29 -0300 Subject: [PATCH 01/14] qom: strdup() target property name on object_property_add_alias() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this, object_property_add_alias() callers can safely free the target property name, like what already happens with the 'name' argument to all object_property_add*() functions. Signed-off-by: Eduardo Habkost Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Andreas Färber --- qom/object.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qom/object.c b/qom/object.c index 96abd347b1..d142d15644 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1705,7 +1705,7 @@ void object_property_add_uint64_ptr(Object *obj, const char *name, typedef struct { Object *target_obj; - const char *target_name; + char *target_name; } AliasProperty; static void property_get_alias(Object *obj, struct Visitor *v, void *opaque, @@ -1736,6 +1736,7 @@ static void property_release_alias(Object *obj, const char *name, void *opaque) { AliasProperty *prop = opaque; + g_free(prop->target_name); g_free(prop); } @@ -1763,7 +1764,7 @@ void object_property_add_alias(Object *obj, const char *name, prop = g_malloc(sizeof(*prop)); prop->target_obj = target_obj; - prop->target_name = target_name; + prop->target_name = g_strdup(target_name); op = object_property_add(obj, name, prop_type, property_get_alias, From 6bc5cf92c0ab0085ba9a6e0cebcf5a544f416ca7 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 9 Apr 2015 16:57:30 -0300 Subject: [PATCH 02/14] qdev: Free property names after registering gpio aliases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that object_property_add_alias() strdup()s target_name, we can free the property names in qdev_pass_gpios(). Signed-off-by: Eduardo Habkost Reviewed-by: Paolo Bonzini Reviewed-by: Stefan Hajnoczi Signed-off-by: Andreas Färber --- hw/core/qdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index b0f0f84564..d10fa5fbf6 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -563,6 +563,7 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container, object_property_add_alias(OBJECT(container), propname, OBJECT(dev), propname, &error_abort); + g_free(propname); } for (i = 0; i < ngl->num_out; i++) { const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out"; @@ -571,6 +572,7 @@ void qdev_pass_gpios(DeviceState *dev, DeviceState *container, object_property_add_alias(OBJECT(container), propname, OBJECT(dev), propname, &error_abort); + g_free(propname); } QLIST_REMOVE(ngl, node); QLIST_INSERT_HEAD(&container->gpios, ngl, node); From 53f77e4562f85ccf82c8831a4448e9aefb538837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Wed, 25 Mar 2015 18:40:15 +0100 Subject: [PATCH 03/14] tests: Use qtest_add_data_func() consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace uses of g_test_add_data_func() for QTest test cases. It is still valid to use it for any non-QTest test cases, which are not run for multiple target binaries. Suggested-by: John Snow Reviewed-by: John Snow Signed-off-by: Andreas Färber --- tests/ahci-test.c | 9 ++------- tests/e1000-test.c | 4 ++-- tests/eepro100-test.c | 5 ++--- tests/endianness-test.c | 18 +++++++++--------- tests/pc-cpu-test.c | 13 ++++++------- tests/qom-test.c | 4 ++-- 6 files changed, 23 insertions(+), 30 deletions(-) diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 6e3fa819e0..ae9415d74c 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -1486,7 +1486,6 @@ static void test_io_interface(gconstpointer opaque) static void create_ahci_io_test(enum IOMode type, enum AddrMode addr, enum BuffLen len, enum OffsetType offset) { - static const char *arch; char *name; AHCIIOTestOptions *opts = g_malloc(sizeof(AHCIIOTestOptions)); @@ -1495,17 +1494,13 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr, opts->io_type = type; opts->offset = offset; - if (!arch) { - arch = qtest_get_arch(); - } - - name = g_strdup_printf("/%s/ahci/io/%s/%s/%s/%s", arch, + name = g_strdup_printf("ahci/io/%s/%s/%s/%s", io_mode_str[type], addr_mode_str[addr], buff_len_str[len], offset_str[offset]); - g_test_add_data_func(name, opts, test_io_interface); + qtest_add_data_func(name, opts, test_io_interface); g_free(name); } diff --git a/tests/e1000-test.c b/tests/e1000-test.c index 81f164d9e9..7ca6d7e72e 100644 --- a/tests/e1000-test.c +++ b/tests/e1000-test.c @@ -44,8 +44,8 @@ int main(int argc, char **argv) for (i = 0; i < ARRAY_SIZE(models); i++) { char *path; - path = g_strdup_printf("/%s/e1000/%s", qtest_get_arch(), models[i]); - g_test_add_data_func(path, models[i], test_device); + path = g_strdup_printf("e1000/%s", models[i]); + qtest_add_data_func(path, models[i], test_device); } return g_test_run(); diff --git a/tests/eepro100-test.c b/tests/eepro100-test.c index bf8252627e..8bfaccdcbb 100644 --- a/tests/eepro100-test.c +++ b/tests/eepro100-test.c @@ -54,9 +54,8 @@ int main(int argc, char **argv) for (i = 0; i < ARRAY_SIZE(models); i++) { char *path; - path = g_strdup_printf("/%s/eepro100/%s", - qtest_get_arch(), models[i]); - g_test_add_data_func(path, models[i], test_device); + path = g_strdup_printf("eepro100/%s", models[i]); + qtest_add_data_func(path, models[i], test_device); } return g_test_run(); diff --git a/tests/endianness-test.c b/tests/endianness-test.c index 26ee734f42..2054338e18 100644 --- a/tests/endianness-test.c +++ b/tests/endianness-test.c @@ -296,17 +296,17 @@ int main(int argc, char **argv) if (strcmp(test_cases[i].arch, arch) != 0) { continue; } - path = g_strdup_printf("/%s/endianness/%s", - arch, test_cases[i].machine); - g_test_add_data_func(path, &test_cases[i], test_endianness); + path = g_strdup_printf("endianness/%s", + test_cases[i].machine); + qtest_add_data_func(path, &test_cases[i], test_endianness); - path = g_strdup_printf("/%s/endianness/split/%s", - arch, test_cases[i].machine); - g_test_add_data_func(path, &test_cases[i], test_endianness_split); + path = g_strdup_printf("endianness/split/%s", + test_cases[i].machine); + qtest_add_data_func(path, &test_cases[i], test_endianness_split); - path = g_strdup_printf("/%s/endianness/combine/%s", - arch, test_cases[i].machine); - g_test_add_data_func(path, &test_cases[i], test_endianness_combine); + path = g_strdup_printf("endianness/combine/%s", + test_cases[i].machine); + qtest_add_data_func(path, &test_cases[i], test_endianness_combine); } ret = g_test_run(); diff --git a/tests/pc-cpu-test.c b/tests/pc-cpu-test.c index a0122d3d61..3505c7c43f 100644 --- a/tests/pc-cpu-test.c +++ b/tests/pc-cpu-test.c @@ -75,7 +75,6 @@ static void test_pc_without_cpu_add(gconstpointer data) static void add_pc_test_cases(void) { - const char *arch = qtest_get_arch(); QDict *response, *minfo; QList *list; const QListEntry *p; @@ -119,15 +118,15 @@ static void add_pc_test_cases(void) (strcmp(mname, "pc-0.12") == 0) || (strcmp(mname, "pc-0.11") == 0) || (strcmp(mname, "pc-0.10") == 0)) { - path = g_strdup_printf("/%s/cpu/%s/init/%ux%ux%u&maxcpus=%u", - arch, mname, data->sockets, data->cores, + path = g_strdup_printf("cpu/%s/init/%ux%ux%u&maxcpus=%u", + mname, data->sockets, data->cores, data->threads, data->maxcpus); - g_test_add_data_func(path, data, test_pc_without_cpu_add); + qtest_add_data_func(path, data, test_pc_without_cpu_add); } else { - path = g_strdup_printf("/%s/cpu/%s/add/%ux%ux%u&maxcpus=%u", - arch, mname, data->sockets, data->cores, + path = g_strdup_printf("cpu/%s/add/%ux%ux%u&maxcpus=%u", + mname, data->sockets, data->cores, data->threads, data->maxcpus); - g_test_add_data_func(path, data, test_pc_with_cpu_add); + qtest_add_data_func(path, data, test_pc_with_cpu_add); } } qtest_end(); diff --git a/tests/qom-test.c b/tests/qom-test.c index 4246382d38..fde04e7a19 100644 --- a/tests/qom-test.c +++ b/tests/qom-test.c @@ -128,8 +128,8 @@ static void add_machine_test_cases(void) g_assert(qstr); mname = qstring_get_str(qstr); if (!is_blacklisted(arch, mname)) { - path = g_strdup_printf("/%s/qom/%s", arch, mname); - g_test_add_data_func(path, mname, test_machine); + path = g_strdup_printf("qom/%s", mname); + qtest_add_data_func(path, mname, test_machine); } } qtest_end(); From ff5397bc72a1716bb34302dd470343ebee7d6bf2 Mon Sep 17 00:00:00 2001 From: Martin Cerveny Date: Wed, 13 May 2015 14:14:54 +0200 Subject: [PATCH 04/14] scripts: Add support for path as argument of qom-tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add processing of optional argument path as "tree base". Signed-off-by: Martin Cerveny Signed-off-by: Andreas Färber --- scripts/qmp/qom-tree | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree index aea11d4b1a..906fcd2640 100755 --- a/scripts/qmp/qom-tree +++ b/scripts/qmp/qom-tree @@ -65,6 +65,11 @@ def list_node(path): print '' for item in items: if item['type'].startswith('child<'): - list_node(path + '/' + item['name']) + list_node((path if (path != '/') else '') + '/' + item['name']) -list_node('/machine') +if len(args) == 0: + path = '/' +else: + path = args[0] + +list_node(path) From b1028b4e8683740cd257a9b77577734664e61511 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 13 May 2015 17:14:02 +0100 Subject: [PATCH 05/14] backends: Fix typename of 'policy' enum property in hostmem obj MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'policy' property was being registered with a typename of 'str', but it is in fact an enum of the 'HostMemPolicy' type. Signed-off-by: Daniel P. Berrange Reviewed-by: Paolo Bonzini Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- backends/hostmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backends/hostmem.c b/backends/hostmem.c index b7b6cf8f4a..f6db33c14e 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -252,7 +252,7 @@ static void host_memory_backend_init(Object *obj) object_property_add(obj, "host-nodes", "int", host_memory_backend_get_host_nodes, host_memory_backend_set_host_nodes, NULL, NULL, NULL); - object_property_add(obj, "policy", "str", + object_property_add(obj, "policy", "HostMemPolicy", host_memory_backend_get_policy, host_memory_backend_set_policy, NULL, NULL, NULL); } From b9174d4f250cacb43b7cd9e07cf9f86818d62afd Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 13 May 2015 17:14:03 +0100 Subject: [PATCH 06/14] doc: Document user creatable object types in help text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The QEMU help for -object is essentially useless, just giving users the generic syntax. Move it down into its own section and introduce a nested table where each user creatable object can be documented. The existing memory-backend-file, rng-random and rng-egd object types are documented. Signed-off-by: Daniel P. Berrange Reviewed-by: Paolo Bonzini Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- qemu-options.hx | 70 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index 5438f9862c..39304d7ea6 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3476,22 +3476,6 @@ DEF("no-kvm-irqchip", 0, QEMU_OPTION_no_kvm_irqchip, "", QEMU_ARCH_I386) HXCOMM Deprecated (ignored) DEF("tdf", 0, QEMU_OPTION_tdf,"", QEMU_ARCH_ALL) -DEF("object", HAS_ARG, QEMU_OPTION_object, - "-object TYPENAME[,PROP1=VALUE1,...]\n" - " create an new object of type TYPENAME setting properties\n" - " in the order they are specified. Note that the 'id'\n" - " property must be set. These objects are placed in the\n" - " '/objects' path.\n", - QEMU_ARCH_ALL) -STEXI -@item -object @var{typename}[,@var{prop1}=@var{value1},...] -@findex -object -Create an new object of type @var{typename} setting properties -in the order they are specified. Note that the 'id' -property must be set. These objects are placed in the -'/objects' path. -ETEXI - DEF("msg", HAS_ARG, QEMU_OPTION_msg, "-msg timestamp[=on|off]\n" " change the format of messages\n" @@ -3517,6 +3501,60 @@ Dump json-encoded vmstate information for current machine type to file in @var{file} ETEXI +DEFHEADING(Generic object creation) + +DEF("object", HAS_ARG, QEMU_OPTION_object, + "-object TYPENAME[,PROP1=VALUE1,...]\n" + " create a new object of type TYPENAME setting properties\n" + " in the order they are specified. Note that the 'id'\n" + " property must be set. These objects are placed in the\n" + " '/objects' path.\n", + QEMU_ARCH_ALL) +STEXI +@item -object @var{typename}[,@var{prop1}=@var{value1},...] +@findex -object +Create a new object of type @var{typename} setting properties +in the order they are specified. Note that the 'id' +property must be set. These objects are placed in the +'/objects' path. + +@table @option + +@item -object memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off} + +Creates a memory file backend object, which can be used to back +the guest RAM with huge pages. The @option{id} parameter is a +unique ID that will be used to reference this memory region +when configuring the @option{-numa} argument. The @option{size} +option provides the size of the memory region, and accepts +common suffixes, eg @option{500M}. The @option{mem-path} provides +the path to either a shared memory or huge page filesystem mount. +The @option{share} boolean option determines whether the memory +region is marked as private to QEMU, or shared. The latter allows +a co-operating external process to access the QEMU memory region. + +@item -object rng-random,id=@var{id},filename=@var{/dev/random} + +Creates a random number generator backend which obtains entropy from +a device on the host. The @option{id} parameter is a unique ID that +will be used to reference this entropy backend from the @option{virtio-rng} +device. The @option{filename} parameter specifies which file to obtain +entropy from and if omitted defaults to @option{/dev/random}. + +@item -object rng-egd,id=@var{id},chardev=@var{chardevid} + +Creates a random number generator backend which obtains entropy from +an external daemon running on the host. The @option{id} parameter is +a unique ID that will be used to reference this entropy backend from +the @option{virtio-rng} device. The @option{chardev} parameter is +the unique ID of a character device backend that provides the connection +to the RNG daemon. + +@end table + +ETEXI + + HXCOMM This is the last statement. Insert new options before this line! STEXI @end table From f08f9271bfe3f19a5eb3d7a2f48532065304d5c8 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 13 May 2015 17:14:04 +0100 Subject: [PATCH 07/14] vl: Create (most) objects before creating chardev backends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some types of object must be created before chardevs, other types of object must be created after chardevs. As such there is no option but to create objects in two phases. This takes the decision to create as many object types as possible right away before anyother backends are created, and only delay creation of those few which have an explicit dependency on the chardevs. Hopefully the set which need delaying will remain small over time. Signed-off-by: Daniel P. Berrange Reviewed-by: Paolo Bonzini Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- vl.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 2201e27fdc..c8390716b5 100644 --- a/vl.c +++ b/vl.c @@ -2671,6 +2671,33 @@ static int machine_set_property(void *opaque, return 0; } + +/* + * Initial object creation happens before all other + * QEMU data types are created. The majority of objects + * can be created at this point. The rng-egd object + * cannot be created here, as it depends on the chardev + * already existing. + */ +static bool object_create_initial(const char *type) +{ + if (g_str_equal(type, "rng-egd")) { + return false; + } + return true; +} + + +/* + * The remainder of object creation happens after the + * creation of chardev, fsdev and device data types. + */ +static bool object_create_delayed(const char *type) +{ + return !object_create_initial(type); +} + + static int object_create(void *opaque, QemuOpts *opts, Error **errp) { Error *err = NULL; @@ -2679,6 +2706,7 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) void *dummy = NULL; OptsVisitor *ov; QDict *pdict; + bool (*type_predicate)(const char *) = opaque; ov = opts_visitor_new(opts); pdict = qemu_opts_to_qdict(opts, NULL); @@ -2693,6 +2721,9 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) if (err) { goto out; } + if (!type_predicate(type)) { + goto out; + } qdict_del(pdict, "id"); visit_type_str(opts_get_visitor(ov), &id, "id", &err); @@ -4114,6 +4145,12 @@ int main(int argc, char **argv, char **envp) socket_init(); + if (qemu_opts_foreach(qemu_find_opts("object"), + object_create, + object_create_initial, NULL)) { + exit(1); + } + if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL, NULL)) { exit(1); @@ -4137,7 +4174,8 @@ int main(int argc, char **argv, char **envp) } if (qemu_opts_foreach(qemu_find_opts("object"), - object_create, NULL, NULL)) { + object_create, + object_create_delayed, NULL)) { exit(1); } From bc2256c4ae86308a1521c89456b599d441119418 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 13 May 2015 17:14:05 +0100 Subject: [PATCH 08/14] qom: Add helper function for getting user objects root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add object_get_objects_root() function which is a convenience for obtaining the Object * located at /objects in the object composition tree. Convert existing code over to use the new API where appropriate. Signed-off-by: Daniel P. Berrange Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- include/qom/object.h | 12 ++++++++++++ iothread.c | 4 +--- numa.c | 2 +- qmp.c | 6 +++--- qom/object.c | 5 +++++ 5 files changed, 22 insertions(+), 7 deletions(-) diff --git a/include/qom/object.h b/include/qom/object.h index 0505f20e71..b03ede032d 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1026,6 +1026,18 @@ const char *object_property_get_type(Object *obj, const char *name, */ Object *object_get_root(void); + +/** + * object_get_objects_root: + * + * Get the container object that holds user created + * object instances. This is the object at path + * "/objects" + * + * Returns: the user object container + */ +Object *object_get_objects_root(void); + /** * object_get_canonical_path_component: * diff --git a/iothread.c b/iothread.c index 878a594ef4..6d2a33faf9 100644 --- a/iothread.c +++ b/iothread.c @@ -19,8 +19,6 @@ #include "qmp-commands.h" #include "qemu/error-report.h" -#define IOTHREADS_PATH "/objects" - typedef ObjectClass IOThreadClass; #define IOTHREAD_GET_CLASS(obj) \ @@ -160,7 +158,7 @@ IOThreadInfoList *qmp_query_iothreads(Error **errp) { IOThreadInfoList *head = NULL; IOThreadInfoList **prev = &head; - Object *container = container_get(object_get_root(), IOTHREADS_PATH); + Object *container = object_get_objects_root(); object_child_foreach(container, query_one_iothread, &prev); return head; diff --git a/numa.c b/numa.c index d227ccc23b..e67322a69b 100644 --- a/numa.c +++ b/numa.c @@ -485,7 +485,7 @@ MemdevList *qmp_query_memdev(Error **errp) Object *obj; MemdevList *list = NULL; - obj = object_resolve_path("/objects", NULL); + obj = object_get_objects_root(); if (obj == NULL) { return NULL; } diff --git a/qmp.c b/qmp.c index 3f5dfe3f51..fa013e31b8 100644 --- a/qmp.c +++ b/qmp.c @@ -651,7 +651,7 @@ void object_add(const char *type, const char *id, const QDict *qdict, } } - object_property_add_child(container_get(object_get_root(), "/objects"), + object_property_add_child(object_get_objects_root(), id, obj, &local_err); if (local_err) { goto out; @@ -659,7 +659,7 @@ void object_add(const char *type, const char *id, const QDict *qdict, user_creatable_complete(obj, &local_err); if (local_err) { - object_property_del(container_get(object_get_root(), "/objects"), + object_property_del(object_get_objects_root(), id, &error_abort); goto out; } @@ -706,7 +706,7 @@ void qmp_object_del(const char *id, Error **errp) Object *container; Object *obj; - container = container_get(object_get_root(), "/objects"); + container = object_get_objects_root(); obj = object_resolve_path_component(container, id); if (!obj) { error_setg(errp, "object id not found"); diff --git a/qom/object.c b/qom/object.c index d142d15644..69b7077c2b 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1054,6 +1054,11 @@ Object *object_get_root(void) return root; } +Object *object_get_objects_root(void) +{ + return container_get(object_get_root(), "/objects"); +} + static void object_get_child_property(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { From a31bdae5a76ecc060c1eb8a66be1896072c1e8b2 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 13 May 2015 17:14:06 +0100 Subject: [PATCH 09/14] qom: Add object_new_with_props() / object_new_withpropv() helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 functions 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 Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- 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 diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index ac7c4c441e..df9dd514f1 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 b03ede032d..018e27eb0c 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -606,6 +606,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 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. + * + * + * Creating an object with properties + * + * 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)); + * } + * + * + * + * 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. + * + * + * Update an object's properties + * + * 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)); + * } + * + * + * + * 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. diff --git a/qom/object.c b/qom/object.c index 69b7077c2b..dd82287a57 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 0dcb61829c..dc813c2713 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 4de40deb2f..af22fd9bfb 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) @@ -267,7 +269,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 @@ -276,6 +278,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 0000000000..e82532e1f2 --- /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 + * . + * + * Author: Daniel P. Berrange + */ + +#include + +#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(); +} From 2e4450ff432daef524cb3557fca68a3b7b5c7823 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 13 May 2015 17:14:07 +0100 Subject: [PATCH 10/14] qom: Make enum string tables const-correct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The enum string table parameters in various QOM/QAPI methods are declared 'const char *strings[]'. This results in const warnings if passed a variable that was declared as static const char * const strings[] = { .... }; Add the extra const annotation to the parameters, since neither the string elements, nor the array itself should ever be modified. Signed-off-by: Daniel P. Berrange Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- include/hw/qdev-core.h | 2 +- include/qapi/util.h | 2 +- include/qapi/visitor-impl.h | 6 +++--- include/qapi/visitor.h | 2 +- include/qom/object.h | 2 +- qapi/qapi-dealloc-visitor.c | 3 ++- qapi/qapi-util.c | 2 +- qapi/qapi-visit-core.c | 6 +++--- qom/object.c | 2 +- scripts/qapi-types.py | 4 ++-- 10 files changed, 16 insertions(+), 15 deletions(-) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index d4be92fbee..ad9eb89e59 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -236,7 +236,7 @@ struct Property { struct PropertyInfo { const char *name; const char *description; - const char **enum_table; + const char * const *enum_table; int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len); ObjectPropertyAccessor *get; ObjectPropertyAccessor *set; diff --git a/include/qapi/util.h b/include/qapi/util.h index de9238bf95..7ad26c0aca 100644 --- a/include/qapi/util.h +++ b/include/qapi/util.h @@ -11,7 +11,7 @@ #ifndef QAPI_UTIL_H #define QAPI_UTIL_H -int qapi_enum_parse(const char *lookup[], const char *buf, +int qapi_enum_parse(const char * const lookup[], const char *buf, int max, int def, Error **errp); #endif diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 09bb0fd408..f4a2f746c8 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -30,7 +30,7 @@ struct Visitor GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp); void (*end_list)(Visitor *v, Error **errp); - void (*type_enum)(Visitor *v, int *obj, const char *strings[], + void (*type_enum)(Visitor *v, int *obj, const char * const strings[], const char *kind, const char *name, Error **errp); void (*get_next_type)(Visitor *v, int *kind, const int *qobjects, const char *name, Error **errp); @@ -59,9 +59,9 @@ struct Visitor void (*end_union)(Visitor *v, bool data_present, Error **errp); }; -void input_type_enum(Visitor *v, int *obj, const char *strings[], +void input_type_enum(Visitor *v, int *obj, const char * const strings[], const char *kind, const char *name, Error **errp); -void output_type_enum(Visitor *v, int *obj, const char *strings[], +void output_type_enum(Visitor *v, int *obj, const char * const strings[], const char *kind, const char *name, Error **errp); #endif diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 5934f59ad8..00ba104cd4 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -43,7 +43,7 @@ void visit_optional(Visitor *v, bool *present, const char *name, Error **errp); void visit_get_next_type(Visitor *v, int *obj, const int *qtypes, const char *name, Error **errp); -void visit_type_enum(Visitor *v, int *obj, const char *strings[], +void visit_type_enum(Visitor *v, int *obj, const char * const strings[], const char *kind, const char *name, Error **errp); void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp); void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp); diff --git a/include/qom/object.h b/include/qom/object.h index 018e27eb0c..701b4c57e7 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1081,7 +1081,7 @@ int64_t object_property_get_int(Object *obj, const char *name, * an enum). */ int object_property_get_enum(Object *obj, const char *name, - const char *strings[], Error **errp); + const char * const strings[], Error **errp); /** * object_property_get_uint16List: diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index a14a1c7146..d7f92c5d68 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -156,7 +156,8 @@ static void qapi_dealloc_type_size(Visitor *v, uint64_t *obj, const char *name, { } -static void qapi_dealloc_type_enum(Visitor *v, int *obj, const char *strings[], +static void qapi_dealloc_type_enum(Visitor *v, int *obj, + const char * const strings[], const char *kind, const char *name, Error **errp) { diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c index 1d8fb96eff..bcdc94d5a9 100644 --- a/qapi/qapi-util.c +++ b/qapi/qapi-util.c @@ -14,7 +14,7 @@ #include "qapi/error.h" #include "qapi/util.h" -int qapi_enum_parse(const char *lookup[], const char *buf, +int qapi_enum_parse(const char * const lookup[], const char *buf, int max, int def, Error **errp) { int i; diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index b66b93ae2b..a18ba1670f 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -89,7 +89,7 @@ void visit_get_next_type(Visitor *v, int *obj, const int *qtypes, } } -void visit_type_enum(Visitor *v, int *obj, const char *strings[], +void visit_type_enum(Visitor *v, int *obj, const char * const strings[], const char *kind, const char *name, Error **errp) { v->type_enum(v, obj, strings, kind, name, errp); @@ -260,7 +260,7 @@ void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp) v->type_number(v, obj, name, errp); } -void output_type_enum(Visitor *v, int *obj, const char *strings[], +void output_type_enum(Visitor *v, int *obj, const char * const strings[], const char *kind, const char *name, Error **errp) { @@ -279,7 +279,7 @@ void output_type_enum(Visitor *v, int *obj, const char *strings[], visit_type_str(v, &enum_str, name, errp); } -void input_type_enum(Visitor *v, int *obj, const char *strings[], +void input_type_enum(Visitor *v, int *obj, const char * const strings[], const char *kind, const char *name, Error **errp) { diff --git a/qom/object.c b/qom/object.c index dd82287a57..523307d839 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1070,7 +1070,7 @@ int64_t object_property_get_int(Object *obj, const char *name, } int object_property_get_enum(Object *obj, const char *name, - const char *strings[], Error **errp) + const char * const strings[], Error **errp) { StringOutputVisitor *sov; StringInputVisitor *siv; diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index d28a6b07be..e6eb4b613a 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -105,7 +105,7 @@ struct %(name)s def generate_enum_lookup(name, values): ret = mcgen(''' -const char *%(name)s_lookup[] = { +const char * const %(name)s_lookup[] = { ''', name=c_name(name)) i = 0 @@ -128,7 +128,7 @@ const char *%(name)s_lookup[] = { def generate_enum(name, values): name = c_name(name) lookup_decl = mcgen(''' -extern const char *%(name)s_lookup[]; +extern const char * const %(name)s_lookup[]; ''', name=name) From a8e3fbedc827f992657f5670212e854f62ec12ad Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 13 May 2015 17:14:08 +0100 Subject: [PATCH 11/14] qom: Add an object_property_add_enum() helper function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A QOM property can be parsed as enum using the visit_type_enum() helper function, but this forces callers to use the more complex generic object_property_add() method when registering it. It also requires that users of that object have access to the string map when they want to read the property value. This patch introduces a specialized object_property_add_enum() method which simplifies the use of enum properties, so the setters/getters directly get passed the int value. typedef enum { MYDEV_TYPE_FROG, MYDEV_TYPE_ALLIGATOR, MYDEV_TYPE_PLATYPUS, MYDEV_TYPE_LAST } MyDevType; Then provide a table of enum <-> string mappings static const char *const mydevtypemap[MYDEV_TYPE_LAST + 1] = { [MYDEV_TYPE_FROG] = "frog", [MYDEV_TYPE_ALLIGATOR] = "alligator", [MYDEV_TYPE_PLATYPUS] = "platypus", [MYDEV_TYPE_LAST] = NULL, }; Assuming an object struct of typedef struct { Object parent_obj; MyDevType devtype; ...other fields... } MyDev; The property can then be registered as follows: static int mydev_prop_get_devtype(Object *obj, Error **errp G_GNUC_UNUSED) { MyDev *dev = MYDEV(obj); return dev->devtype; } static void mydev_prop_set_devtype(Object *obj, int value, Error **errp G_GNUC_UNUSED) { MyDev *dev = MYDEV(obj); dev->devtype = value; } object_property_add_enum(obj, "devtype", mydevtypemap, "MyDevType", mydev_prop_get_devtype, mydev_prop_set_devtype, NULL); Note there is no need to check the range of 'value' in the setter, because the string->enum conversion code will have already done that and reported an error as required. Signed-off-by: Daniel P. Berrange Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- include/qom/object.h | 19 ++++++++++ qom/object.c | 58 ++++++++++++++++++++++++++++++ tests/check-qom-proplist.c | 73 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+) diff --git a/include/qom/object.h b/include/qom/object.h index 701b4c57e7..a8a53049cb 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1343,6 +1343,25 @@ void object_property_add_bool(Object *obj, const char *name, void (*set)(Object *, bool, Error **), Error **errp); +/** + * object_property_add_enum: + * @obj: the object to add a property to + * @name: the name of the property + * @typename: the name of the enum data type + * @get: the getter or %NULL if the property is write-only. + * @set: the setter or %NULL if the property is read-only + * @errp: if an error occurs, a pointer to an area to store the error + * + * Add an enum property using getters/setters. This function will add a + * property of type '@typename'. + */ +void object_property_add_enum(Object *obj, const char *name, + const char *typename, + const char * const *strings, + int (*get)(Object *, Error **), + void (*set)(Object *, int, Error **), + Error **errp); + /** * object_property_add_tm: * @obj: the object to add a property to diff --git a/qom/object.c b/qom/object.c index 523307d839..8f0d8a7b13 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1069,6 +1069,12 @@ int64_t object_property_get_int(Object *obj, const char *name, return retval; } +typedef struct EnumProperty { + const char * const *strings; + int (*get)(Object *, Error **); + void (*set)(Object *, int, Error **); +} EnumProperty; + int object_property_get_enum(Object *obj, const char *name, const char * const strings[], Error **errp) { @@ -1673,6 +1679,58 @@ void object_property_add_bool(Object *obj, const char *name, } } +static void property_get_enum(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + EnumProperty *prop = opaque; + int value; + + value = prop->get(obj, errp); + visit_type_enum(v, &value, prop->strings, NULL, name, errp); +} + +static void property_set_enum(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + EnumProperty *prop = opaque; + int value; + + visit_type_enum(v, &value, prop->strings, NULL, name, errp); + prop->set(obj, value, errp); +} + +static void property_release_enum(Object *obj, const char *name, + void *opaque) +{ + EnumProperty *prop = opaque; + g_free(prop); +} + +void object_property_add_enum(Object *obj, const char *name, + const char *typename, + const char * const *strings, + int (*get)(Object *, Error **), + void (*set)(Object *, int, Error **), + Error **errp) +{ + Error *local_err = NULL; + EnumProperty *prop = g_malloc(sizeof(*prop)); + + prop->strings = strings; + prop->get = get; + prop->set = set; + + object_property_add(obj, name, typename, + get ? property_get_enum : NULL, + set ? property_set_enum : NULL, + property_release_enum, + prop, &local_err); + if (local_err) { + error_propagate(errp, local_err); + g_free(prop); + } +} + typedef struct TMProperty { void (*get)(Object *, struct tm *, Error **); } TMProperty; diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c index e82532e1f2..ae4cd4cde6 100644 --- a/tests/check-qom-proplist.c +++ b/tests/check-qom-proplist.c @@ -32,10 +32,28 @@ typedef struct DummyObjectClass DummyObjectClass; #define DUMMY_OBJECT(obj) \ OBJECT_CHECK(DummyObject, (obj), TYPE_DUMMY) +typedef enum DummyAnimal DummyAnimal; + +enum DummyAnimal { + DUMMY_FROG, + DUMMY_ALLIGATOR, + DUMMY_PLATYPUS, + + DUMMY_LAST, +}; + +static const char *const dummy_animal_map[DUMMY_LAST + 1] = { + [DUMMY_FROG] = "frog", + [DUMMY_ALLIGATOR] = "alligator", + [DUMMY_PLATYPUS] = "platypus", + [DUMMY_LAST] = NULL, +}; + struct DummyObject { Object parent_obj; bool bv; + DummyAnimal av; char *sv; }; @@ -62,6 +80,24 @@ static bool dummy_get_bv(Object *obj, } +static void dummy_set_av(Object *obj, + int value, + Error **errp) +{ + DummyObject *dobj = DUMMY_OBJECT(obj); + + dobj->av = value; +} + +static int dummy_get_av(Object *obj, + Error **errp) +{ + DummyObject *dobj = DUMMY_OBJECT(obj); + + return dobj->av; +} + + static void dummy_set_sv(Object *obj, const char *value, Error **errp) @@ -91,6 +127,12 @@ static void dummy_init(Object *obj) dummy_get_sv, dummy_set_sv, NULL); + object_property_add_enum(obj, "av", + "DummyAnimal", + dummy_animal_map, + dummy_get_av, + dummy_set_av, + NULL); } static void dummy_finalize(Object *obj) @@ -121,11 +163,13 @@ static void test_dummy_createv(void) &err, "bv", "yes", "sv", "Hiss hiss hiss", + "av", "platypus", NULL)); g_assert(err == NULL); g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss"); g_assert(dobj->bv == true); + g_assert(dobj->av == DUMMY_PLATYPUS); g_assert(object_resolve_path_component(parent, "dummy0") == OBJECT(dobj)); @@ -160,11 +204,13 @@ static void test_dummy_createlist(void) parent, "bv", "yes", "sv", "Hiss hiss hiss", + "av", "platypus", NULL)); g_assert(err == NULL); g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss"); g_assert(dobj->bv == true); + g_assert(dobj->av == DUMMY_PLATYPUS); g_assert(object_resolve_path_component(parent, "dummy0") == OBJECT(dobj)); @@ -172,6 +218,32 @@ static void test_dummy_createlist(void) object_unparent(OBJECT(dobj)); } +static void test_dummy_badenum(void) +{ + Error *err = NULL; + Object *parent = object_get_objects_root(); + Object *dobj = + object_new_with_props(TYPE_DUMMY, + parent, + "dummy0", + &err, + "bv", "yes", + "sv", "Hiss hiss hiss", + "av", "yeti", + NULL); + + g_assert(dobj == NULL); + g_assert(err != NULL); + g_assert_cmpstr(error_get_pretty(err), ==, + "Invalid parameter 'yeti'"); + + g_assert(object_resolve_path_component(parent, "dummy0") + == NULL); + + error_free(err); +} + + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -181,6 +253,7 @@ int main(int argc, char **argv) g_test_add_func("/qom/proplist/createlist", test_dummy_createlist); g_test_add_func("/qom/proplist/createv", test_dummy_createv); + g_test_add_func("/qom/proplist/badenum", test_dummy_badenum); return g_test_run(); } From a3590dacce94519c1747d8bf423744c6bb7d9941 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 27 May 2015 16:07:56 +0100 Subject: [PATCH 12/14] qom: Don't pass string table to object_get_enum() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that properties can be explicitly registered as an enum type, there is no need to pass the string table to the object_get_enum() function. The object property registration already has a pointer to the string table. In changing this method signature, the hostmem backend object has to be converted to use the new enum property registration code, which simplifies it somewhat. Signed-off-by: Daniel P. Berrange Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- backends/hostmem.c | 22 +++++++------------ include/qom/object.h | 4 ++-- numa.c | 2 +- qom/object.c | 19 +++++++++++++++-- tests/check-qom-proplist.c | 43 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 19 deletions(-) diff --git a/backends/hostmem.c b/backends/hostmem.c index f6db33c14e..7d74be04c3 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -113,24 +113,17 @@ host_memory_backend_set_host_nodes(Object *obj, Visitor *v, void *opaque, #endif } -static void -host_memory_backend_get_policy(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +static int +host_memory_backend_get_policy(Object *obj, Error **errp G_GNUC_UNUSED) { HostMemoryBackend *backend = MEMORY_BACKEND(obj); - int policy = backend->policy; - - visit_type_enum(v, &policy, HostMemPolicy_lookup, NULL, name, errp); + return backend->policy; } static void -host_memory_backend_set_policy(Object *obj, Visitor *v, void *opaque, - const char *name, Error **errp) +host_memory_backend_set_policy(Object *obj, int policy, Error **errp) { HostMemoryBackend *backend = MEMORY_BACKEND(obj); - int policy; - - visit_type_enum(v, &policy, HostMemPolicy_lookup, NULL, name, errp); backend->policy = policy; #ifndef CONFIG_NUMA @@ -252,9 +245,10 @@ static void host_memory_backend_init(Object *obj) object_property_add(obj, "host-nodes", "int", host_memory_backend_get_host_nodes, host_memory_backend_set_host_nodes, NULL, NULL, NULL); - object_property_add(obj, "policy", "HostMemPolicy", - host_memory_backend_get_policy, - host_memory_backend_set_policy, NULL, NULL, NULL); + object_property_add_enum(obj, "policy", "HostMemPolicy", + HostMemPolicy_lookup, + host_memory_backend_get_policy, + host_memory_backend_set_policy, NULL); } MemoryRegion * diff --git a/include/qom/object.h b/include/qom/object.h index a8a53049cb..807978eec7 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -1073,7 +1073,7 @@ int64_t object_property_get_int(Object *obj, const char *name, * object_property_get_enum: * @obj: the object * @name: the name of the property - * @strings: strings corresponding to enums + * @typename: the name of the enum data type * @errp: returns an error if this function fails * * Returns: the value of the property, converted to an integer, or @@ -1081,7 +1081,7 @@ int64_t object_property_get_int(Object *obj, const char *name, * an enum). */ int object_property_get_enum(Object *obj, const char *name, - const char * const strings[], Error **errp); + const char *typename, Error **errp); /** * object_property_get_uint16List: diff --git a/numa.c b/numa.c index e67322a69b..28c857c66a 100644 --- a/numa.c +++ b/numa.c @@ -456,7 +456,7 @@ static int query_memdev(Object *obj, void *opaque) m->value->policy = object_property_get_enum(obj, "policy", - HostMemPolicy_lookup, + "HostMemPolicy", &err); if (err) { goto error; diff --git a/qom/object.c b/qom/object.c index 8f0d8a7b13..ee384311f9 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1076,12 +1076,27 @@ typedef struct EnumProperty { } EnumProperty; int object_property_get_enum(Object *obj, const char *name, - const char * const strings[], Error **errp) + const char *typename, Error **errp) { StringOutputVisitor *sov; StringInputVisitor *siv; char *str; int ret; + ObjectProperty *prop = object_property_find(obj, name, errp); + EnumProperty *enumprop; + + if (prop == NULL) { + return 0; + } + + if (!g_str_equal(prop->type, typename)) { + error_setg(errp, "Property %s on %s is not '%s' enum type", + name, object_class_get_name( + object_get_class(obj)), typename); + return 0; + } + + enumprop = prop->opaque; sov = string_output_visitor_new(false); object_property_get(obj, string_output_get_visitor(sov), name, errp); @@ -1089,7 +1104,7 @@ int object_property_get_enum(Object *obj, const char *name, siv = string_input_visitor_new(str); string_output_visitor_cleanup(sov); visit_type_enum(string_input_get_visitor(siv), - &ret, strings, NULL, name, errp); + &ret, enumprop->strings, NULL, name, errp); g_free(str); string_input_visitor_cleanup(siv); diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c index ae4cd4cde6..7400b1fce9 100644 --- a/tests/check-qom-proplist.c +++ b/tests/check-qom-proplist.c @@ -244,6 +244,48 @@ static void test_dummy_badenum(void) } +static void test_dummy_getenum(void) +{ + Error *err = NULL; + int val; + Object *parent = object_get_objects_root(); + DummyObject *dobj = DUMMY_OBJECT( + object_new_with_props(TYPE_DUMMY, + parent, + "dummy0", + &err, + "av", "platypus", + NULL)); + + g_assert(err == NULL); + g_assert(dobj->av == DUMMY_PLATYPUS); + + val = object_property_get_enum(OBJECT(dobj), + "av", + "DummyAnimal", + &err); + g_assert(err == NULL); + g_assert(val == DUMMY_PLATYPUS); + + /* A bad enum type name */ + val = object_property_get_enum(OBJECT(dobj), + "av", + "BadAnimal", + &err); + g_assert(err != NULL); + error_free(err); + err = NULL; + + /* A non-enum property name */ + val = object_property_get_enum(OBJECT(dobj), + "iv", + "DummyAnimal", + &err); + g_assert(err != NULL); + error_free(err); +} + + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -254,6 +296,7 @@ int main(int argc, char **argv) g_test_add_func("/qom/proplist/createlist", test_dummy_createlist); g_test_add_func("/qom/proplist/createv", test_dummy_createv); g_test_add_func("/qom/proplist/badenum", test_dummy_badenum); + g_test_add_func("/qom/proplist/getenum", test_dummy_getenum); return g_test_run(); } From 0210afe6690be045cb849b2f16bffabda575a9bf Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 19 Jun 2015 16:17:22 +0200 Subject: [PATCH 13/14] qdev: Deprecated qdev_init() is finally unused, drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qdev_init() is a wrapper around setting property "realized" to true, plus error handling that passes errors to qerror_report_err(). qerror_report_err() is a transitional interface to help with converting existing monitor commands to QMP. It should not be used elsewhere. All code has been modernized to avoid qdev_init() and its inappropriate error handling. We can finally drop it. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- hw/core/qdev.c | 47 ++++++++++++++---------------------------- include/hw/qdev-core.h | 3 +-- 2 files changed, 17 insertions(+), 33 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index d10fa5fbf6..a6353c177f 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -126,9 +126,9 @@ void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp) qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp); } -/* Create a new device. This only initializes the device state structure - and allows properties to be set. qdev_init should be called to - initialize the actual device emulation. */ +/* Create a new device. This only initializes the device state + structure and allows properties to be set. The device still needs + to be realized. See qdev-core.h. */ DeviceState *qdev_create(BusState *bus, const char *name) { DeviceState *dev; @@ -168,27 +168,6 @@ DeviceState *qdev_try_create(BusState *bus, const char *type) return dev; } -/* Initialize a device. Device properties should be set before calling - this function. IRQs and MMIO regions should be connected/mapped after - calling this function. - On failure, destroy the device and return negative value. - Return 0 on success. */ -int qdev_init(DeviceState *dev) -{ - Error *local_err = NULL; - - assert(!dev->realized); - - object_property_set_bool(OBJECT(dev), true, "realized", &local_err); - if (local_err != NULL) { - qerror_report_err(local_err); - error_free(local_err); - object_unparent(OBJECT(dev)); - return -1; - } - return 0; -} - static QTAILQ_HEAD(device_listeners, DeviceListener) device_listeners = QTAILQ_HEAD_INITIALIZER(device_listeners); @@ -364,13 +343,19 @@ void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, object_unparent(OBJECT(dev)); } -/* Like qdev_init(), but terminate program via error_report() instead of - returning an error value. This is okay during machine creation. - Don't use for hotplug, because there callers need to recover from - failure. Exception: if you know the device's init() callback can't - fail, then qdev_init_nofail() can't fail either, and is therefore - usable even then. But relying on the device implementation that - way is somewhat unclean, and best avoided. */ +/* + * Realize @dev. + * Device properties should be set before calling this function. IRQs + * and MMIO regions should be connected/mapped after calling this + * function. + * On failure, report an error with error_report() and terminate the + * program. This is okay during machine creation. Don't use for + * hotplug, because there callers need to recover from failure. + * Exception: if you know the device's init() callback can't fail, + * then qdev_init_nofail() can't fail either, and is therefore usable + * even then. But relying on the device implementation that way is + * somewhat unclean, and best avoided. + */ void qdev_init_nofail(DeviceState *dev) { Error *err = NULL; diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index ad9eb89e59..64154a929c 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -66,7 +66,7 @@ struct VMStateDescription; * After successful realization, setting static properties will fail. * * As an interim step, the #DeviceState:realized property is set by deprecated - * functions qdev_init() and qdev_init_nofail(). + * function qdev_init_nofail(). * In the future, devices will propagate this state change to their children * and along busses they expose. * The point in time will be deferred to machine creation, so that values @@ -262,7 +262,6 @@ typedef struct GlobalProperty { DeviceState *qdev_create(BusState *bus, const char *name); DeviceState *qdev_try_create(BusState *bus, const char *name); -int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT; void qdev_init_nofail(DeviceState *dev); void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, int required_for_version); From daeba9699d41ad79e2f3d34acea9c85c5d67a2ac Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 19 Jun 2015 16:17:23 +0200 Subject: [PATCH 14/14] qdev: Un-deprecate qdev_init_nofail() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's a perfectly sensible helper function. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Signed-off-by: Andreas Färber --- include/hw/qdev-core.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 64154a929c..038b54d94b 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -65,8 +65,8 @@ struct VMStateDescription; * Operations depending on @props static properties should go into @realize. * After successful realization, setting static properties will fail. * - * As an interim step, the #DeviceState:realized property is set by deprecated - * function qdev_init_nofail(). + * As an interim step, the #DeviceState:realized property can also be + * set with qdev_init_nofail(). * In the future, devices will propagate this state change to their children * and along busses they expose. * The point in time will be deferred to machine creation, so that values