From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42898) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj7q8-0003cs-P1 for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:05:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yj7q2-0002lJ-NG for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:05:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39171) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yj7q2-0002l8-CM for qemu-devel@nongnu.org; Fri, 17 Apr 2015 11:05:50 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 0AA21A0E7A for ; Fri, 17 Apr 2015 15:05:50 +0000 (UTC) Message-ID: <55312148.6070005@redhat.com> Date: Fri, 17 Apr 2015 17:05:44 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1429280557-8887-1-git-send-email-berrange@redhat.com> <1429280557-8887-8-git-send-email-berrange@redhat.com> In-Reply-To: <1429280557-8887-8-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v1 RFC 07/34] qom: don't pass string table to object_get_enum method List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Gerd Hoffmann , Stefan Hajnoczi On 17/04/2015 16:22, Daniel P. Berrange wrote: > 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. >=20 > 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. >=20 > Signed-off-by: Daniel P. Berrange > --- > backends/hostmem.c | 22 ++++++++-------------- > include/qom/object.h | 3 +-- > numa.c | 1 - > qom/object.c | 32 ++++++++++++++++++++++++-------- > 4 files changed, 33 insertions(+), 25 deletions(-) >=20 > diff --git a/backends/hostmem.c b/backends/hostmem.c > index b7b6cf8..c3b9df1 100644 > --- a/backends/hostmem.c > +++ b/backends/hostmem.c > @@ -113,24 +113,17 @@ host_memory_backend_set_host_nodes(Object *obj, V= isitor *v, void *opaque, > #endif > } > =20 > -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 =3D MEMORY_BACKEND(obj); > - int policy =3D backend->policy; > - > - visit_type_enum(v, &policy, HostMemPolicy_lookup, NULL, name, errp= ); > + return backend->policy; > } > =20 > 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 =3D MEMORY_BACKEND(obj); > - int policy; > - > - visit_type_enum(v, &policy, HostMemPolicy_lookup, NULL, name, errp= ); > backend->policy =3D policy; > =20 > #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", "str", > - host_memory_backend_get_policy, > - host_memory_backend_set_policy, NULL, NULL, NU= LL); > + object_property_add_enum(obj, "policy", > + HostMemPolicy_lookup, > + host_memory_backend_get_policy, > + host_memory_backend_set_policy, NULL); > } > =20 > MemoryRegion * > diff --git a/include/qom/object.h b/include/qom/object.h > index 3462821..bfad22f 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -1003,7 +1003,6 @@ int64_t object_property_get_int(Object *obj, cons= t char *name, > * object_property_get_enum: > * @obj: the object > * @name: the name of the property > - * @strings: strings corresponding to enums > * @errp: returns an error if this function fails > * > * Returns: the value of the property, converted to an integer, or > @@ -1011,7 +1010,7 @@ int64_t object_property_get_int(Object *obj, cons= t char *name, > * an enum). > */ > int object_property_get_enum(Object *obj, const char *name, > - const char * const strings[], Error **err= p); > + Error **errp); > =20 > /** > * object_property_get_uint16List: > diff --git a/numa.c b/numa.c > index c975fb2..881a123 100644 > --- a/numa.c > +++ b/numa.c > @@ -457,7 +457,6 @@ static int query_memdev(Object *obj, void *opaque) > =20 > m->value->policy =3D object_property_get_enum(obj, > "policy", > - HostMemPolicy_look= up, > &err); > if (err) { > goto error; > diff --git a/qom/object.c b/qom/object.c > index 543cc57..e99b3c9 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1024,13 +1024,35 @@ int64_t object_property_get_int(Object *obj, co= nst char *name, > return retval; > } > =20 > +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 **err= p) > + Error **errp) > { > StringOutputVisitor *sov; > StringInputVisitor *siv; > char *str; > int ret; > + ObjectProperty *prop =3D object_property_find(obj, name, errp); > + EnumProperty *enumprop; > + > + if (prop =3D=3D NULL) { > + return 0; > + } > + > + if (!g_str_equal(prop->type, "enum")) { Inspecting the type is a bit gross, and in fact it breaks if you keep the QAPI type name in patch 6. The HostMemPolicy conversion is certainly okay, but unfortunately this is not. If you decide to go with macros in patch 6, perhaps you can do the same here so that object_property_get_enum(obj, "policy", HostMemPolicy, &err) can compare against the "HostMemPolicy" type. The fact that HostMemPolicy_lookup exists is then a (mild, but sufficient) guarantee that the opaque pointer of the property is really an EnumProperty pointer. Paolo > + error_setg(errp, "Property %s on %s is not an 'enum' type", > + name, object_class_get_name( > + object_get_class(obj))); > + return 0; > + } > + > + enumprop =3D prop->opaque; > =20 > sov =3D string_output_visitor_new(false); > object_property_get(obj, string_output_get_visitor(sov), name, err= p); > @@ -1038,7 +1060,7 @@ int object_property_get_enum(Object *obj, const c= har *name, > siv =3D 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); > =20 > g_free(str); > string_input_visitor_cleanup(siv); > @@ -1607,12 +1629,6 @@ void object_property_add_bool(Object *obj, const= char *name, > } > } > =20 > -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) > { >=20