From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37102) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bn3dM-0003Vl-H1 for qemu-devel@nongnu.org; Thu, 22 Sep 2016 09:01:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bn3dG-0002LE-EQ for qemu-devel@nongnu.org; Thu, 22 Sep 2016 09:01:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38980) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bn3dG-0002KE-5O for qemu-devel@nongnu.org; Thu, 22 Sep 2016 09:01:42 -0400 Date: Thu, 22 Sep 2016 14:01:37 +0100 From: "Daniel P. Berrange" Message-ID: <20160922130136.GO352@redhat.com> Reply-To: "Daniel P. Berrange" References: <20160911055301.26650-1-lma@suse.com> <20160911055301.26650-1-lma@suse.com> <87zindrup8.fsf@dusky.pond.sub.org> <57DDA45C02000062000996DE@prv-mh.provo.novell.com> <87fuowje3u.fsf@dusky.pond.sub.org> <9ae54df6-e098-8ca9-53c8-1a65eba32dae@suse.de> <87r38f7qzm.fsf@dusky.pond.sub.org> <57E31E25020000620009B116@prv-mh.provo.novell.com> <87shssnxeq.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <87shssnxeq.fsf@dusky.pond.sub.org> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] =?utf-8?b?562U5aSN77yaIFJlOiBbUEFUQ0ggdjJdIG9iamVj?= =?utf-8?q?t=3A_Add_=27help=27_option_for_all_available_backends_and_prope?= =?utf-8?q?rties?= List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Lin Ma , pbonzini@redhat.com, afaerber@suse.de, qemu-devel@nongnu.org On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote: > "Lin Ma" writes: >=20 > >>>> Markus Armbruster 2016/9/20 =E6=98=9F=E6=9C=9F= =E4=BA=8C =E4=B8=8A=E5=8D=88 1:13 >>> > >>Andreas F=C3=A4rber writes: > > Saving acceptable values of enumeration types into member description > > of ObjectProperty is a good idea. > > =20 > > The member description of ObjectProperty instance of any user-creatab= le > > object is NULL so far, >=20 > It's null until set with object_property_set_description(). We do that > for a few properties, and may do it more. >=20 > > If I use object_property_set_description() to = fill the > > acceptable values of enumeration type into the description in functio= n > > object_property_add_enum and object_class_property_add_enum, Then I > > can use this description to judge whether a ObjectProperty instance' = type > > is enumeration or not in function user_creatable_help_func. In this c= ase, If > > member description is not NULL, it means this ObjectProperty instance= is > > an enumeration. >=20 > No. If you need to decide in user_creatable_help_func() whether a > property has an enumeration type, something's wrong at the > infrastructure level. >=20 > > Any suggestions? >=20 > Yes: >=20 > >>When it's null we could still fall back to a description of the type. > >>Does such a thing exist? Enumeration types could provide one listing > >>their values. >=20 > Don't make up a description in user_creatable_help_func(), improve the > description infrastructure and its use so you get more useful ones > there. >=20 > The existing description infrastructure is just Property member > description and object_property_set_description(). Rarely used, so > description is generally null. >=20 > Calling object_property_set_description() more often could be helpful, > but to come up with a sensible description string, you need to know wha= t > the property does. Needs to be left to people actually familiar with > the objects. >=20 > Aside: historically, we add properties to *instances*. All the propert= y > meta-data gets duplicated for every instance, including property > descriptions. This is more flexible than adding the meta-data to the > class. The flexibility is rarely needed, but the price in wasted memor= y > is always paid. Only since commit 16bf7f5, we can add it to classes. > Adding lots of helpful property descriptions would increase the cost of > instance properties further. >=20 > What you could perhaps do is adding a *type* description. For enums, > that would show the set of acceptable values. Then if the property has > no description, fall back to the description of its type. I don't think we need to invent anything new. We can use the existing property description facility and auto-generate the string containing the permitted values thus: diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index dabc42e..0446839 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -202,9 +202,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): self._btin +=3D gen_enum(name, values, prefix) if do_builtins: self.defn +=3D gen_enum_lookup(name, values, prefix) + self._btin +=3D gen_enum_value_str(name, values) else: self._fwdecl +=3D gen_enum(name, values, prefix) self.defn +=3D gen_enum_lookup(name, values, prefix) + self._fwdecl +=3D gen_enum_value_str(name, values) =20 def visit_array_type(self, name, info, element_type): if isinstance(element_type, QAPISchemaBuiltinType): diff --git a/scripts/qapi.py b/scripts/qapi.py index 21bc32f..d11c414 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1649,6 +1649,15 @@ const char *const %(c_name)s_lookup[] =3D { return ret =20 =20 +def gen_enum_value_str(name, values): + return mcgen(''' + +#define %(c_name)s_value_str "%(value_str)s" +''', + c_name=3Dc_name(name), + value_str=3D", ".join(["'%s'" % c for c in values])) + + def gen_enum(name, values, prefix=3DNone): # append automatically generated _MAX value enum_values =3D values + ['_MAX'] Now, when registering an enum property do something like this: object_class_property_add_enum(oc, "format", "QCryptoSecretFormat", QCryptoSecretFormat_lookup, qcrypto_secret_prop_get_format, qcrypto_secret_prop_set_format, NULL); object_class_property_set_description(oc, "format", "Data format, one of " QCryptoSecretFormat_value_str, &error_abort); So that description ends up being "Data format, one of 'base64', 'plain'" It would be nicer if the object_property_add_* methods just accepted a 'const char *description' too. As well as being more efficient that a second method call which has to search for the ObjectProperty struct, potentially reporting errors, it will also encourage people to actually provide descriptions Regards, Daniel --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|