[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 7/7] qom: don't pass string table to object_g
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH v3 7/7] qom: don't pass string table to object_get_enum method |
Date: |
Fri, 08 May 2015 19:54:48 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
Am 01.05.2015 um 12:30 schrieb Daniel P. Berrange:
> 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 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.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> backends/hostmem.c | 22 ++++++++--------------
> include/qom/object.h | 4 ++--
> numa.c | 2 +-
> qom/object.c | 32 ++++++++++++++++++++++++--------
> tests/check-qom-proplist.c | 46
> ++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 81 insertions(+), 25 deletions(-)
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index f6db33c..7d74be0 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 f6a2a9d..fc347b9 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1012,7 +1012,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
> @@ -1020,7 +1020,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 c975fb2..a64279a 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -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;
> diff --git a/qom/object.c b/qom/object.c
> index ba0e4b8..6d2a2a9 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1026,13 +1026,35 @@ 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 **);
Since get and set and moved unchanged, I would prefer placing it in the
final destination in the original patch to avoid churn.
> +} 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);
> @@ -1040,7 +1062,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);
> @@ -1609,12 +1631,6 @@ void object_property_add_bool(Object *obj, const char
> *name,
> }
> }
>
> -typedef struct EnumProperty {
> - const char * const *strings;
> - int (*get)(Object *, Error **);
> - void (*set)(Object *, int, Error **);
> -} EnumProperty;
> -
> static void property_get_enum(Object *obj, Visitor *v, void *opaque,
> const char *name, Error **errp)
> {
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index de142e3..d5cd38b 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -249,6 +249,51 @@ static void test_dummy_badenum(void)
> }
>
>
> +
> +static void test_dummy_getenum(void)
> +{
> + Error *err = NULL;
> + int val;
> + Object *parent = container_get(object_get_root(),
> + "/objects");
> + DummyObject *dobj = DUMMY_OBJECT(
> + object_new_propv(TYPE_DUMMY,
> + parent,
> + "dummy0",
> + &err,
> + "av", "platypus",
> + NULL));
> +
> + g_assert(dobj != 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);
Is there any significant difference between g_assert()'ing on error and
passing &error_abort?
> + 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);
> @@ -259,6 +304,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();
> }
Otherwise looks good (apart from the dependencies).
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)
[Qemu-devel] [PATCH v3 5/7] qom: make enum string tables const-correct, Daniel P. Berrange, 2015/05/01
[Qemu-devel] [PATCH v3 6/7] qom: add a object_property_add_enum helper method, Daniel P. Berrange, 2015/05/01
[Qemu-devel] [PATCH v3 7/7] qom: don't pass string table to object_get_enum method, Daniel P. Berrange, 2015/05/01
- Re: [Qemu-devel] [PATCH v3 7/7] qom: don't pass string table to object_get_enum method,
Andreas Färber <=
Re: [Qemu-devel] [PATCH v3 0/7] qom: misc fixes & enhancements to support TLS work, Paolo Bonzini, 2015/05/05