[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each e
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4 10/10] qapi: Populate info['name'] for each entity |
Date: |
Tue, 08 Mar 2016 17:46:00 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Every non-implicit entity is associated with an 'info'
> dictionary, but it is not easy to reverse-engineer the name of
> the top-most entity associated with that 'info'. Our use of
> 'case_whitelist' (added in commit 893e1f2) is thus currently
> tied to the owner of a member instead; but as the previous patch
> showed with CpuInfo, this requires whitelist exceptions to know
> how an implicit name will be generated.
Why is that a problem?
> While we have a ._pretty_owner() that maps from implicit names
> back to a human readable phrase, that produces more than just a
> plain top-level entity name. What's more, the current use of
> ._pretty_owner() is via .check_clash(), which can be called on
> the same member object more than once (once through the
> standalone type, and a second time when used as a base class of
> a derived tpye); if a clash is only introduced in the derived
> class, using ._pretty_owner() to report the error on behalf of
> the base class named in member.owner seems wrong. Therefore,
> we need a new mechanism.
Now I'm confused. Are you fixing suboptimal error messages?
If yes, can you show the message before & after the patch?
> Add a new info['name'] field to track the information we need,
> allowing us to change 'case_whitelist' to use only names present
> in the qapi files.
>
> Unfortunately, there is no one good place to add the mapping:
> at the point 'info' is created in QAPISchemaParser.__init__(),
> we don't know the name. Info is then stored into
> QAPISchemaParser.exprs, which then then gets fed to
> QAPISchema.__init__() via the global check_exprs(), but we want
> check_exprs() to go away. And QAPISchema._def_exprs() sees
> every entity, except that the various _def_FOO() helpers don't
> return anything. So we have to modify all of the _def_FOO()
> methods.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v4: Change series again :)
> [previously posted in subset F]:
> v6: sink later in series, and rework commit message
> [previously posted in subset D]:
> v14: rearrange assignment, improve commit message
> v13: new patch
> ---
> scripts/qapi.py | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index ebcd207..6c991c3 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -62,8 +62,8 @@ returns_whitelist = [
> # Whitelist of entities allowed to violate case conventions
> case_whitelist = [
> # From QMP:
> - ':obj-CpuInfo-base', # CPU, visible through query-cpu
> 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
> + 'CpuInfo', # CPU and PC, visible through query-cpu
> 'CpuInfoMIPS', # PC, visible through query-cpu
> 'CpuInfoTricore', # PC, visible through query-cpu
> 'QapiErrorClass', # all members, visible through errors
> @@ -1035,7 +1035,7 @@ class QAPISchemaMember(object):
>
> def check_clash(self, info, seen):
> cname = c_name(self.name)
> - if cname.lower() != cname and self.owner not in case_whitelist:
> + if cname.lower() != cname and info['name'] not in case_whitelist:
> raise QAPIExprError(info,
> "%s should not use uppercase" %
> self.describe())
> if cname in seen:
Doesn't look like you're touching the error message: QAPIExprError()
doesn't use info['name'].
> @@ -1296,7 +1296,7 @@ class QAPISchema(object):
> return name
>
> def _def_enum_type(self, expr, info):
> - name = expr['enum']
> + name = info['name'] = expr['enum']
> data = expr['data']
> prefix = expr.get('prefix')
> self._def_entity(QAPISchemaEnumType(
> @@ -1317,7 +1317,7 @@ class QAPISchema(object):
> for (key, value) in data.iteritems()]
>
> def _def_struct_type(self, expr, info):
> - name = expr['struct']
> + name = info['name'] = expr['struct']
> base = expr.get('base')
> data = expr['data']
> self._def_entity(QAPISchemaObjectType(name, info, base,
> @@ -1336,7 +1336,7 @@ class QAPISchema(object):
> return QAPISchemaObjectTypeVariant(case, typ)
>
> def _def_union_type(self, expr, info):
> - name = expr['union']
> + name = info['name'] = expr['union']
> data = expr['data']
> base = expr.get('base')
> tag_name = expr.get('discriminator')
> @@ -1362,7 +1362,7 @@ class QAPISchema(object):
> variants)))
>
> def _def_alternate_type(self, expr, info):
> - name = expr['alternate']
> + name = info['name'] = expr['alternate']
> data = expr['data']
> variants = [self._make_variant(key, value)
> for (key, value) in data.iteritems()]
> @@ -1374,7 +1374,7 @@ class QAPISchema(object):
> variants)))
>
> def _def_command(self, expr, info):
> - name = expr['command']
> + name = info['name'] = expr['command']
> data = expr.get('data')
> rets = expr.get('returns')
> gen = expr.get('gen', True)
> @@ -1389,7 +1389,7 @@ class QAPISchema(object):
> success_response))
>
> def _def_event(self, expr, info):
> - name = expr['event']
> + name = info['name'] = expr['event']
> data = expr.get('data')
> if isinstance(data, OrderedDict):
> data = self._make_implicit_object_type(
[Qemu-devel] [PATCH v4 09/10] qapi: Use anonymous bases in QMP flat unions, Eric Blake, 2016/03/05
[Qemu-devel] [PATCH v4 07/10] qapi: Don't special-case simple union wrappers, Eric Blake, 2016/03/05