qom: misc fixes & enhancements to support TLS work
v5 cover qom: misc fixes & enhancements to support TLS work
This series contains the 7^H8 generic QOM API fixes and enhancements that I previously posted as part of the large series refactoring and extending the TLS support in QEMU:
https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02038.html
I'm sending it separately, since the patches are reasonably well self-contained and thus hopefully suitable for quicker review and merge.
The changes in this posting are in response to Andreas' feedback on v3.
Changed in v5:
- Fix test suite build -Wunused-value warning on some GCC versions reported by Peter Maydell
Changed in v4:
- Introduce object_get_objects_root() to replace duplicated container_get(object_get_root(), "/objects") calls
- Add object_set_prop[sv]()and call them to populate properties from object_new_with_prop[sv]() constructor
- Use <programlisting> in inline docs example
- Fix naming for '...' vs 'va_list' arg method variants to match QEMU convention (va_list one has the 'v' suffix)
- Conditionally use __attribute__((sentinel)) only on new enough GCC (>= 4.0)
- Use a local Error * object where Error **errp could be NULL
- Change class def to use parent_obj and parent_class for first struct member
- Use g_assert_cmpstr() in test case assertions on strings
- Other misc typos in docs, commit messages and code.
Changed in v3:
- Fix test suite additions for change in object_new_propv API
Changed in v2:
- Pass "Object * parent" instead of "char *path" paremeter
- Rely on stable reference from parent to keep new object alive
- Use object_unparent() where appropriate
Daniel P. Berrange (8):
backends: fix typename of 'policy' enum property in hostmem obj
doc: document user creatable object types in help text
vl: create (most) objects before creating chardev backends
qom: add helper method for getting user objects root
qom: add object_new_with_props / object_new_withpropv constructors
qom: make enum string tables const-correct
qom: add a object_property_add_enum helper method
qom: don't pass string table to object_get_enum method
backends/hostmem.c | 22 ++-- include/hw/qdev-core.h | 2 +- include/qapi/util.h | 2 +- include/qapi/visitor-impl.h | 6 +- include/qapi/visitor.h | 2 +- include/qemu/compiler.h | 6 + include/qom/object.h | 163 +++++++++++++++++++++++- iothread.c | 4 +- numa.c | 4 +- qapi/qapi-dealloc-visitor.c | 3 +- qapi/qapi-util.c | 2 +- qapi/qapi-visit-core.c | 6 +- qemu-options.hx | 70 +++++++--- qmp.c | 6 +- qom/object.c | 191 +++++++++++++++++++++++++++- scripts/qapi-types.py | 4 +- tests/.gitignore | 1 + tests/Makefile | 5 +- tests/check-qom-proplist.c | 302 ++++++++++++++++++++++++++++++++++++++++++++ vl.c | 40 +++++- 20 files changed, 784 insertions(+), 57 deletions(-) create mode 100644 tests/check-qom-proplist.c
Signature · 1 line
2.4.1
PATCH v5 1/8 backends: fix typename of 'policy' enum property in hostmem obj
The 'policy' property was being registered with a typename of 'str', but it is in fact an enum of the 'HostMemPolicy' type.
--- backends/hostmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
backends/hostmem.c +1 -1
@@ -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 ); }
Signature · 1 line
2.4.1
PATCH v5 2/8 doc: document user creatable object types in help text
In reply to Daniel P. BerrangeThe 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.
--- qemu-options.hx | 70 ++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 16 deletions(-)
qemu-options.hx +54 -16
@@ -3421,22 +3421,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"
@@ -3462,6 +3446,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
Signature · 1 line
2.4.1
PATCH v5 3/8 vl: create (most) objects before creating chardev backends
In reply to Daniel P. BerrangeSome 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.
--- vl.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)
vl.c +39 -1
@@ -2601,6 +2601,33 @@ static int machine_set_property(const char *name, const char *value,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 (QemuOpts *opts,void *opaque) {Error *err =NULL ;
@@ -2609,6 +2636,7 @@ static int object_create(QemuOpts *opts, void *opaque)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 );
@@ -2623,6 +2651,9 @@ static int object_create(QemuOpts *opts, void *opaque)if (err) { goto out; } +if (!type_predicate (type)) { + goto out; + }qdict_del (pdict,"id" );visit_type_str (opts_get_visitor (ov), &id,"id" , &err);
@@ -4031,6 +4062,12 @@ int main(int argc, char **argv, char **envp)socket_init (); +if (qemu_opts_foreach (qemu_find_opts ("object" ), + object_create, + object_create_initial,0 ) !=0 ) { +exit (1 ); + } +if (qemu_opts_foreach (qemu_find_opts ("chardev" ), chardev_init_func,NULL ,1 ) !=0 )exit (1 );#ifdef CONFIG_VIRTFS
@@ -4050,7 +4087,8 @@ int main(int argc, char **argv, char **envp) }if (qemu_opts_foreach (qemu_find_opts ("object" ), - object_create,NULL ,0 ) !=0 ) { + object_create, + object_create_delayed,0 ) !=0 ) {exit (1 ); }
Signature · 1 line
2.4.1
PATCH v5 4/8 qom: add helper method for getting user objects root
In reply to Daniel P. BerrangeAdd object_get_objects_root() method which is a convience for obtaining the Object * located at /objects in the object composition tree. Convert existing code over to use the new API where appropriate.
--- 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(-)
include/qom/object.h +12 -0
@@ -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: *
iothread.c +1 -3
@@ -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) \
@@ -153,7 +151,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;
numa.c +1 -1
@@ -486,7 +486,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 ; }
qmp.c +3 -3
@@ -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" );
qom/object.c +5 -0
@@ -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) {
Signature · 1 line
2.4.1
Re: [Qemu-devel] [PATCH v5 4/8] qom: add helper method for getting user objects root
Am 27.05.2015 um 17:07 schrieb Daniel P. Berrange:
Quoted 1 line
Add
object_get_objects_root()method which is a convience for
"convenience", I assume - fixing.
Andreas
Quoted 5 lines
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 <[email protected]>
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
PATCH v5 5/8 qom: add object_new_with_props / object_new_withpropv constructors
In reply to Daniel P. BerrangeIt 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.
--- 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
include/qemu/compiler.h +6 -0
@@ -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
include/qom/object.h +128 -0
134 lines not on this page · raw message
qom/object.c +109 -0
121 lines not on this page · raw message
tests/.gitignore +1 -0
@@ -5,6 +5,7 @@ check-qjson check-qlist check-qstring check-qom-interface +check-qom-proplist rcutorture test-aio test-bitops
tests/Makefile +4 -1
@@ -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
tests/check-qom-proplist.c new file +186 -0
186 lines not on this page · raw message
Signature · 1 line
2.4.1
Re: [Qemu-devel] [PATCH v5 5/8] qom: add object_new_with_props / object_new_withpropv constructors
Quoted 6 lines · Daniel P. Berrange
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.
Quoted 14 lines
/**
+ *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/
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v5 5/8] qom: add object_new_with_props / object_new_withpropv constructors
Am 19.06.2015 um 18:04 schrieb Eric Blake:
Quoted 22 lines · Daniel P. Berrange
Quoted 6 lines
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.
Quoted 14 lines
/**
+ *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.
Quoted 1 line
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
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH v5 5/8] qom: add object_new_with_props / object_new_withpropv constructors
Quoted 23 lines · Andreas Färber
Am 19.06.2015 um 18:04 schrieb Eric Blake:
Quoted 22 lines · Daniel P. Berrange
Quoted 20 lines
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.
Quoted 3 lines
Quoted 1 line
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).
Quoted 8 lines
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
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v5 5/8] qom: add object_new_with_props / object_new_withpropv constructors
Am 19.06.2015 um 18:17 schrieb Eric Blake:
Quoted 24 lines · Andreas Färber
Quoted 23 lines
Am 19.06.2015 um 18:04 schrieb Eric Blake:
Quoted 22 lines · Daniel P. Berrange
Quoted 20 lines
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.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
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
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
PATCH v5 6/8 qom: make enum string tables const-correct
In reply to Daniel P. BerrangeThe 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.
--- 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(-)
include/hw/qdev-core.h +1 -1
@@ -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;
include/qapi/util.h +1 -1
@@ -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
include/qapi/visitor-impl.h +3 -3
@@ -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
include/qapi/visitor.h +1 -1
@@ -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);
include/qom/object.h +1 -1
@@ -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:
qapi/qapi-dealloc-visitor.c +2 -1
@@ -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) {
qapi/qapi-util.c +1 -1
@@ -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;
qapi/qapi-visit-core.c +3 -3
@@ -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) {
qom/object.c +1 -1
@@ -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;
scripts/qapi-types.py +2 -2
@@ -109,7 +109,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
@@ -132,7 +132,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)
Signature · 1 line
2.4.1
Re: [Qemu-devel] [PATCH v5 6/8] qom: make enum string tables const-correct
Quoted 5 lines · Daniel P. Berrange
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[] = { .... };
Not sure if the style would be '*const' instead of '* const' everywhere, but if checkpatch.pl didn't complain, it's not worth a respin.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH v5 7/8 qom: add a object_property_add_enum helper method
In reply to Daniel P. BerrangeA QOM property can be parsed as enum using the visit_type_enum() helper method, 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;
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->endpoint = 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.
--- include/qom/object.h | 19 ++++++++++++ qom/object.c | 58 ++++++++++++++++++++++++++++++++++++ tests/check-qom-proplist.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+)
include/qom/object.h +19 -0
@@ -1344,6 +1344,25 @@ void object_property_add_bool(Object *obj, const char *name, 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 a 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 * @name: the name of the property
qom/object.c +58 -0
70 lines not on this page · raw message
tests/check-qom-proplist.c +73 -0
129 lines not on this page · raw message
Signature · 1 line
2.4.1
Re: [Qemu-devel] [PATCH v5 7/8] qom: add a object_property_add_enum helper method
Quoted 12 lines · Daniel P. Berrange
A QOM property can be parsed as enum using the
visit_type_enum()helper method, but this forces callers to use the more complex genericobject_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 {
Quoted 10 lines
/**
+ *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 a enum property using getters/setters. This function will add a
s/a enum/an enum/
qom/object.c void object_property_add_bool(Object *obj, const char *name, · line 1690
Quoted 5 lines
qom/object.c +5
-0+static void property_release_enum (Object *obj,const char *name, +void *opaque) +{ +EnumProperty *prop = opaque; +g_free (prop);
Could also be written as g_free(opaque), but that's a little less readable, so I don't see a point in changing it.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v5 7/8] qom: add a object_property_add_enum helper method
Am 19.06.2015 um 18:12 schrieb Eric Blake:
Quoted 24 lines · Daniel P. Berrange
Quoted 12 lines
A QOM property can be parsed as enum using the
visit_type_enum()helper method, but this forces callers to use the more complex genericobject_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 {Quoted 10 lines
/**
+ *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 a enum property using getters/setters. This function will add a
Quoted 1 line
s/a enum/an enum/
Thanks, fixed on https://github.com/afaerber/qemu-cpu/commits/qom-next
Quoted 8 lines
qom/object.c void object_property_add_bool(Object *obj, const char *name, · line 1690
Quoted 5 lines
qom/object.c +5
-0+static void property_release_enum (Object *obj,const char *name, +void *opaque) +{ +EnumProperty *prop = opaque; +g_free (prop);Could also be written as
g_free(opaque), but that's a little less readable, so I don't see a point in changing it.
I prefer it this way. Consuming opaques directly broke a few call sites when I refactored the CPU structs, so the savings of one line is not really worth it IMO.
Regards,
Andreas
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Re: [Qemu-devel] [PATCH v5 7/8] qom: add a object_property_add_enum helper method
In reply to Daniel P. BerrangeQuoted 25 lines · Daniel P. Berrange
A QOM property can be parsed as enum using the
visit_type_enum()helper method, but this forces callers to use the more complex genericobject_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.Assuming an object struct of
typedef struct { Object parent; 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;}
Quoted 7 lines
static void mydev_prop_set_devtype(Object *obj, int value, Error **errp G_GNUC_UNUSED) { MyDev *dev = MYDEV(obj);dev->endpoint = value;
s/endpoint/devtype/
Quoted 2 lines
Signed-off-by Daniel P. Berrange <[email protected]>---
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH v5 7/8] qom: add a object_property_add_enum helper method
Am 19.06.2015 um 18:38 schrieb Eric Blake:
Quoted 32 lines · Daniel P. Berrange
Quoted 31 lines
A QOM property can be parsed as enum using the
visit_type_enum()helper method, but this forces callers to use the more complex genericobject_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.Assuming an object struct of
typedef struct { Object parent; 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);
Quoted 3 lines
Quoted 1 line
dev->endpoint = value;s/endpoint/devtype/
Fixed, and s/parent/parent_obj/ while at it.
Andreas
Quoted 4 lines
Quoted 3 lines
Signed-off-by Daniel P. Berrange <[email protected]>---
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
PATCH v5 8/8 qom: don't pass string table to object_get_enum method
In reply to Daniel P. BerrangeNow that properties can be explicitly registered as an enum type, there is no need to pass the string table to the object_get_enum method. 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.
--- 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(-)
backends/hostmem.c +8 -14
@@ -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 *
include/qom/object.h +2 -2
@@ -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:
numa.c +1 -1
@@ -457,7 +457,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;
qom/object.c +17 -2
@@ -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);
tests/check-qom-proplist.c +43 -0
55 lines not on this page · raw message
Signature · 1 line
2.4.1
Re: [Qemu-devel] [PATCH v5 8/8] qom: don't pass string table to object_get_enum method
Am 27.05.2015 um 17:07 schrieb Daniel P. Berrange:
Quoted 9 lines
Now that properties can be explicitly registered as an enum type, there is no need to pass the string table to the
object_get_enummethod. 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.
Quoted 8 lines
Signed-off-by Daniel P. Berrange <[email protected]>--- 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(-)
There's been no protest after it appeared in v4, so I'm applying it for my v3 pull. If anyone has objections, please speak up now.
Regards,
Andreas
Signature · 3 lines
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)
Eric Blake
Reviewed-byRe: [Qemu-devel] [PATCH v5 8/8] qom: don't pass string table to object_get_enum method
Quoted 19 lines · Andreas Färber
Am 27.05.2015 um 17:07 schrieb Daniel P. Berrange:
Quoted 9 lines
Now that properties can be explicitly registered as an enum type, there is no need to pass the string table to the
object_get_enummethod. 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.
Quoted 8 lines
Signed-off-by Daniel P. Berrange <[email protected]>--- 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(-)
Quoted 2 lines
There's been no protest after it appeared in v4, so I'm applying it for my v3 pull. If anyone has objections, please speak up now.
I pointed out a couple of minor issues, but they are not showstoppers, so you can add to the series:
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org