From patchwork Thu Sep 22 13:01:37 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= X-Patchwork-Id: 673420 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sfynN5FtSz9t2C for ; Fri, 23 Sep 2016 00:00:40 +1000 (AEST) Received: from localhost ([::1]:46055 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bn4YI-00054J-9a for incoming@patchwork.ozlabs.org; Thu, 22 Sep 2016 10:00:38 -0400 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 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9808D65878; Thu, 22 Sep 2016 13:01:41 +0000 (UTC) Received: from redhat.com (vpn1-7-180.ams2.redhat.com [10.36.7.180]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8MD1bae019464 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 22 Sep 2016 09:01:39 -0400 Date: Thu, 22 Sep 2016 14:01:37 +0100 From: "Daniel P. Berrange" To: Markus Armbruster Message-ID: <20160922130136.GO352@redhat.com> 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-Disposition: inline In-Reply-To: <87shssnxeq.fsf@dusky.pond.sub.org> User-Agent: Mutt/1.7.0 (2016-08-17) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 22 Sep 2016 13:01:41 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 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?= X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, afaerber@suse.de, Lin Ma Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote: > "Lin Ma" writes: > > >>>> Markus Armbruster 2016/9/20 星期二 上午 1:13 >>> > >>Andreas Färber writes: > > Saving acceptable values of enumeration types into member description > > of ObjectProperty is a good idea. > > > > The member description of ObjectProperty instance of any user-creatable > > object is NULL so far, > > It's null until set with object_property_set_description(). We do that > for a few properties, and may do it more. > > > If I use object_property_set_description() to fill the > > acceptable values of enumeration type into the description in function > > 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 case, If > > member description is not NULL, it means this ObjectProperty instance is > > an enumeration. > > 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. > > > Any suggestions? > > Yes: > > >>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. > > 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. > > The existing description infrastructure is just Property member > description and object_property_set_description(). Rarely used, so > description is generally null. > > Calling object_property_set_description() more often could be helpful, > but to come up with a sensible description string, you need to know what > the property does. Needs to be left to people actually familiar with > the objects. > > Aside: historically, we add properties to *instances*. All the property > 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 memory > 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. > > 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: 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 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 += gen_enum(name, values, prefix) if do_builtins: self.defn += gen_enum_lookup(name, values, prefix) + self._btin += gen_enum_value_str(name, values) else: self._fwdecl += gen_enum(name, values, prefix) self.defn += gen_enum_lookup(name, values, prefix) + self._fwdecl += gen_enum_value_str(name, values) 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[] = { return ret +def gen_enum_value_str(name, values): + return mcgen(''' + +#define %(c_name)s_value_str "%(value_str)s" +''', + c_name=c_name(name), + value_str=", ".join(["'%s'" % c for c in values])) + + def gen_enum(name, values, prefix=None): # append automatically generated _MAX value enum_values = values + ['_MAX']