* [PULL 01/29] qapi/pragma: Tidy up after removal of deprecated commands
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 02/29] tests/qapi-schema: Drop redundant flat-union-inline test Markus Armbruster
` (28 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, John Snow
Commit cbde7be900 "migrate: remove QMP/HMP commands for speed,
downtime and cache size" neglected to remove query-migrate-cache-size
from pragma returns-whitelist.
Commit 8af54b9172 "machine: remove 'query-cpus' QMP command" neglected
to remove CpuInfo & friends from pragma name-case-exceptions.
Remove these now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-2-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
qapi/pragma.json | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/qapi/pragma.json b/qapi/pragma.json
index cffae27666..7f158e183d 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -7,18 +7,14 @@
'returns-whitelist': [
'human-monitor-command',
'qom-get',
- 'query-migrate-cache-size',
'query-tpm-models',
'query-tpm-types',
'ringbuf-read' ],
'name-case-whitelist': [
'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
- 'CpuInfoMIPS', # PC, visible through query-cpu
- 'CpuInfoTricore', # PC, visible through query-cpu
'BlockdevVmdkSubformat', # all members, to match VMDK spec spellings
'BlockdevVmdkAdapterType', # legacyESX, to match VMDK spec spellings
'QapiErrorClass', # all members, visible through errors
'UuidInfo', # UUID, visible through query-uuid
- 'X86CPURegister32', # all members, visible indirectly through qom-get
- 'CpuInfo' # CPU, visible through query-cpu
+ 'X86CPURegister32' # all members, visible indirectly through qom-get
] } }
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 02/29] tests/qapi-schema: Drop redundant flat-union-inline test
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
2021-03-23 21:56 ` [PULL 01/29] qapi/pragma: Tidy up after removal of deprecated commands Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 03/29] tests/qapi-schema: Rework comments on longhand member definitions Markus Armbruster
` (27 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, John Snow
flat-union-inline.json covers longhand branch definition with an
invalid type value. It's redundant: longhand branch definition is
covered by flat-union-inline-invalid-dict.json, and invalid type value
is covered by nested-struct-data.json. Drop the test.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-3-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
tests/qapi-schema/flat-union-inline.err | 2 --
tests/qapi-schema/flat-union-inline.json | 11 -----------
tests/qapi-schema/flat-union-inline.out | 0
tests/qapi-schema/meson.build | 1 -
4 files changed, 14 deletions(-)
delete mode 100644 tests/qapi-schema/flat-union-inline.err
delete mode 100644 tests/qapi-schema/flat-union-inline.json
delete mode 100644 tests/qapi-schema/flat-union-inline.out
diff --git a/tests/qapi-schema/flat-union-inline.err b/tests/qapi-schema/flat-union-inline.err
deleted file mode 100644
index 538283f5db..0000000000
--- a/tests/qapi-schema/flat-union-inline.err
+++ /dev/null
@@ -1,2 +0,0 @@
-flat-union-inline.json: In union 'TestUnion':
-flat-union-inline.json:7: 'data' member 'value1' should be a type name
diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json
deleted file mode 100644
index a9b3ce3f0d..0000000000
--- a/tests/qapi-schema/flat-union-inline.json
+++ /dev/null
@@ -1,11 +0,0 @@
-# we require branches to be a struct name
-# TODO: should we allow anonymous inline branch types?
-{ 'enum': 'TestEnum',
- 'data': [ 'value1', 'value2' ] }
-{ 'struct': 'Base',
- 'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
-{ 'union': 'TestUnion',
- 'base': 'Base',
- 'discriminator': 'enum1',
- 'data': { 'value1': { 'type': {} },
- 'value2': { 'integer': 'int' } } }
diff --git a/tests/qapi-schema/flat-union-inline.out b/tests/qapi-schema/flat-union-inline.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 304ef939bd..d5fa035507 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -111,7 +111,6 @@ schemas = [
'flat-union-clash-member.json',
'flat-union-discriminator-bad-name.json',
'flat-union-empty.json',
- 'flat-union-inline.json',
'flat-union-inline-invalid-dict.json',
'flat-union-int-branch.json',
'flat-union-invalid-branch-key.json',
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 03/29] tests/qapi-schema: Rework comments on longhand member definitions
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
2021-03-23 21:56 ` [PULL 01/29] qapi/pragma: Tidy up after removal of deprecated commands Markus Armbruster
2021-03-23 21:56 ` [PULL 02/29] tests/qapi-schema: Drop redundant flat-union-inline test Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 04/29] tests/qapi-schema: Belatedly update comment on alternate clash Markus Armbruster
` (26 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: John Snow, peter.maydell
A few old comments talk about "desired future use of defaults" and
"anonymous inline branch types". Kind of misleading since commit
87adbbffd4 "qapi: add a dictionary form for TYPE" added longhand
member definitions. Talk about that instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-4-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
tests/qapi-schema/event-member-invalid-dict.err | 2 +-
tests/qapi-schema/event-member-invalid-dict.json | 2 ++
tests/qapi-schema/flat-union-inline-invalid-dict.json | 4 ++--
tests/qapi-schema/nested-struct-data-invalid-dict.err | 2 +-
tests/qapi-schema/nested-struct-data-invalid-dict.json | 3 ++-
tests/qapi-schema/nested-struct-data.json | 2 +-
tests/qapi-schema/struct-member-invalid-dict.err | 2 +-
tests/qapi-schema/struct-member-invalid-dict.json | 3 ++-
8 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/tests/qapi-schema/event-member-invalid-dict.err b/tests/qapi-schema/event-member-invalid-dict.err
index c7a6a24305..82f8989344 100644
--- a/tests/qapi-schema/event-member-invalid-dict.err
+++ b/tests/qapi-schema/event-member-invalid-dict.err
@@ -1,2 +1,2 @@
event-member-invalid-dict.json: In event 'EVENT_A':
-event-member-invalid-dict.json:1: 'data' member 'a' misses key 'type'
+event-member-invalid-dict.json:3: 'data' member 'a' misses key 'type'
diff --git a/tests/qapi-schema/event-member-invalid-dict.json b/tests/qapi-schema/event-member-invalid-dict.json
index ee6f3ecb6f..e58560abca 100644
--- a/tests/qapi-schema/event-member-invalid-dict.json
+++ b/tests/qapi-schema/event-member-invalid-dict.json
@@ -1,2 +1,4 @@
+# event 'data' member with dict value is (longhand) argument
+# definition, not inline complex type
{ 'event': 'EVENT_A',
'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.json b/tests/qapi-schema/flat-union-inline-invalid-dict.json
index 62c7cda617..1779712795 100644
--- a/tests/qapi-schema/flat-union-inline-invalid-dict.json
+++ b/tests/qapi-schema/flat-union-inline-invalid-dict.json
@@ -1,5 +1,5 @@
-# we require branches to be a struct name
-# TODO: should we allow anonymous inline branch types?
+# union 'data' member with dict value is (longhand) branch
+# definition, not inline complex type
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
{ 'struct': 'Base',
diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.err b/tests/qapi-schema/nested-struct-data-invalid-dict.err
index c044b2b17a..375e155fe6 100644
--- a/tests/qapi-schema/nested-struct-data-invalid-dict.err
+++ b/tests/qapi-schema/nested-struct-data-invalid-dict.err
@@ -1,2 +1,2 @@
nested-struct-data-invalid-dict.json: In command 'foo':
-nested-struct-data-invalid-dict.json:2: 'data' member 'a' misses key 'type'
+nested-struct-data-invalid-dict.json:3: 'data' member 'a' misses key 'type'
diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.json b/tests/qapi-schema/nested-struct-data-invalid-dict.json
index efbe773ded..aa37b85e19 100644
--- a/tests/qapi-schema/nested-struct-data-invalid-dict.json
+++ b/tests/qapi-schema/nested-struct-data-invalid-dict.json
@@ -1,3 +1,4 @@
-# inline subtypes collide with our desired future use of defaults
+# command 'data' member with dict value is (longhand) argument
+# definition, not inline complex type
{ 'command': 'foo',
'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json
index 5b8a40cca3..2980d45d05 100644
--- a/tests/qapi-schema/nested-struct-data.json
+++ b/tests/qapi-schema/nested-struct-data.json
@@ -1,3 +1,3 @@
-# inline subtypes collide with our desired future use of defaults
+# {} is not a valid type reference
{ 'command': 'foo',
'data': { 'a' : { 'type': {} }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err
index 0621aecfbd..f9b3f33551 100644
--- a/tests/qapi-schema/struct-member-invalid-dict.err
+++ b/tests/qapi-schema/struct-member-invalid-dict.err
@@ -1,2 +1,2 @@
struct-member-invalid-dict.json: In struct 'foo':
-struct-member-invalid-dict.json:2: 'data' member '*a' misses key 'type'
+struct-member-invalid-dict.json:3: 'data' member '*a' misses key 'type'
diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json
index 9fe0d455a9..bc3d62ae63 100644
--- a/tests/qapi-schema/struct-member-invalid-dict.json
+++ b/tests/qapi-schema/struct-member-invalid-dict.json
@@ -1,3 +1,4 @@
-# Long form of member must have a value member 'type'
+# struct 'data' member with dict value is (longhand) member
+# definition, not inline complex type
{ 'struct': 'foo',
'data': { '*a': { 'case': 'foo' } } }
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 04/29] tests/qapi-schema: Belatedly update comment on alternate clash
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (2 preceding siblings ...)
2021-03-23 21:56 ` [PULL 03/29] tests/qapi-schema: Rework comments on longhand member definitions Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 05/29] tests/qapi-schema: Drop TODO comment on simple unions Markus Armbruster
` (25 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, John Snow
Commit 0426d53c65 "qapi: Simplify visiting of alternate types"
eliminated the implicit alternate enum, but neglected to update a
comment about it in a test. Do that now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-5-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
tests/qapi-schema/alternate-clash.err | 2 +-
tests/qapi-schema/alternate-clash.json | 5 +----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/tests/qapi-schema/alternate-clash.err b/tests/qapi-schema/alternate-clash.err
index f58b977f7b..0fe02f2c99 100644
--- a/tests/qapi-schema/alternate-clash.err
+++ b/tests/qapi-schema/alternate-clash.err
@@ -1,2 +1,2 @@
alternate-clash.json: In alternate 'Alt1':
-alternate-clash.json:7: branch 'a_b' collides with branch 'a-b'
+alternate-clash.json:4: branch 'a_b' collides with branch 'a-b'
diff --git a/tests/qapi-schema/alternate-clash.json b/tests/qapi-schema/alternate-clash.json
index 9a59b88ced..039c4be658 100644
--- a/tests/qapi-schema/alternate-clash.json
+++ b/tests/qapi-schema/alternate-clash.json
@@ -1,8 +1,5 @@
# Alternate branch name collision
# Reject an alternate that would result in a collision in generated C
-# names (this would try to generate two enum values 'ALT1_KIND_A_B').
-# TODO: In the future, if alternates are simplified to not generate
-# the implicit Alt1Kind enum, we would still have a collision with the
-# resulting C union trying to have two members named 'a_b'.
+# names (this would try to generate two union members named 'a_b').
{ 'alternate': 'Alt1',
'data': { 'a-b': 'bool', 'a_b': 'int' } }
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 05/29] tests/qapi-schema: Drop TODO comment on simple unions
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (3 preceding siblings ...)
2021-03-23 21:56 ` [PULL 04/29] tests/qapi-schema: Belatedly update comment on alternate clash Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 06/29] tests/qapi-schema: Tweak to demonstrate buggy member name check Markus Armbruster
` (24 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, John Snow
Simple unions don't need more features, they need to die.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-6-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
tests/qapi-schema/flat-union-no-base.err | 2 +-
tests/qapi-schema/flat-union-no-base.json | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err
index 9bd595bcfb..5167565b00 100644
--- a/tests/qapi-schema/flat-union-no-base.err
+++ b/tests/qapi-schema/flat-union-no-base.err
@@ -1,2 +1,2 @@
flat-union-no-base.json: In union 'TestUnion':
-flat-union-no-base.json:9: 'discriminator' requires 'base'
+flat-union-no-base.json:8: 'discriminator' requires 'base'
diff --git a/tests/qapi-schema/flat-union-no-base.json b/tests/qapi-schema/flat-union-no-base.json
index ffc4c6f0e6..327877b563 100644
--- a/tests/qapi-schema/flat-union-no-base.json
+++ b/tests/qapi-schema/flat-union-no-base.json
@@ -1,5 +1,4 @@
# flat unions require a base
-# TODO: simple unions should be able to use an enum discriminator
{ 'struct': 'TestTypeA',
'data': { 'string': 'str' } }
{ 'struct': 'TestTypeB',
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 06/29] tests/qapi-schema: Tweak to demonstrate buggy member name check
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (4 preceding siblings ...)
2021-03-23 21:56 ` [PULL 05/29] tests/qapi-schema: Drop TODO comment on simple unions Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 07/29] qapi: Fix to reject optional members with reserved names Markus Armbruster
` (23 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, John Snow
Member name 'u' and names starting with 'has-' or 'has_' are reserved
for the generator. check_type() enforces this, covered by tests
reserved-member-u and reserved-member-has.
These tests neglect to cover optional members, where the name starts
with '*'. Tweak reserved-member-u to fix that. Test
reserved-member-has still covers non-optional members.
This demonstrates the reserved member name check is broken for
optional members. The next commit will fix it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-7-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
[Commit message improved slightly]
---
tests/qapi-schema/reserved-member-u.err | 2 --
tests/qapi-schema/reserved-member-u.json | 3 ++-
tests/qapi-schema/reserved-member-u.out | 14 ++++++++++++++
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/tests/qapi-schema/reserved-member-u.err b/tests/qapi-schema/reserved-member-u.err
index 231d552494..e69de29bb2 100644
--- a/tests/qapi-schema/reserved-member-u.err
+++ b/tests/qapi-schema/reserved-member-u.err
@@ -1,2 +0,0 @@
-reserved-member-u.json: In struct 'Oops':
-reserved-member-u.json:7: 'data' member 'u' uses reserved name
diff --git a/tests/qapi-schema/reserved-member-u.json b/tests/qapi-schema/reserved-member-u.json
index 1eaf0f301c..15005abb09 100644
--- a/tests/qapi-schema/reserved-member-u.json
+++ b/tests/qapi-schema/reserved-member-u.json
@@ -4,4 +4,5 @@
# This is true even for non-unions, because it is possible to convert a
# struct to flat union while remaining backwards compatible in QMP.
# TODO - we could munge the member name to 'q_u' to avoid the collision
-{ 'struct': 'Oops', 'data': { 'u': 'str' } }
+# BUG: not rejected
+{ 'struct': 'Oops', 'data': { '*u': 'str' } }
diff --git a/tests/qapi-schema/reserved-member-u.out b/tests/qapi-schema/reserved-member-u.out
index e69de29bb2..6a3705518b 100644
--- a/tests/qapi-schema/reserved-member-u.out
+++ b/tests/qapi-schema/reserved-member-u.out
@@ -0,0 +1,14 @@
+module ./builtin
+object q_empty
+enum QType
+ prefix QTYPE
+ member none
+ member qnull
+ member qnum
+ member qstring
+ member qdict
+ member qlist
+ member qbool
+module reserved-member-u.json
+object Oops
+ member u: str optional=True
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 07/29] qapi: Fix to reject optional members with reserved names
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (5 preceding siblings ...)
2021-03-23 21:56 ` [PULL 06/29] tests/qapi-schema: Tweak to demonstrate buggy member name check Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 08/29] qapi: Permit flat union members for any tag value Markus Armbruster
` (22 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, John Snow
check_type() fails to reject optional members with reserved names,
because it neglects to strip off the leading '*'. Fix that.
The stripping in check_name_str() is now useless. Drop.
Also drop the "no leading '*'" assertion, because valid_name.match()
ensures it can't fail.
Fixes: 9fb081e0b98409556d023c7193eeb68947cd1211
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-8-armbru@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/expr.py | 9 ++++-----
tests/qapi-schema/reserved-member-u.err | 2 ++
tests/qapi-schema/reserved-member-u.json | 1 -
tests/qapi-schema/reserved-member-u.out | 14 --------------
4 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 2fcaaa2497..cf09fa9fd3 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -34,12 +34,10 @@ def check_name_is_str(name, info, source):
def check_name_str(name, info, source,
- allow_optional=False, enum_member=False,
+ enum_member=False,
permit_upper=False):
membername = name
- if allow_optional and name.startswith('*'):
- membername = name[1:]
# Enum members can start with a digit, because the generated C
# code always prefixes it with the enum name
if enum_member and membername[0].isdigit():
@@ -52,7 +50,6 @@ def check_name_str(name, info, source,
if not permit_upper and name.lower() != name:
raise QAPISemError(
info, "%s uses uppercase in name" % source)
- assert not membername.startswith('*')
def check_defn_name_str(name, info, meta):
@@ -171,8 +168,10 @@ def check_type(value, info, source,
# value is a dictionary, check that each member is okay
for (key, arg) in value.items():
key_source = "%s member '%s'" % (source, key)
+ if key.startswith('*'):
+ key = key[1:]
check_name_str(key, info, key_source,
- allow_optional=True, permit_upper=permit_upper)
+ permit_upper=permit_upper)
if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
raise QAPISemError(info, "%s uses reserved name" % key_source)
check_keys(arg, info, key_source, ['type'], ['if', 'features'])
diff --git a/tests/qapi-schema/reserved-member-u.err b/tests/qapi-schema/reserved-member-u.err
index e69de29bb2..b58e599a00 100644
--- a/tests/qapi-schema/reserved-member-u.err
+++ b/tests/qapi-schema/reserved-member-u.err
@@ -0,0 +1,2 @@
+reserved-member-u.json: In struct 'Oops':
+reserved-member-u.json:7: 'data' member '*u' uses reserved name
diff --git a/tests/qapi-schema/reserved-member-u.json b/tests/qapi-schema/reserved-member-u.json
index 15005abb09..2bfb8f59b6 100644
--- a/tests/qapi-schema/reserved-member-u.json
+++ b/tests/qapi-schema/reserved-member-u.json
@@ -4,5 +4,4 @@
# This is true even for non-unions, because it is possible to convert a
# struct to flat union while remaining backwards compatible in QMP.
# TODO - we could munge the member name to 'q_u' to avoid the collision
-# BUG: not rejected
{ 'struct': 'Oops', 'data': { '*u': 'str' } }
diff --git a/tests/qapi-schema/reserved-member-u.out b/tests/qapi-schema/reserved-member-u.out
index 6a3705518b..e69de29bb2 100644
--- a/tests/qapi-schema/reserved-member-u.out
+++ b/tests/qapi-schema/reserved-member-u.out
@@ -1,14 +0,0 @@
-module ./builtin
-object q_empty
-enum QType
- prefix QTYPE
- member none
- member qnull
- member qnum
- member qstring
- member qdict
- member qlist
- member qbool
-module reserved-member-u.json
-object Oops
- member u: str optional=True
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 08/29] qapi: Permit flat union members for any tag value
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (6 preceding siblings ...)
2021-03-23 21:56 ` [PULL 07/29] qapi: Fix to reject optional members with reserved names Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 09/29] qapi: Lift enum-specific code out of check_name_str() Markus Armbruster
` (21 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Flat union branch names match the tag enum's member names. Omitted
branches default to "no members for this tag value".
Branch names starting with a digit get rejected like "'data' member
'0' has an invalid name". However, omitting the branch works.
This is because flat union tag values get checked twice: as enum
member name, and as union branch name. The former accepts leading
digits, the latter doesn't.
Branches whose names start with a digit therefore cannot have members.
Feels wrong. Get rid of the restriction by skipping the latter check.
This can expose c_name() to input it can't handle: a name starting
with a digit. Improve it to return a valid C identifier for any
input.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-9-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message rewritten]
---
scripts/qapi/common.py | 8 ++++----
scripts/qapi/expr.py | 4 +++-
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 11b86beeab..cbd3fd81d3 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -18,7 +18,6 @@
#: Magic string that gets removed along with all space to its right.
EATSPACE = '\033EATSPACE.'
POINTER_SUFFIX = ' *' + EATSPACE
-_C_NAME_TRANS = str.maketrans('.-', '__')
def camel_to_upper(value: str) -> str:
@@ -109,9 +108,10 @@ def c_name(name: str, protect: bool = True) -> str:
'not_eq', 'or', 'or_eq', 'xor', 'xor_eq'])
# namespace pollution:
polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386'])
- name = name.translate(_C_NAME_TRANS)
- if protect and (name in c89_words | c99_words | c11_words | gcc_words
- | cpp_words | polluted_words):
+ name = re.sub(r'[^A-Za-z0-9_]', '_', name)
+ if protect and (name in (c89_words | c99_words | c11_words | gcc_words
+ | cpp_words | polluted_words)
+ or name[0].isdigit()):
return 'q_' + name
return name
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index cf09fa9fd3..507550c340 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -246,7 +246,9 @@ def check_union(expr, info):
for (key, value) in members.items():
source = "'data' member '%s'" % key
- check_name_str(key, info, source)
+ if discriminator is None:
+ check_name_str(key, info, source)
+ # else: name is in discriminator enum, which gets checked
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
check_type(value['type'], info, source, allow_array=not base)
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 09/29] qapi: Lift enum-specific code out of check_name_str()
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (7 preceding siblings ...)
2021-03-23 21:56 ` [PULL 08/29] qapi: Permit flat union members for any tag value Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 10/29] qapi: Rework name checking in preparation of stricter checking Markus Armbruster
` (20 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
check_name_str() masks leading digits when passed enum_member=True.
Only check_enum() does. Lift the masking into check_enum().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-10-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/expr.py | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 507550c340..e00467636c 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -34,18 +34,11 @@ def check_name_is_str(name, info, source):
def check_name_str(name, info, source,
- enum_member=False,
permit_upper=False):
- membername = name
-
- # Enum members can start with a digit, because the generated C
- # code always prefixes it with the enum name
- if enum_member and membername[0].isdigit():
- membername = 'D' + membername
# Reserve the entire 'q_' namespace for c_name(), and for 'q_empty'
# and 'q_obj_*' implicit type names.
- if not valid_name.match(membername) or \
- c_name(membername, False).startswith('q_'):
+ if not valid_name.match(name) or \
+ c_name(name, False).startswith('q_'):
raise QAPISemError(info, "%s has an invalid name" % source)
if not permit_upper and name.lower() != name:
raise QAPISemError(
@@ -213,11 +206,15 @@ def check_enum(expr, info):
for m in members]
for member in members:
source = "'data' member"
+ member_name = member['name']
check_keys(member, info, source, ['name'], ['if'])
- check_name_is_str(member['name'], info, source)
- source = "%s '%s'" % (source, member['name'])
- check_name_str(member['name'], info, source,
- enum_member=True, permit_upper=permit_upper)
+ check_name_is_str(member_name, info, source)
+ source = "%s '%s'" % (source, member_name)
+ # Enum members may start with a digit
+ if member_name[0].isdigit():
+ member_name = 'd' + member_name # Hack: hide the digit
+ check_name_str(member_name, info, source,
+ permit_upper=permit_upper)
check_if(member, info, source)
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 10/29] qapi: Rework name checking in preparation of stricter checking
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (8 preceding siblings ...)
2021-03-23 21:56 ` [PULL 09/29] qapi: Lift enum-specific code out of check_name_str() Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 11/29] qapi: Move uppercase rejection to check_name_lower() Markus Armbruster
` (19 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Naming rules differ for the various kinds of names. To prepare
enforcing them, define functions to check them: check_name_upper(),
check_name_lower(), and check_name_camel(). For now, these merely
wrap around check_name_str(), but that will change shortly. Replace
the other uses of check_name_str() by appropriate uses of the
wrappers. No change in behavior just yet.
check_name_str() now returns the name without downstream and x-
prefix, for use by the wrappers in later patches. Requires tweaking
regexp @valid_name. It accepts the same strings as before.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-11-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message improved]
---
scripts/qapi/expr.py | 51 +++++++++++++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 15 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index e00467636c..30285fe334 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -21,11 +21,12 @@
from .error import QAPISemError
-# Names must be letters, numbers, -, and _. They must start with letter,
-# except for downstream extensions which must start with __RFQDN_.
-# Dots are only valid in the downstream extension prefix.
-valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?'
- '[a-zA-Z][a-zA-Z0-9_-]*$')
+# Names consist of letters, digits, -, and _, starting with a letter.
+# An experimental name is prefixed with x-. A name of a downstream
+# extension is prefixed with __RFQDN_. The latter prefix goes first.
+valid_name = re.compile(r'(__[a-z0-9.-]+_)?'
+ r'(x-)?'
+ r'([a-z][a-z0-9_-]*)$', re.IGNORECASE)
def check_name_is_str(name, info, source):
@@ -37,16 +38,38 @@ def check_name_str(name, info, source,
permit_upper=False):
# Reserve the entire 'q_' namespace for c_name(), and for 'q_empty'
# and 'q_obj_*' implicit type names.
- if not valid_name.match(name) or \
- c_name(name, False).startswith('q_'):
+ match = valid_name.match(name)
+ if not match or c_name(name, False).startswith('q_'):
raise QAPISemError(info, "%s has an invalid name" % source)
if not permit_upper and name.lower() != name:
raise QAPISemError(
info, "%s uses uppercase in name" % source)
+ return match.group(3)
+
+
+def check_name_upper(name, info, source):
+ stem = check_name_str(name, info, source, permit_upper=True)
+ # TODO reject '[a-z-]' in @stem
+
+
+def check_name_lower(name, info, source,
+ permit_upper=False):
+ stem = check_name_str(name, info, source, permit_upper)
+ # TODO reject '_' in stem
+
+
+def check_name_camel(name, info, source):
+ stem = check_name_str(name, info, source, permit_upper=True)
+ # TODO reject '[_-]' in stem, require CamelCase
def check_defn_name_str(name, info, meta):
- check_name_str(name, info, meta, permit_upper=True)
+ if meta == 'event':
+ check_name_upper(name, info, meta)
+ elif meta == 'command':
+ check_name_lower(name, info, meta, permit_upper=True)
+ else:
+ check_name_camel(name, info, meta)
if name.endswith('Kind') or name.endswith('List'):
raise QAPISemError(
info, "%s name should not end in '%s'" % (meta, name[-4:]))
@@ -163,8 +186,7 @@ def check_type(value, info, source,
key_source = "%s member '%s'" % (source, key)
if key.startswith('*'):
key = key[1:]
- check_name_str(key, info, key_source,
- permit_upper=permit_upper)
+ check_name_lower(key, info, key_source, permit_upper)
if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
raise QAPISemError(info, "%s uses reserved name" % key_source)
check_keys(arg, info, key_source, ['type'], ['if', 'features'])
@@ -186,7 +208,7 @@ def check_features(features, info):
check_keys(f, info, source, ['name'], ['if'])
check_name_is_str(f['name'], info, source)
source = "%s '%s'" % (source, f['name'])
- check_name_str(f['name'], info, source)
+ check_name_lower(f['name'], info, source)
check_if(f, info, source)
@@ -213,8 +235,7 @@ def check_enum(expr, info):
# Enum members may start with a digit
if member_name[0].isdigit():
member_name = 'd' + member_name # Hack: hide the digit
- check_name_str(member_name, info, source,
- permit_upper=permit_upper)
+ check_name_lower(member_name, info, source, permit_upper)
check_if(member, info, source)
@@ -244,7 +265,7 @@ def check_union(expr, info):
for (key, value) in members.items():
source = "'data' member '%s'" % key
if discriminator is None:
- check_name_str(key, info, source)
+ check_name_lower(key, info, source)
# else: name is in discriminator enum, which gets checked
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
@@ -258,7 +279,7 @@ def check_alternate(expr, info):
raise QAPISemError(info, "'data' must not be empty")
for (key, value) in members.items():
source = "'data' member '%s'" % key
- check_name_str(key, info, source)
+ check_name_lower(key, info, source)
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
check_type(value['type'], info, source)
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 11/29] qapi: Move uppercase rejection to check_name_lower()
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (9 preceding siblings ...)
2021-03-23 21:56 ` [PULL 10/29] qapi: Rework name checking in preparation of stricter checking Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 12/29] qapi: Consistently permit any case in downstream prefixes Markus Armbruster
` (18 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
check_name_lower() is the only user of check_name_str() using
permit_upper=False. Move the associated code from check_name_str() to
check_name_lower(), and drop the parameter.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-12-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/expr.py | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 30285fe334..a815060ee2 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -34,32 +34,31 @@ def check_name_is_str(name, info, source):
raise QAPISemError(info, "%s requires a string name" % source)
-def check_name_str(name, info, source,
- permit_upper=False):
+def check_name_str(name, info, source):
# Reserve the entire 'q_' namespace for c_name(), and for 'q_empty'
# and 'q_obj_*' implicit type names.
match = valid_name.match(name)
if not match or c_name(name, False).startswith('q_'):
raise QAPISemError(info, "%s has an invalid name" % source)
- if not permit_upper and name.lower() != name:
- raise QAPISemError(
- info, "%s uses uppercase in name" % source)
return match.group(3)
def check_name_upper(name, info, source):
- stem = check_name_str(name, info, source, permit_upper=True)
+ stem = check_name_str(name, info, source)
# TODO reject '[a-z-]' in @stem
def check_name_lower(name, info, source,
permit_upper=False):
- stem = check_name_str(name, info, source, permit_upper)
+ stem = check_name_str(name, info, source)
+ if not permit_upper and name.lower() != name:
+ raise QAPISemError(
+ info, "%s uses uppercase in name" % source)
# TODO reject '_' in stem
def check_name_camel(name, info, source):
- stem = check_name_str(name, info, source, permit_upper=True)
+ stem = check_name_str(name, info, source)
# TODO reject '[_-]' in stem, require CamelCase
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 12/29] qapi: Consistently permit any case in downstream prefixes
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (10 preceding siblings ...)
2021-03-23 21:56 ` [PULL 11/29] qapi: Move uppercase rejection to check_name_lower() Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 13/29] qapi: Enforce event naming rules Markus Armbruster
` (17 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
We require lowercase __RFQDN_ downstream prefixes only where we
require the prefixed name to be lowercase. Don't; permit any case in
__RFQDN_ prefixes anywhere.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-13-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/expr.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index a815060ee2..b5fb0be48b 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -51,7 +51,7 @@ def check_name_upper(name, info, source):
def check_name_lower(name, info, source,
permit_upper=False):
stem = check_name_str(name, info, source)
- if not permit_upper and name.lower() != name:
+ if not permit_upper and re.search(r'[A-Z]', stem):
raise QAPISemError(
info, "%s uses uppercase in name" % source)
# TODO reject '_' in stem
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 13/29] qapi: Enforce event naming rules
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (11 preceding siblings ...)
2021-03-23 21:56 ` [PULL 12/29] qapi: Consistently permit any case in downstream prefixes Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 14/29] qapi: Enforce type " Markus Armbruster
` (16 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Event names should be ALL_CAPS with words separated by underscore.
Enforce this. The only offenders are in tests/. Fix them. Existing
test event-case covers the new error.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-14-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-qmp-event.c | 6 +++---
scripts/qapi/expr.py | 4 +++-
tests/qapi-schema/doc-good.json | 4 ++--
tests/qapi-schema/doc-good.out | 4 ++--
tests/qapi-schema/doc-good.txt | 2 +-
tests/qapi-schema/doc-invalid-return.json | 4 ++--
tests/qapi-schema/event-case.err | 2 ++
tests/qapi-schema/event-case.json | 2 --
tests/qapi-schema/event-case.out | 14 --------------
tests/qapi-schema/qapi-schema-test.json | 6 +++---
tests/qapi-schema/qapi-schema-test.out | 8 ++++----
11 files changed, 22 insertions(+), 34 deletions(-)
diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c
index 047f44ff9a..d58c3b78f2 100644
--- a/tests/unit/test-qmp-event.c
+++ b/tests/unit/test-qmp-event.c
@@ -143,7 +143,7 @@ static void test_event_d(TestEventData *data,
static void test_event_deprecated(TestEventData *data, const void *unused)
{
- data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }");
+ data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }");
memset(&compat_policy, 0, sizeof(compat_policy));
@@ -163,7 +163,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused)
{
memset(&compat_policy, 0, sizeof(compat_policy));
- data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0',"
+ data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0',"
" 'data': { 'foo': 42 } }");
qapi_event_send_test_event_features0(42);
g_assert(data->emitted);
@@ -172,7 +172,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused)
compat_policy.has_deprecated_output = true;
compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
- data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }");
+ data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }");
qapi_event_send_test_event_features0(42);
g_assert(data->emitted);
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index b5fb0be48b..c065505b27 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -45,7 +45,9 @@ def check_name_str(name, info, source):
def check_name_upper(name, info, source):
stem = check_name_str(name, info, source)
- # TODO reject '[a-z-]' in @stem
+ if re.search(r'[a-z-]', stem):
+ raise QAPISemError(
+ info, "name of %s must not use lowercase or '-'" % source)
def check_name_lower(name, info, source,
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index e9af0857db..423ea23e07 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -179,10 +179,10 @@
'features': [ 'cmd-feat1', 'cmd-feat2' ] }
##
-# @EVT-BOXED:
+# @EVT_BOXED:
# Features:
# @feat3: a feature
##
-{ 'event': 'EVT-BOXED', 'boxed': true,
+{ 'event': 'EVT_BOXED', 'boxed': true,
'features': [ 'feat3' ],
'data': 'Object' }
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 715b0bbc1a..8f54ceff2e 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -63,7 +63,7 @@ command cmd-boxed Object -> None
gen=True success_response=True boxed=True oob=False preconfig=False
feature cmd-feat1
feature cmd-feat2
-event EVT-BOXED Object
+event EVT_BOXED Object
boxed=True
feature feat3
doc freeform
@@ -211,7 +211,7 @@ another feature
-> in
<- out
-doc symbol=EVT-BOXED
+doc symbol=EVT_BOXED
body=
feature=feat3
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index 6ca03d49d0..726727af74 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -272,7 +272,7 @@ Example
<- out
-"EVT-BOXED" (Event)
+"EVT_BOXED" (Event)
-------------------
diff --git a/tests/qapi-schema/doc-invalid-return.json b/tests/qapi-schema/doc-invalid-return.json
index 1ba45de414..95e7583930 100644
--- a/tests/qapi-schema/doc-invalid-return.json
+++ b/tests/qapi-schema/doc-invalid-return.json
@@ -1,7 +1,7 @@
# Events can't have 'Returns' section
##
-# @foo:
+# @FOO:
# Returns: blah
##
-{ 'event': 'foo' }
+{ 'event': 'FOO' }
diff --git a/tests/qapi-schema/event-case.err b/tests/qapi-schema/event-case.err
index e69de29bb2..d3007cfa63 100644
--- a/tests/qapi-schema/event-case.err
+++ b/tests/qapi-schema/event-case.err
@@ -0,0 +1,2 @@
+event-case.json: In event 'oops':
+event-case.json:1: name of event must not use lowercase or '-'
diff --git a/tests/qapi-schema/event-case.json b/tests/qapi-schema/event-case.json
index 3a92d8b610..4d8a5d8a71 100644
--- a/tests/qapi-schema/event-case.json
+++ b/tests/qapi-schema/event-case.json
@@ -1,3 +1 @@
-# TODO: might be nice to enforce naming conventions; but until then this works
-# even though events should usually be ALL_CAPS
{ 'event': 'oops' }
diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out
index 9ae44052ac..e69de29bb2 100644
--- a/tests/qapi-schema/event-case.out
+++ b/tests/qapi-schema/event-case.out
@@ -1,14 +0,0 @@
-module ./builtin
-object q_empty
-enum QType
- prefix QTYPE
- member none
- member qnull
- member qnum
- member qstring
- member qdict
- member qlist
- member qbool
-module event-case.json
-event oops None
- boxed=False
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 12ec588b52..a355321258 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -249,7 +249,7 @@
{ 'command': 'TestCmdReturnDefThree', 'returns': 'UserDefThree' }
-{ 'event': 'TestIfEvent', 'data':
+{ 'event': 'TEST_IF_EVENT', 'data':
{ 'foo': 'TestIfStruct',
'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } },
'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' }
@@ -324,8 +324,8 @@
'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)',
'defined(TEST_IF_COND_2)'] } ] }
-{ 'event': 'TEST-EVENT-FEATURES0',
+{ 'event': 'TEST_EVENT_FEATURES0',
'data': 'FeatureStruct1' }
-{ 'event': 'TEST-EVENT-FEATURES1',
+{ 'event': 'TEST_EVENT_FEATURES1',
'features': [ 'deprecated' ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index f5741df97f..882d0e7c56 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -349,12 +349,12 @@ command TestCmdReturnDefThree None -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
array TestIfEnumList TestIfEnum
if ['defined(TEST_IF_ENUM)']
-object q_obj_TestIfEvent-arg
+object q_obj_TEST_IF_EVENT-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnumList optional=False
if ['defined(TEST_IF_EVT_BAR)']
if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
-event TestIfEvent q_obj_TestIfEvent-arg
+event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
boxed=False
if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)']
object FeatureStruct0
@@ -440,9 +440,9 @@ command test-command-cond-features3 None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
feature feature1
if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)']
-event TEST-EVENT-FEATURES0 FeatureStruct1
+event TEST_EVENT_FEATURES0 FeatureStruct1
boxed=False
-event TEST-EVENT-FEATURES1 None
+event TEST_EVENT_FEATURES1 None
boxed=False
feature deprecated
module include/sub-module.json
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 14/29] qapi: Enforce type naming rules
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (12 preceding siblings ...)
2021-03-23 21:56 ` [PULL 13/29] qapi: Enforce event naming rules Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 15/29] tests/qapi-schema: Rename redefined-builtin to redefined-predefined Markus Armbruster
` (15 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Type names should be CamelCase. Enforce this. The only offenders are
in tests/. Fix them. Add test type-case to cover the new error.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-15-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Regexp simplified, new test made more robust]
---
scripts/qapi/expr.py | 3 ++-
tests/qapi-schema/doc-bad-union-member.json | 4 ++--
tests/qapi-schema/double-type.err | 2 +-
tests/qapi-schema/double-type.json | 2 +-
tests/qapi-schema/features-deprecated-type.err | 2 +-
tests/qapi-schema/features-deprecated-type.json | 2 +-
tests/qapi-schema/meson.build | 1 +
tests/qapi-schema/redefined-builtin.err | 4 ++--
tests/qapi-schema/redefined-builtin.json | 4 ++--
tests/qapi-schema/redefined-type.err | 6 +++---
tests/qapi-schema/redefined-type.json | 4 ++--
tests/qapi-schema/struct-data-invalid.err | 2 +-
tests/qapi-schema/struct-data-invalid.json | 2 +-
tests/qapi-schema/struct-member-invalid-dict.err | 2 +-
tests/qapi-schema/struct-member-invalid-dict.json | 2 +-
tests/qapi-schema/struct-member-invalid.err | 2 +-
tests/qapi-schema/struct-member-invalid.json | 2 +-
tests/qapi-schema/type-case.err | 2 ++
tests/qapi-schema/type-case.json | 2 ++
tests/qapi-schema/type-case.out | 0
tests/qapi-schema/unknown-expr-key.err | 2 +-
tests/qapi-schema/unknown-expr-key.json | 2 +-
22 files changed, 30 insertions(+), 24 deletions(-)
create mode 100644 tests/qapi-schema/type-case.err
create mode 100644 tests/qapi-schema/type-case.json
create mode 100644 tests/qapi-schema/type-case.out
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index c065505b27..7bd15559de 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -61,7 +61,8 @@ def check_name_lower(name, info, source,
def check_name_camel(name, info, source):
stem = check_name_str(name, info, source)
- # TODO reject '[_-]' in stem, require CamelCase
+ if not re.match(r'[A-Z][A-Za-z0-9]*[a-z][A-Za-z0-9]*$', stem):
+ raise QAPISemError(info, "name of %s must use CamelCase" % source)
def check_defn_name_str(name, info, meta):
diff --git a/tests/qapi-schema/doc-bad-union-member.json b/tests/qapi-schema/doc-bad-union-member.json
index d611435f6a..bd231a0109 100644
--- a/tests/qapi-schema/doc-bad-union-member.json
+++ b/tests/qapi-schema/doc-bad-union-member.json
@@ -11,9 +11,9 @@
'data': { 'nothing': 'Empty' } }
{ 'struct': 'Base',
- 'data': { 'type': 'T' } }
+ 'data': { 'type': 'FrobType' } }
{ 'struct': 'Empty',
'data': { } }
-{ 'enum': 'T', 'data': ['nothing'] }
+{ 'enum': 'FrobType', 'data': ['nothing'] }
diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err
index 71fc4dbb52..576e716197 100644
--- a/tests/qapi-schema/double-type.err
+++ b/tests/qapi-schema/double-type.err
@@ -1,3 +1,3 @@
-double-type.json: In struct 'bar':
+double-type.json: In struct 'Bar':
double-type.json:2: struct has unknown key 'command'
Valid keys are 'base', 'data', 'features', 'if', 'struct'.
diff --git a/tests/qapi-schema/double-type.json b/tests/qapi-schema/double-type.json
index 911fa7af50..2c0809f38d 100644
--- a/tests/qapi-schema/double-type.json
+++ b/tests/qapi-schema/double-type.json
@@ -1,2 +1,2 @@
# we reject an expression with ambiguous metatype
-{ 'command': 'foo', 'struct': 'bar', 'data': { } }
+{ 'command': 'foo', 'struct': 'Bar', 'data': { } }
diff --git a/tests/qapi-schema/features-deprecated-type.err b/tests/qapi-schema/features-deprecated-type.err
index af4ffe20aa..ddaedf604e 100644
--- a/tests/qapi-schema/features-deprecated-type.err
+++ b/tests/qapi-schema/features-deprecated-type.err
@@ -1,2 +1,2 @@
-features-deprecated-type.json: In struct 'S':
+features-deprecated-type.json: In struct 'Foo':
features-deprecated-type.json:2: feature 'deprecated' is not supported for types
diff --git a/tests/qapi-schema/features-deprecated-type.json b/tests/qapi-schema/features-deprecated-type.json
index 4b5bf5b86e..265849b1f7 100644
--- a/tests/qapi-schema/features-deprecated-type.json
+++ b/tests/qapi-schema/features-deprecated-type.json
@@ -1,3 +1,3 @@
# Feature 'deprecated' is not supported for types
-{ 'struct': 'S', 'data': {},
+{ 'struct': 'Foo', 'data': {},
'features': [ 'deprecated' ] }
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index d5fa035507..ba11cb76ac 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -180,6 +180,7 @@ schemas = [
'trailing-comma-list.json',
'trailing-comma-object.json',
'type-bypass-bad-gen.json',
+ 'type-case.json',
'unclosed-list.json',
'unclosed-object.json',
'unclosed-string.json',
diff --git a/tests/qapi-schema/redefined-builtin.err b/tests/qapi-schema/redefined-builtin.err
index 58c7e42ffc..92bc62dc76 100644
--- a/tests/qapi-schema/redefined-builtin.err
+++ b/tests/qapi-schema/redefined-builtin.err
@@ -1,2 +1,2 @@
-redefined-builtin.json: In struct 'size':
-redefined-builtin.json:2: built-in type 'size' is already defined
+redefined-builtin.json: In struct 'QType':
+redefined-builtin.json:2: enum type 'QType' is already defined
diff --git a/tests/qapi-schema/redefined-builtin.json b/tests/qapi-schema/redefined-builtin.json
index 45b8a550ad..cad555cc73 100644
--- a/tests/qapi-schema/redefined-builtin.json
+++ b/tests/qapi-schema/redefined-builtin.json
@@ -1,2 +1,2 @@
-# we reject types that duplicate builtin names
-{ 'struct': 'size', 'data': { 'myint': 'size' } }
+# we reject types that clash with predefined types
+{ 'struct': 'QType', 'data': { 'myint': 'size' } }
diff --git a/tests/qapi-schema/redefined-type.err b/tests/qapi-schema/redefined-type.err
index b7103fc15f..5e5406f811 100644
--- a/tests/qapi-schema/redefined-type.err
+++ b/tests/qapi-schema/redefined-type.err
@@ -1,4 +1,4 @@
-redefined-type.json: In enum 'foo':
-redefined-type.json:3: 'foo' is already defined
-redefined-type.json: In struct 'foo':
+redefined-type.json: In enum 'Foo':
+redefined-type.json:3: 'Foo' is already defined
+redefined-type.json: In struct 'Foo':
redefined-type.json:2: previous definition
diff --git a/tests/qapi-schema/redefined-type.json b/tests/qapi-schema/redefined-type.json
index a09e768bae..291453e70d 100644
--- a/tests/qapi-schema/redefined-type.json
+++ b/tests/qapi-schema/redefined-type.json
@@ -1,3 +1,3 @@
# we reject types defined more than once
-{ 'struct': 'foo', 'data': { 'one': 'str' } }
-{ 'enum': 'foo', 'data': [ 'two' ] }
+{ 'struct': 'Foo', 'data': { 'one': 'str' } }
+{ 'enum': 'Foo', 'data': [ 'two' ] }
diff --git a/tests/qapi-schema/struct-data-invalid.err b/tests/qapi-schema/struct-data-invalid.err
index 5ed4bec573..23cbfc60ea 100644
--- a/tests/qapi-schema/struct-data-invalid.err
+++ b/tests/qapi-schema/struct-data-invalid.err
@@ -1,2 +1,2 @@
-struct-data-invalid.json: In struct 'foo':
+struct-data-invalid.json: In struct 'Foo':
struct-data-invalid.json:1: 'data' should be an object or type name
diff --git a/tests/qapi-schema/struct-data-invalid.json b/tests/qapi-schema/struct-data-invalid.json
index 9adbc3bb6b..00ad11ef94 100644
--- a/tests/qapi-schema/struct-data-invalid.json
+++ b/tests/qapi-schema/struct-data-invalid.json
@@ -1,2 +1,2 @@
-{ 'struct': 'foo',
+{ 'struct': 'Foo',
'data': false }
diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err
index f9b3f33551..517793cc9b 100644
--- a/tests/qapi-schema/struct-member-invalid-dict.err
+++ b/tests/qapi-schema/struct-member-invalid-dict.err
@@ -1,2 +1,2 @@
-struct-member-invalid-dict.json: In struct 'foo':
+struct-member-invalid-dict.json: In struct 'Foo':
struct-member-invalid-dict.json:3: 'data' member '*a' misses key 'type'
diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json
index bc3d62ae63..df5d018f65 100644
--- a/tests/qapi-schema/struct-member-invalid-dict.json
+++ b/tests/qapi-schema/struct-member-invalid-dict.json
@@ -1,4 +1,4 @@
# struct 'data' member with dict value is (longhand) member
# definition, not inline complex type
-{ 'struct': 'foo',
+{ 'struct': 'Foo',
'data': { '*a': { 'case': 'foo' } } }
diff --git a/tests/qapi-schema/struct-member-invalid.err b/tests/qapi-schema/struct-member-invalid.err
index 9a2c934538..7e01a41d7c 100644
--- a/tests/qapi-schema/struct-member-invalid.err
+++ b/tests/qapi-schema/struct-member-invalid.err
@@ -1,2 +1,2 @@
-struct-member-invalid.json: In struct 'foo':
+struct-member-invalid.json: In struct 'Foo':
struct-member-invalid.json:1: 'data' member 'a' should be a type name
diff --git a/tests/qapi-schema/struct-member-invalid.json b/tests/qapi-schema/struct-member-invalid.json
index 8f172f7a87..a4cd860c67 100644
--- a/tests/qapi-schema/struct-member-invalid.json
+++ b/tests/qapi-schema/struct-member-invalid.json
@@ -1,2 +1,2 @@
-{ 'struct': 'foo',
+{ 'struct': 'Foo',
'data': { 'a': false } }
diff --git a/tests/qapi-schema/type-case.err b/tests/qapi-schema/type-case.err
new file mode 100644
index 0000000000..36d2de2d00
--- /dev/null
+++ b/tests/qapi-schema/type-case.err
@@ -0,0 +1,2 @@
+type-case.json: In struct 'not-a-camel':
+type-case.json:2: name of struct must use CamelCase
diff --git a/tests/qapi-schema/type-case.json b/tests/qapi-schema/type-case.json
new file mode 100644
index 0000000000..a43c68e7eb
--- /dev/null
+++ b/tests/qapi-schema/type-case.json
@@ -0,0 +1,2 @@
+# Type names should use CamelCase
+{ 'struct': 'not-a-camel', 'data': {} }
diff --git a/tests/qapi-schema/type-case.out b/tests/qapi-schema/type-case.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/unknown-expr-key.err b/tests/qapi-schema/unknown-expr-key.err
index c5f395bf79..f2538e3ce7 100644
--- a/tests/qapi-schema/unknown-expr-key.err
+++ b/tests/qapi-schema/unknown-expr-key.err
@@ -1,3 +1,3 @@
-unknown-expr-key.json: In struct 'bar':
+unknown-expr-key.json: In struct 'Bar':
unknown-expr-key.json:2: struct has unknown keys 'bogus', 'phony'
Valid keys are 'base', 'data', 'features', 'if', 'struct'.
diff --git a/tests/qapi-schema/unknown-expr-key.json b/tests/qapi-schema/unknown-expr-key.json
index 13292d75ed..8003a0c36e 100644
--- a/tests/qapi-schema/unknown-expr-key.json
+++ b/tests/qapi-schema/unknown-expr-key.json
@@ -1,2 +1,2 @@
# we reject an expression with unknown top-level keys
-{ 'struct': 'bar', 'data': { 'string': 'str'}, 'bogus': { }, 'phony': { } }
+{ 'struct': 'Bar', 'data': { 'string': 'str'}, 'bogus': { }, 'phony': { } }
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 15/29] tests/qapi-schema: Rename redefined-builtin to redefined-predefined
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (13 preceding siblings ...)
2021-03-23 21:56 ` [PULL 14/29] qapi: Enforce type " Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 16/29] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str() Markus Armbruster
` (14 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
The previous commit changed this test to clash with a predefined enum
type, not a built-in type. Adjust its name.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-16-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qapi-schema/meson.build | 2 +-
tests/qapi-schema/redefined-builtin.err | 2 --
tests/qapi-schema/redefined-predefined.err | 2 ++
.../{redefined-builtin.json => redefined-predefined.json} | 0
.../{redefined-builtin.out => redefined-predefined.out} | 0
5 files changed, 3 insertions(+), 3 deletions(-)
delete mode 100644 tests/qapi-schema/redefined-builtin.err
create mode 100644 tests/qapi-schema/redefined-predefined.err
rename tests/qapi-schema/{redefined-builtin.json => redefined-predefined.json} (100%)
rename tests/qapi-schema/{redefined-builtin.out => redefined-predefined.out} (100%)
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index ba11cb76ac..664f9ee22d 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -152,9 +152,9 @@ schemas = [
'pragma-returns-whitelist-crap.json',
'qapi-schema-test.json',
'quoted-structural-chars.json',
- 'redefined-builtin.json',
'redefined-command.json',
'redefined-event.json',
+ 'redefined-predefined.json',
'redefined-type.json',
'reserved-command-q.json',
'reserved-enum-q.json',
diff --git a/tests/qapi-schema/redefined-builtin.err b/tests/qapi-schema/redefined-builtin.err
deleted file mode 100644
index 92bc62dc76..0000000000
--- a/tests/qapi-schema/redefined-builtin.err
+++ /dev/null
@@ -1,2 +0,0 @@
-redefined-builtin.json: In struct 'QType':
-redefined-builtin.json:2: enum type 'QType' is already defined
diff --git a/tests/qapi-schema/redefined-predefined.err b/tests/qapi-schema/redefined-predefined.err
new file mode 100644
index 0000000000..2924dde60b
--- /dev/null
+++ b/tests/qapi-schema/redefined-predefined.err
@@ -0,0 +1,2 @@
+redefined-predefined.json: In struct 'QType':
+redefined-predefined.json:2: enum type 'QType' is already defined
diff --git a/tests/qapi-schema/redefined-builtin.json b/tests/qapi-schema/redefined-predefined.json
similarity index 100%
rename from tests/qapi-schema/redefined-builtin.json
rename to tests/qapi-schema/redefined-predefined.json
diff --git a/tests/qapi-schema/redefined-builtin.out b/tests/qapi-schema/redefined-predefined.out
similarity index 100%
rename from tests/qapi-schema/redefined-builtin.out
rename to tests/qapi-schema/redefined-predefined.out
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 16/29] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str()
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (14 preceding siblings ...)
2021-03-23 21:56 ` [PULL 15/29] tests/qapi-schema: Rename redefined-builtin to redefined-predefined Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 17/29] tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-* Markus Armbruster
` (13 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-17-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/parser.py | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 116afe549a..8eed69333f 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -119,6 +119,13 @@ def _include(self, include, info, incl_fname, previously_included):
return QAPISchemaParser(incl_fname, previously_included, info)
+ def _check_pragma_list_of_str(self, name, value, info):
+ if (not isinstance(value, list)
+ or any([not isinstance(elt, str) for elt in value])):
+ raise QAPISemError(
+ info,
+ "pragma %s must be a list of strings" % name)
+
def _pragma(self, name, value, info):
if name == 'doc-required':
if not isinstance(value, bool):
@@ -126,18 +133,10 @@ def _pragma(self, name, value, info):
"pragma 'doc-required' must be boolean")
info.pragma.doc_required = value
elif name == 'returns-whitelist':
- if (not isinstance(value, list)
- or any([not isinstance(elt, str) for elt in value])):
- raise QAPISemError(
- info,
- "pragma returns-whitelist must be a list of strings")
+ self._check_pragma_list_of_str(name, value, info)
info.pragma.returns_whitelist = value
elif name == 'name-case-whitelist':
- if (not isinstance(value, list)
- or any([not isinstance(elt, str) for elt in value])):
- raise QAPISemError(
- info,
- "pragma name-case-whitelist must be a list of strings")
+ self._check_pragma_list_of_str(name, value, info)
info.pragma.name_case_whitelist = value
else:
raise QAPISemError(info, "unknown pragma '%s'" % name)
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 17/29] tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-*
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (15 preceding siblings ...)
2021-03-23 21:56 ` [PULL 16/29] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str() Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 18/29] tests/qapi-schema: Rename returns-whitelist to returns-bad-type Markus Armbruster
` (12 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Rename pragma-doc-required-crap to pragma-not-bool,
pragma-returns-whitelist-crap to pragma-value-not-list, and
pragma-name-case-whitelist-crap to pragma-value-not-list-of-str.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-18-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qapi-schema/meson.build | 6 +++---
tests/qapi-schema/pragma-doc-required-crap.err | 1 -
tests/qapi-schema/pragma-name-case-whitelist-crap.err | 1 -
tests/qapi-schema/pragma-returns-whitelist-crap.err | 1 -
tests/qapi-schema/pragma-value-not-bool.err | 1 +
...ma-doc-required-crap.json => pragma-value-not-bool.json} | 2 +-
...agma-doc-required-crap.out => pragma-value-not-bool.out} | 0
tests/qapi-schema/pragma-value-not-list-of-str.err | 1 +
...hitelist-crap.json => pragma-value-not-list-of-str.json} | 2 +-
...-whitelist-crap.out => pragma-value-not-list-of-str.out} | 0
tests/qapi-schema/pragma-value-not-list.err | 1 +
...-case-whitelist-crap.json => pragma-value-not-list.json} | 2 +-
...returns-whitelist-crap.out => pragma-value-not-list.out} | 0
13 files changed, 9 insertions(+), 9 deletions(-)
delete mode 100644 tests/qapi-schema/pragma-doc-required-crap.err
delete mode 100644 tests/qapi-schema/pragma-name-case-whitelist-crap.err
delete mode 100644 tests/qapi-schema/pragma-returns-whitelist-crap.err
create mode 100644 tests/qapi-schema/pragma-value-not-bool.err
rename tests/qapi-schema/{pragma-doc-required-crap.json => pragma-value-not-bool.json} (55%)
rename tests/qapi-schema/{pragma-doc-required-crap.out => pragma-value-not-bool.out} (100%)
create mode 100644 tests/qapi-schema/pragma-value-not-list-of-str.err
rename tests/qapi-schema/{pragma-returns-whitelist-crap.json => pragma-value-not-list-of-str.json} (57%)
rename tests/qapi-schema/{pragma-name-case-whitelist-crap.out => pragma-value-not-list-of-str.out} (100%)
create mode 100644 tests/qapi-schema/pragma-value-not-list.err
rename tests/qapi-schema/{pragma-name-case-whitelist-crap.json => pragma-value-not-list.json} (50%)
rename tests/qapi-schema/{pragma-returns-whitelist-crap.out => pragma-value-not-list.out} (100%)
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 664f9ee22d..ffc276b765 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -144,12 +144,12 @@ schemas = [
'oob-coroutine.json',
'oob-test.json',
'allow-preconfig-test.json',
- 'pragma-doc-required-crap.json',
'pragma-extra-junk.json',
- 'pragma-name-case-whitelist-crap.json',
'pragma-non-dict.json',
'pragma-unknown.json',
- 'pragma-returns-whitelist-crap.json',
+ 'pragma-value-not-bool.json',
+ 'pragma-value-not-list-of-str.json',
+ 'pragma-value-not-list.json',
'qapi-schema-test.json',
'quoted-structural-chars.json',
'redefined-command.json',
diff --git a/tests/qapi-schema/pragma-doc-required-crap.err b/tests/qapi-schema/pragma-doc-required-crap.err
deleted file mode 100644
index 717062cb14..0000000000
--- a/tests/qapi-schema/pragma-doc-required-crap.err
+++ /dev/null
@@ -1 +0,0 @@
-pragma-doc-required-crap.json:3: pragma 'doc-required' must be boolean
diff --git a/tests/qapi-schema/pragma-name-case-whitelist-crap.err b/tests/qapi-schema/pragma-name-case-whitelist-crap.err
deleted file mode 100644
index fbea90d6c5..0000000000
--- a/tests/qapi-schema/pragma-name-case-whitelist-crap.err
+++ /dev/null
@@ -1 +0,0 @@
-pragma-name-case-whitelist-crap.json:3: pragma name-case-whitelist must be a list of strings
diff --git a/tests/qapi-schema/pragma-returns-whitelist-crap.err b/tests/qapi-schema/pragma-returns-whitelist-crap.err
deleted file mode 100644
index 69784259df..0000000000
--- a/tests/qapi-schema/pragma-returns-whitelist-crap.err
+++ /dev/null
@@ -1 +0,0 @@
-pragma-returns-whitelist-crap.json:3: pragma returns-whitelist must be a list of strings
diff --git a/tests/qapi-schema/pragma-value-not-bool.err b/tests/qapi-schema/pragma-value-not-bool.err
new file mode 100644
index 0000000000..6247539616
--- /dev/null
+++ b/tests/qapi-schema/pragma-value-not-bool.err
@@ -0,0 +1 @@
+pragma-value-not-bool.json:3: pragma 'doc-required' must be boolean
diff --git a/tests/qapi-schema/pragma-doc-required-crap.json b/tests/qapi-schema/pragma-value-not-bool.json
similarity index 55%
rename from tests/qapi-schema/pragma-doc-required-crap.json
rename to tests/qapi-schema/pragma-value-not-bool.json
index ed763c5ffc..feb489f15b 100644
--- a/tests/qapi-schema/pragma-doc-required-crap.json
+++ b/tests/qapi-schema/pragma-value-not-bool.json
@@ -1,3 +1,3 @@
-# 'doc-required' must be bool
+# pragma value must be bool
{ 'pragma': { 'doc-required': {} } }
diff --git a/tests/qapi-schema/pragma-doc-required-crap.out b/tests/qapi-schema/pragma-value-not-bool.out
similarity index 100%
rename from tests/qapi-schema/pragma-doc-required-crap.out
rename to tests/qapi-schema/pragma-value-not-bool.out
diff --git a/tests/qapi-schema/pragma-value-not-list-of-str.err b/tests/qapi-schema/pragma-value-not-list-of-str.err
new file mode 100644
index 0000000000..44870fe5d9
--- /dev/null
+++ b/tests/qapi-schema/pragma-value-not-list-of-str.err
@@ -0,0 +1 @@
+pragma-value-not-list-of-str.json:3: pragma returns-whitelist must be a list of strings
diff --git a/tests/qapi-schema/pragma-returns-whitelist-crap.json b/tests/qapi-schema/pragma-value-not-list-of-str.json
similarity index 57%
rename from tests/qapi-schema/pragma-returns-whitelist-crap.json
rename to tests/qapi-schema/pragma-value-not-list-of-str.json
index f6b81b093f..12bbbed2e8 100644
--- a/tests/qapi-schema/pragma-returns-whitelist-crap.json
+++ b/tests/qapi-schema/pragma-value-not-list-of-str.json
@@ -1,3 +1,3 @@
-# 'returns-whitelist' must be list of strings
+# pragma value must be list of strings
{ 'pragma': { 'returns-whitelist': [ 'good', [ 'bad' ] ] } }
diff --git a/tests/qapi-schema/pragma-name-case-whitelist-crap.out b/tests/qapi-schema/pragma-value-not-list-of-str.out
similarity index 100%
rename from tests/qapi-schema/pragma-name-case-whitelist-crap.out
rename to tests/qapi-schema/pragma-value-not-list-of-str.out
diff --git a/tests/qapi-schema/pragma-value-not-list.err b/tests/qapi-schema/pragma-value-not-list.err
new file mode 100644
index 0000000000..21736c5723
--- /dev/null
+++ b/tests/qapi-schema/pragma-value-not-list.err
@@ -0,0 +1 @@
+pragma-value-not-list.json:3: pragma name-case-whitelist must be a list of strings
diff --git a/tests/qapi-schema/pragma-name-case-whitelist-crap.json b/tests/qapi-schema/pragma-value-not-list.json
similarity index 50%
rename from tests/qapi-schema/pragma-name-case-whitelist-crap.json
rename to tests/qapi-schema/pragma-value-not-list.json
index 734e1c617b..2c61a97dd3 100644
--- a/tests/qapi-schema/pragma-name-case-whitelist-crap.json
+++ b/tests/qapi-schema/pragma-value-not-list.json
@@ -1,3 +1,3 @@
-# 'name-case-whitelist' must be list of strings
+# pragma value must be list
{ 'pragma': { 'name-case-whitelist': false } }
diff --git a/tests/qapi-schema/pragma-returns-whitelist-crap.out b/tests/qapi-schema/pragma-value-not-list.out
similarity index 100%
rename from tests/qapi-schema/pragma-returns-whitelist-crap.out
rename to tests/qapi-schema/pragma-value-not-list.out
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 18/29] tests/qapi-schema: Rename returns-whitelist to returns-bad-type
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (16 preceding siblings ...)
2021-03-23 21:56 ` [PULL 17/29] tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-* Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 19/29] qapi: Rename pragma *-whitelist to *-exceptions Markus Armbruster
` (11 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
This test covers returning "bad" types. Pragma returns-whitelist is
just one aspect. Naming it returns-whitelist is suboptimal. Rename
to returns-bad-type.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-19-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qapi-schema/meson.build | 2 +-
tests/qapi-schema/returns-bad-type.err | 2 ++
.../{returns-whitelist.json => returns-bad-type.json} | 0
.../qapi-schema/{returns-whitelist.out => returns-bad-type.out} | 0
tests/qapi-schema/returns-whitelist.err | 2 --
5 files changed, 3 insertions(+), 3 deletions(-)
create mode 100644 tests/qapi-schema/returns-bad-type.err
rename tests/qapi-schema/{returns-whitelist.json => returns-bad-type.json} (100%)
rename tests/qapi-schema/{returns-whitelist.out => returns-bad-type.out} (100%)
delete mode 100644 tests/qapi-schema/returns-whitelist.err
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index ffc276b765..4e7635f0a8 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -166,9 +166,9 @@ schemas = [
'reserved-type-list.json',
'returns-alternate.json',
'returns-array-bad.json',
+ 'returns-bad-type.json',
'returns-dict.json',
'returns-unknown.json',
- 'returns-whitelist.json',
'string-code-point-31.json',
'string-code-point-127.json',
'struct-base-clash-deep.json',
diff --git a/tests/qapi-schema/returns-bad-type.err b/tests/qapi-schema/returns-bad-type.err
new file mode 100644
index 0000000000..2c270de9ad
--- /dev/null
+++ b/tests/qapi-schema/returns-bad-type.err
@@ -0,0 +1,2 @@
+returns-bad-type.json: In command 'no-way-this-will-get-whitelisted':
+returns-bad-type.json:14: command's 'returns' cannot take array type ['int']
diff --git a/tests/qapi-schema/returns-whitelist.json b/tests/qapi-schema/returns-bad-type.json
similarity index 100%
rename from tests/qapi-schema/returns-whitelist.json
rename to tests/qapi-schema/returns-bad-type.json
diff --git a/tests/qapi-schema/returns-whitelist.out b/tests/qapi-schema/returns-bad-type.out
similarity index 100%
rename from tests/qapi-schema/returns-whitelist.out
rename to tests/qapi-schema/returns-bad-type.out
diff --git a/tests/qapi-schema/returns-whitelist.err b/tests/qapi-schema/returns-whitelist.err
deleted file mode 100644
index c6e46b9b86..0000000000
--- a/tests/qapi-schema/returns-whitelist.err
+++ /dev/null
@@ -1,2 +0,0 @@
-returns-whitelist.json: In command 'no-way-this-will-get-whitelisted':
-returns-whitelist.json:14: command's 'returns' cannot take array type ['int']
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 19/29] qapi: Rename pragma *-whitelist to *-exceptions
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (17 preceding siblings ...)
2021-03-23 21:56 ` [PULL 18/29] tests/qapi-schema: Rename returns-whitelist to returns-bad-type Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 20/29] qapi/pragma: Streamline comments on member-name-exceptions Markus Armbruster
` (10 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Rename pragma returns-whitelist to command-returns-exceptions, and
name-case-whitelist to member-name-case-exceptions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-20-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
docs/devel/qapi-code-gen.txt | 21 ++++++++++---------
qapi/pragma.json | 4 ++--
qga/qapi-schema.json | 2 +-
scripts/qapi/expr.py | 4 ++--
scripts/qapi/parser.py | 8 +++----
scripts/qapi/schema.py | 2 +-
scripts/qapi/source.py | 8 +++----
tests/qapi-schema/enum-member-case.json | 2 +-
.../pragma-value-not-list-of-str.err | 2 +-
.../pragma-value-not-list-of-str.json | 2 +-
tests/qapi-schema/pragma-value-not-list.err | 2 +-
tests/qapi-schema/pragma-value-not-list.json | 3 +--
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/returns-bad-type.json | 2 +-
14 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 6906a06ad2..23e9823019 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -147,9 +147,10 @@ prevent incomplete include files.
=== Pragma directives ===
Syntax:
- PRAGMA = { 'pragma': { '*doc-required': BOOL,
- '*returns-whitelist': [ STRING, ... ],
- '*name-case-whitelist': [ STRING, ... ] } }
+ PRAGMA = { 'pragma': {
+ '*doc-required': BOOL,
+ '*command-returns-exceptions': [ STRING, ... ],
+ '*member-name-exceptions': [ STRING, ... ] } }
The pragma directive lets you control optional generator behavior.
@@ -159,11 +160,11 @@ pragma to different values in parts of the schema doesn't work.
Pragma 'doc-required' takes a boolean value. If true, documentation
is required. Default is false.
-Pragma 'returns-whitelist' takes a list of command names that may
+Pragma 'command-returns-exceptions' takes a list of commands that may
violate the rules on permitted return types. Default is none.
-Pragma 'name-case-whitelist' takes a list of names that may violate
-rules on use of upper- vs. lower-case letters. Default is none.
+Pragma 'member-name-exceptions' takes a list of types whose member
+names may contain uppercase letters. Default is none.
=== Enumeration types ===
@@ -490,9 +491,9 @@ are the arguments. A union type requires 'boxed': true.
Member 'returns' defines the command's return type. It defaults to an
empty struct type. It must normally be a complex type or an array of
a complex type. To return anything else, the command must be listed
-in pragma 'returns-whitelist'. If you do this, extending the command
-to return additional information will be harder. Use of
-'returns-whitelist' for new commands is strongly discouraged.
+in pragma 'commands-returns-exceptions'. If you do this, extending
+the command to return additional information will be harder. Use of
+the pragma for new commands is strongly discouraged.
A command's error responses are not specified in the QAPI schema.
Error conditions should be documented in comments.
@@ -755,7 +756,7 @@ Any name (command, event, type, member, or enum value) beginning with
"x-" is marked experimental, and may be withdrawn or changed
incompatibly in a future release.
-Pragma 'name-case-whitelist' lets you violate the rules on use of
+Pragma 'member-name-exceptions' lets you violate the rules on use of
upper and lower case. Use for new code is strongly discouraged.
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 7f158e183d..4895848c5e 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -4,13 +4,13 @@
# add to them!
{ 'pragma': {
# Commands allowed to return a non-dictionary:
- 'returns-whitelist': [
+ 'command-returns-exceptions': [
'human-monitor-command',
'qom-get',
'query-tpm-models',
'query-tpm-types',
'ringbuf-read' ],
- 'name-case-whitelist': [
+ 'member-name-exceptions': [
'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
'BlockdevVmdkSubformat', # all members, to match VMDK spec spellings
'BlockdevVmdkAdapterType', # legacyESX, to match VMDK spec spellings
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 9a82b7e952..2b08b761c2 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -20,7 +20,7 @@
# add to them!
{ 'pragma': {
# Commands allowed to return a non-dictionary:
- 'returns-whitelist': [
+ 'command-returns-exceptions': [
'guest-file-open',
'guest-fsfreeze-freeze',
'guest-fsfreeze-freeze-list',
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 7bd15559de..85493efbac 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -181,7 +181,7 @@ def check_type(value, info, source,
raise QAPISemError(info,
"%s should be an object or type name" % source)
- permit_upper = allow_dict in info.pragma.name_case_whitelist
+ permit_upper = allow_dict in info.pragma.member_name_exceptions
# value is a dictionary, check that each member is okay
for (key, arg) in value.items():
@@ -224,7 +224,7 @@ def check_enum(expr, info):
if prefix is not None and not isinstance(prefix, str):
raise QAPISemError(info, "'prefix' must be a string")
- permit_upper = name in info.pragma.name_case_whitelist
+ permit_upper = name in info.pragma.member_name_exceptions
members[:] = [m if isinstance(m, dict) else {'name': m}
for m in members]
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 8eed69333f..c16b8b6995 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -132,12 +132,12 @@ def _pragma(self, name, value, info):
raise QAPISemError(info,
"pragma 'doc-required' must be boolean")
info.pragma.doc_required = value
- elif name == 'returns-whitelist':
+ elif name == 'command-returns-exceptions':
self._check_pragma_list_of_str(name, value, info)
- info.pragma.returns_whitelist = value
- elif name == 'name-case-whitelist':
+ info.pragma.command_returns_exceptions = value
+ elif name == 'member-name-exceptions':
self._check_pragma_list_of_str(name, value, info)
- info.pragma.name_case_whitelist = value
+ info.pragma.member_name_exceptions = value
else:
raise QAPISemError(info, "unknown pragma '%s'" % name)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index ff16578f6d..703b446fd2 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -779,7 +779,7 @@ def check(self, schema):
if self._ret_type_name:
self.ret_type = schema.resolve_type(
self._ret_type_name, self.info, "command's 'returns'")
- if self.name not in self.info.pragma.returns_whitelist:
+ if self.name not in self.info.pragma.command_returns_exceptions:
typ = self.ret_type
if isinstance(typ, QAPISchemaArrayType):
typ = self.ret_type.element_type
diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py
index d7a79a9b8a..d8f9ec377f 100644
--- a/scripts/qapi/source.py
+++ b/scripts/qapi/source.py
@@ -21,10 +21,10 @@ class QAPISchemaPragma:
def __init__(self) -> None:
# Are documentation comments required?
self.doc_required = False
- # Whitelist of commands allowed to return a non-dictionary
- self.returns_whitelist: List[str] = []
- # Whitelist of entities allowed to violate case conventions
- self.name_case_whitelist: List[str] = []
+ # Commands allowed to return a non-dictionary
+ self.command_returns_exceptions: List[str] = []
+ # Types whose member names may violate case conventions
+ self.member_name_exceptions: List[str] = []
class QAPISourceInfo:
diff --git a/tests/qapi-schema/enum-member-case.json b/tests/qapi-schema/enum-member-case.json
index f8af3e4622..b8969d224d 100644
--- a/tests/qapi-schema/enum-member-case.json
+++ b/tests/qapi-schema/enum-member-case.json
@@ -1,4 +1,4 @@
# Member names should be 'lower-case' unless the enum is whitelisted
-{ 'pragma': { 'name-case-whitelist': [ 'UuidInfo' ] } }
+{ 'pragma': { 'member-name-exceptions': [ 'UuidInfo' ] } }
{ 'enum': 'UuidInfo', 'data': [ 'Value' ] } # UuidInfo is whitelisted
{ 'enum': 'NoWayThisWillGetWhitelisted', 'data': [ 'Value' ] }
diff --git a/tests/qapi-schema/pragma-value-not-list-of-str.err b/tests/qapi-schema/pragma-value-not-list-of-str.err
index 44870fe5d9..b81983dd85 100644
--- a/tests/qapi-schema/pragma-value-not-list-of-str.err
+++ b/tests/qapi-schema/pragma-value-not-list-of-str.err
@@ -1 +1 @@
-pragma-value-not-list-of-str.json:3: pragma returns-whitelist must be a list of strings
+pragma-value-not-list-of-str.json:3: pragma command-returns-exceptions must be a list of strings
diff --git a/tests/qapi-schema/pragma-value-not-list-of-str.json b/tests/qapi-schema/pragma-value-not-list-of-str.json
index 12bbbed2e8..27811cff73 100644
--- a/tests/qapi-schema/pragma-value-not-list-of-str.json
+++ b/tests/qapi-schema/pragma-value-not-list-of-str.json
@@ -1,3 +1,3 @@
# pragma value must be list of strings
-{ 'pragma': { 'returns-whitelist': [ 'good', [ 'bad' ] ] } }
+{ 'pragma': { 'command-returns-exceptions': [ 'good', [ 'bad' ] ] } }
diff --git a/tests/qapi-schema/pragma-value-not-list.err b/tests/qapi-schema/pragma-value-not-list.err
index 21736c5723..45dde1bf05 100644
--- a/tests/qapi-schema/pragma-value-not-list.err
+++ b/tests/qapi-schema/pragma-value-not-list.err
@@ -1 +1 @@
-pragma-value-not-list.json:3: pragma name-case-whitelist must be a list of strings
+pragma-value-not-list.json:2: pragma member-name-exceptions must be a list of strings
diff --git a/tests/qapi-schema/pragma-value-not-list.json b/tests/qapi-schema/pragma-value-not-list.json
index 2c61a97dd3..3faa340c3b 100644
--- a/tests/qapi-schema/pragma-value-not-list.json
+++ b/tests/qapi-schema/pragma-value-not-list.json
@@ -1,3 +1,2 @@
# pragma value must be list
-
-{ 'pragma': { 'name-case-whitelist': false } }
+{ 'pragma': { 'member-name-exceptions': false } }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index a355321258..16b03bf308 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -7,7 +7,7 @@
# Whitelists to permit QAPI rule violations
{ 'pragma': {
# Commands allowed to return a non-dictionary:
- 'returns-whitelist': [
+ 'command-returns-exceptions': [
'guest-get-time',
'guest-sync' ] } }
diff --git a/tests/qapi-schema/returns-bad-type.json b/tests/qapi-schema/returns-bad-type.json
index da209329b1..0dd96e739e 100644
--- a/tests/qapi-schema/returns-bad-type.json
+++ b/tests/qapi-schema/returns-bad-type.json
@@ -1,6 +1,6 @@
# we enforce that 'returns' be a dict or array of dict unless whitelisted
-{ 'pragma': { 'returns-whitelist': [
+{ 'pragma': { 'command-returns-exceptions': [
'human-monitor-command', 'query-tpm-models', 'guest-get-time' ] } }
{ 'command': 'human-monitor-command',
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 20/29] qapi/pragma: Streamline comments on member-name-exceptions
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (18 preceding siblings ...)
2021-03-23 21:56 ` [PULL 19/29] qapi: Rename pragma *-whitelist to *-exceptions Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 21/29] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd() Markus Armbruster
` (9 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-21-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/pragma.json | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 4895848c5e..4c47c802d1 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -10,11 +10,13 @@
'query-tpm-models',
'query-tpm-types',
'ringbuf-read' ],
- 'member-name-exceptions': [
- 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
- 'BlockdevVmdkSubformat', # all members, to match VMDK spec spellings
- 'BlockdevVmdkAdapterType', # legacyESX, to match VMDK spec spellings
- 'QapiErrorClass', # all members, visible through errors
- 'UuidInfo', # UUID, visible through query-uuid
- 'X86CPURegister32' # all members, visible indirectly through qom-get
+ # Externally visible types whose member names may use uppercase
+ 'member-name-exceptions': [ # visible in:
+ 'ACPISlotType', # query-acpi-ospm-status
+ 'BlockdevVmdkAdapterType', # blockdev-create (to match VMDK spec)
+ 'BlockdevVmdkSubformat', # blockdev-create (to match VMDK spec)
+ 'QapiErrorClass', # QMP error replies
+ 'UuidInfo', # query-uuid
+ 'X86CPURegister32' # qom-get of x86 CPU properties
+ # feature-words, filtered-features
] } }
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 21/29] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd()
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (19 preceding siblings ...)
2021-03-23 21:56 ` [PULL 20/29] qapi/pragma: Streamline comments on member-name-exceptions Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 22/29] qapi: Prepare for rejecting underscore in command and member names Markus Armbruster
` (8 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Commit 967c885108 "qapi: add 'if' to top-level expressions" added
command TestIfCmd with an 'if' condition. It also added the
qmp_TestIfCmd() to go with it, guarded by the corresponding #if.
Commit ccadd6bcba "qapi: Add 'if' to implicit struct members" changed
the command, but not the function. Compiles only because we don't
satisfy the #if. Instead of fixing the function, simply drop it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-22-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/unit/test-qmp-cmds.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 266db074b4..99973dde7c 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -13,13 +13,6 @@
static QmpCommandList qmp_commands;
-#if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD)
-UserDefThree *qmp_TestIfCmd(TestIfStruct *foo, Error **errp)
-{
- return NULL;
-}
-#endif
-
UserDefThree *qmp_TestCmdReturnDefThree(Error **errp)
{
return NULL;
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 22/29] qapi: Prepare for rejecting underscore in command and member names
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (20 preceding siblings ...)
2021-03-23 21:56 ` [PULL 21/29] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd() Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 23/29] qapi: Enforce feature naming rules Markus Armbruster
` (7 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Command names and member names within a type should be all lower case
with words separated by a hyphen. We also accept underscore. Rework
check_name_lower() to optionally reject underscores, but don't use
that option, yet.
Update expected test output for the changed error message.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-23-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/expr.py | 24 ++++++++++++++----------
tests/qapi-schema/args-member-case.err | 2 +-
tests/qapi-schema/enum-member-case.err | 2 +-
tests/qapi-schema/union-branch-case.err | 2 +-
4 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 85493efbac..e416cc5162 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -51,12 +51,13 @@ def check_name_upper(name, info, source):
def check_name_lower(name, info, source,
- permit_upper=False):
+ permit_upper=False,
+ permit_underscore=False):
stem = check_name_str(name, info, source)
- if not permit_upper and re.search(r'[A-Z]', stem):
+ if ((not permit_upper and re.search(r'[A-Z]', stem))
+ or (not permit_underscore and '_' in stem)):
raise QAPISemError(
- info, "%s uses uppercase in name" % source)
- # TODO reject '_' in stem
+ info, "name of %s must not use uppercase or '_'" % source)
def check_name_camel(name, info, source):
@@ -69,7 +70,8 @@ def check_defn_name_str(name, info, meta):
if meta == 'event':
check_name_upper(name, info, meta)
elif meta == 'command':
- check_name_lower(name, info, meta, permit_upper=True)
+ check_name_lower(name, info, meta,
+ permit_upper=True, permit_underscore=True)
else:
check_name_camel(name, info, meta)
if name.endswith('Kind') or name.endswith('List'):
@@ -188,7 +190,8 @@ def check_type(value, info, source,
key_source = "%s member '%s'" % (source, key)
if key.startswith('*'):
key = key[1:]
- check_name_lower(key, info, key_source, permit_upper)
+ check_name_lower(key, info, key_source,
+ permit_upper, permit_underscore=True)
if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
raise QAPISemError(info, "%s uses reserved name" % key_source)
check_keys(arg, info, key_source, ['type'], ['if', 'features'])
@@ -210,7 +213,7 @@ def check_features(features, info):
check_keys(f, info, source, ['name'], ['if'])
check_name_is_str(f['name'], info, source)
source = "%s '%s'" % (source, f['name'])
- check_name_lower(f['name'], info, source)
+ check_name_lower(f['name'], info, source, permit_underscore=True)
check_if(f, info, source)
@@ -237,7 +240,8 @@ def check_enum(expr, info):
# Enum members may start with a digit
if member_name[0].isdigit():
member_name = 'd' + member_name # Hack: hide the digit
- check_name_lower(member_name, info, source, permit_upper)
+ check_name_lower(member_name, info, source,
+ permit_upper, permit_underscore=True)
check_if(member, info, source)
@@ -267,7 +271,7 @@ def check_union(expr, info):
for (key, value) in members.items():
source = "'data' member '%s'" % key
if discriminator is None:
- check_name_lower(key, info, source)
+ check_name_lower(key, info, source, permit_underscore=True)
# else: name is in discriminator enum, which gets checked
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
@@ -281,7 +285,7 @@ def check_alternate(expr, info):
raise QAPISemError(info, "'data' must not be empty")
for (key, value) in members.items():
source = "'data' member '%s'" % key
- check_name_lower(key, info, source)
+ check_name_lower(key, info, source, permit_underscore=True)
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
check_type(value['type'], info, source)
diff --git a/tests/qapi-schema/args-member-case.err b/tests/qapi-schema/args-member-case.err
index 4f33dbbc38..25c3910a17 100644
--- a/tests/qapi-schema/args-member-case.err
+++ b/tests/qapi-schema/args-member-case.err
@@ -1,2 +1,2 @@
args-member-case.json: In command 'no-way-this-will-get-whitelisted':
-args-member-case.json:2: 'data' member 'Arg' uses uppercase in name
+args-member-case.json:2: name of 'data' member 'Arg' must not use uppercase or '_'
diff --git a/tests/qapi-schema/enum-member-case.err b/tests/qapi-schema/enum-member-case.err
index 8b3aefe37a..c813522c50 100644
--- a/tests/qapi-schema/enum-member-case.err
+++ b/tests/qapi-schema/enum-member-case.err
@@ -1,2 +1,2 @@
enum-member-case.json: In enum 'NoWayThisWillGetWhitelisted':
-enum-member-case.json:4: 'data' member 'Value' uses uppercase in name
+enum-member-case.json:4: name of 'data' member 'Value' must not use uppercase or '_'
diff --git a/tests/qapi-schema/union-branch-case.err b/tests/qapi-schema/union-branch-case.err
index b1e9417303..d2d5cb8993 100644
--- a/tests/qapi-schema/union-branch-case.err
+++ b/tests/qapi-schema/union-branch-case.err
@@ -1,2 +1,2 @@
union-branch-case.json: In union 'Uni':
-union-branch-case.json:2: 'data' member 'Branch' uses uppercase in name
+union-branch-case.json:2: name of 'data' member 'Branch' must not use uppercase or '_'
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 23/29] qapi: Enforce feature naming rules
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (21 preceding siblings ...)
2021-03-23 21:56 ` [PULL 22/29] qapi: Prepare for rejecting underscore in command and member names Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 24/29] qapi: Enforce command " Markus Armbruster
` (6 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Feature names should use '-', not '_'. Enforce this.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-24-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/expr.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index e416cc5162..d778107c18 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -213,7 +213,7 @@ def check_features(features, info):
check_keys(f, info, source, ['name'], ['if'])
check_name_is_str(f['name'], info, source)
source = "%s '%s'" % (source, f['name'])
- check_name_lower(f['name'], info, source, permit_underscore=True)
+ check_name_lower(f['name'], info, source)
check_if(f, info, source)
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 24/29] qapi: Enforce command naming rules
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (22 preceding siblings ...)
2021-03-23 21:56 ` [PULL 23/29] qapi: Enforce feature naming rules Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 25/29] tests/qapi-schema: Switch member name clash test to struct Markus Armbruster
` (5 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Command names should be lower-case. Enforce this. Fix the fixable
offenders (all in tests/), and add the remainder to pragma
command-name-exceptions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-25-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
docs/devel/qapi-code-gen.txt | 8 ++++++--
qapi/pragma.json | 18 +++++++++++++++++
tests/unit/test-qmp-cmds.c | 10 +++++-----
scripts/qapi/expr.py | 5 +++--
scripts/qapi/parser.py | 3 +++
scripts/qapi/source.py | 2 ++
tests/qapi-schema/qapi-schema-test.json | 21 +++++++++++---------
tests/qapi-schema/qapi-schema-test.out | 26 ++++++++++++-------------
8 files changed, 62 insertions(+), 31 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 23e9823019..10e9a0f829 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -149,6 +149,7 @@ prevent incomplete include files.
Syntax:
PRAGMA = { 'pragma': {
'*doc-required': BOOL,
+ '*command-name-exceptions': [ STRING, ... ],
'*command-returns-exceptions': [ STRING, ... ],
'*member-name-exceptions': [ STRING, ... ] } }
@@ -160,6 +161,9 @@ pragma to different values in parts of the schema doesn't work.
Pragma 'doc-required' takes a boolean value. If true, documentation
is required. Default is false.
+Pragma 'command-name-exceptions' takes a list of commands whose names
+may contain '_' instead of '-'. Default is none.
+
Pragma 'command-returns-exceptions' takes a list of commands that may
violate the rules on permitted return types. Default is none.
@@ -756,8 +760,8 @@ Any name (command, event, type, member, or enum value) beginning with
"x-" is marked experimental, and may be withdrawn or changed
incompatibly in a future release.
-Pragma 'member-name-exceptions' lets you violate the rules on use of
-upper and lower case. Use for new code is strongly discouraged.
+Pragmas 'command-name-exceptions' and 'member-name-exceptions' let you
+violate naming rules. Use for new code is strongly discouraged.
=== Downstream extensions ===
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 4c47c802d1..339f067943 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -4,6 +4,24 @@
# add to them!
{ 'pragma': {
# Commands allowed to return a non-dictionary:
+ 'command-name-exceptions': [
+ 'add_client',
+ 'block_passwd',
+ 'block_resize',
+ 'block_set_io_throttle',
+ 'client_migrate_info',
+ 'device_add',
+ 'device_del',
+ 'expire_password',
+ 'migrate_cancel',
+ 'netdev_add',
+ 'netdev_del',
+ 'qmp_capabilities',
+ 'set_link',
+ 'set_password',
+ 'system_powerdown',
+ 'system_reset',
+ 'system_wakeup' ],
'command-returns-exceptions': [
'human-monitor-command',
'qom-get',
diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c
index 99973dde7c..1b0b7d99df 100644
--- a/tests/unit/test-qmp-cmds.c
+++ b/tests/unit/test-qmp-cmds.c
@@ -13,7 +13,7 @@
static QmpCommandList qmp_commands;
-UserDefThree *qmp_TestCmdReturnDefThree(Error **errp)
+UserDefThree *qmp_test_cmd_return_def_three(Error **errp)
{
return NULL;
}
@@ -199,7 +199,7 @@ static void test_dispatch_cmd(void)
ret = qobject_to(QDict,
do_qmp_dispatch(false,
- "{ 'execute': 'user_def_cmd' }"));
+ "{ 'execute': 'user-def-cmd' }"));
assert(ret && qdict_size(ret) == 0);
qobject_unref(ret);
}
@@ -220,11 +220,11 @@ static void test_dispatch_cmd_failure(void)
{
/* missing arguments */
do_qmp_dispatch_error(false, ERROR_CLASS_GENERIC_ERROR,
- "{ 'execute': 'user_def_cmd2' }");
+ "{ 'execute': 'user-def-cmd2' }");
/* extra arguments */
do_qmp_dispatch_error(false, ERROR_CLASS_GENERIC_ERROR,
- "{ 'execute': 'user_def_cmd',"
+ "{ 'execute': 'user-def-cmd',"
" 'arguments': { 'a': 66 } }");
}
@@ -248,7 +248,7 @@ static void test_dispatch_cmd_io(void)
int64_t val;
ret = qobject_to(QDict, do_qmp_dispatch(false,
- "{ 'execute': 'user_def_cmd2', 'arguments': {"
+ "{ 'execute': 'user-def-cmd2', 'arguments': {"
" 'ud1a': { 'integer': 42, 'string': 'hello' },"
" 'ud1b': { 'integer': 422, 'string': 'hello2' } } }"));
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index d778107c18..9193e68763 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -70,8 +70,9 @@ def check_defn_name_str(name, info, meta):
if meta == 'event':
check_name_upper(name, info, meta)
elif meta == 'command':
- check_name_lower(name, info, meta,
- permit_upper=True, permit_underscore=True)
+ check_name_lower(
+ name, info, meta,
+ permit_underscore=name in info.pragma.command_name_exceptions)
else:
check_name_camel(name, info, meta)
if name.endswith('Kind') or name.endswith('List'):
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index c16b8b6995..58267c3db9 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -132,6 +132,9 @@ def _pragma(self, name, value, info):
raise QAPISemError(info,
"pragma 'doc-required' must be boolean")
info.pragma.doc_required = value
+ elif name == 'command-name-exceptions':
+ self._check_pragma_list_of_str(name, value, info)
+ info.pragma.command_name_exceptions = value
elif name == 'command-returns-exceptions':
self._check_pragma_list_of_str(name, value, info)
info.pragma.command_returns_exceptions = value
diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py
index d8f9ec377f..03b6ede082 100644
--- a/scripts/qapi/source.py
+++ b/scripts/qapi/source.py
@@ -21,6 +21,8 @@ class QAPISchemaPragma:
def __init__(self) -> None:
# Are documentation comments required?
self.doc_required = False
+ # Commands whose names may use '_'
+ self.command_name_exceptions: List[str] = []
# Commands allowed to return a non-dictionary
self.command_returns_exceptions: List[str] = []
# Types whose member names may violate case conventions
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 16b03bf308..e635db4a35 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -31,7 +31,7 @@
'base': { 'type': 'EnumOne' }, 'discriminator': 'type',
'data': { } }
-{ 'command': 'user_def_cmd0', 'data': 'Empty2', 'returns': 'Empty2' }
+{ 'command': 'user-def-cmd0', 'data': 'Empty2', 'returns': 'Empty2' }
# for testing override of default naming heuristic
{ 'enum': 'QEnumTwo',
@@ -141,9 +141,9 @@
{ 'include': 'include/sub-module.json' }
# testing commands
-{ 'command': 'user_def_cmd', 'data': {} }
-{ 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
-{ 'command': 'user_def_cmd2',
+{ 'command': 'user-def-cmd', 'data': {} }
+{ 'command': 'user-def-cmd1', 'data': {'ud1a': 'UserDefOne'} }
+{ 'command': 'user-def-cmd2',
'data': {'ud1a': {'type': 'UserDefOne'}, '*ud1b': 'UserDefOne'},
'returns': 'UserDefTwo' }
@@ -230,7 +230,8 @@
'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
-{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' },
+{ 'command': 'test-if-union-cmd',
+ 'data': { 'union_cmd_arg': 'TestIfUnion' },
'if': 'defined(TEST_IF_UNION)' }
{ 'alternate': 'TestIfAlternate', 'data':
@@ -238,16 +239,18 @@
'bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} },
'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
-{ 'command': 'TestIfAlternateCmd', 'data': { 'alt_cmd_arg': 'TestIfAlternate' },
+{ 'command': 'test-if-alternate-cmd',
+ 'data': { 'alt_cmd_arg': 'TestIfAlternate' },
'if': 'defined(TEST_IF_ALT)' }
-{ 'command': 'TestIfCmd', 'data':
- { 'foo': 'TestIfStruct',
+{ 'command': 'test-if-cmd',
+ 'data': {
+ 'foo': 'TestIfStruct',
'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } },
'returns': 'UserDefThree',
'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] }
-{ 'command': 'TestCmdReturnDefThree', 'returns': 'UserDefThree' }
+{ 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' }
{ 'event': 'TEST_IF_EVENT', 'data':
{ 'foo': 'TestIfStruct',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 882d0e7c56..3f1ea345fd 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -32,7 +32,7 @@ object Union
case value2: q_empty
case value3: q_empty
case value4: q_empty
-command user_def_cmd0 Empty2 -> Empty2
+command user-def-cmd0 Empty2 -> Empty2
gen=True success_response=True boxed=False oob=False preconfig=False
enum QEnumTwo
prefix QENUM_TWO
@@ -190,16 +190,16 @@ object UserDefListUnion
case any: q_obj_anyList-wrapper
case user: q_obj_StatusList-wrapper
include include/sub-module.json
-command user_def_cmd None -> None
+command user-def-cmd None -> None
gen=True success_response=True boxed=False oob=False preconfig=False
-object q_obj_user_def_cmd1-arg
+object q_obj_user-def-cmd1-arg
member ud1a: UserDefOne optional=False
-command user_def_cmd1 q_obj_user_def_cmd1-arg -> None
+command user-def-cmd1 q_obj_user-def-cmd1-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
-object q_obj_user_def_cmd2-arg
+object q_obj_user-def-cmd2-arg
member ud1a: UserDefOne optional=False
member ud1b: UserDefOne optional=True
-command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo
+command user-def-cmd2 q_obj_user-def-cmd2-arg -> UserDefTwo
gen=True success_response=True boxed=False oob=False preconfig=False
command cmd-success-response None -> None
gen=True success_response=False boxed=False oob=False preconfig=False
@@ -319,10 +319,10 @@ object TestIfUnion
case union_bar: q_obj_str-wrapper
if ['defined(TEST_IF_UNION_BAR)']
if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
-object q_obj_TestIfUnionCmd-arg
+object q_obj_test-if-union-cmd-arg
member union_cmd_arg: TestIfUnion optional=False
if ['defined(TEST_IF_UNION)']
-command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None
+command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
if ['defined(TEST_IF_UNION)']
alternate TestIfAlternate
@@ -331,21 +331,21 @@ alternate TestIfAlternate
case bar: TestStruct
if ['defined(TEST_IF_ALT_BAR)']
if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
-object q_obj_TestIfAlternateCmd-arg
+object q_obj_test-if-alternate-cmd-arg
member alt_cmd_arg: TestIfAlternate optional=False
if ['defined(TEST_IF_ALT)']
-command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None
+command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
if ['defined(TEST_IF_ALT)']
-object q_obj_TestIfCmd-arg
+object q_obj_test-if-cmd-arg
member foo: TestIfStruct optional=False
member bar: TestIfEnum optional=False
if ['defined(TEST_IF_CMD_BAR)']
if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
-command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree
+command test-if-cmd q_obj_test-if-cmd-arg -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)']
-command TestCmdReturnDefThree None -> UserDefThree
+command test-cmd-return-def-three None -> UserDefThree
gen=True success_response=True boxed=False oob=False preconfig=False
array TestIfEnumList TestIfEnum
if ['defined(TEST_IF_ENUM)']
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 25/29] tests/qapi-schema: Switch member name clash test to struct
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (23 preceding siblings ...)
2021-03-23 21:56 ` [PULL 24/29] qapi: Enforce command " Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 26/29] qapi: Enforce struct member naming rules Markus Armbruster
` (4 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Test args-name-clash covers command parameter name clash. This
effectively covers struct member name clash as well. The next commit
will make parameter name clash impossible. Convert args-name-clash
from testing command to testing a struct, and rename it to
struct-member-name-clash.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-26-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message typo fixed]
---
tests/qapi-schema/args-name-clash.err | 2 --
tests/qapi-schema/meson.build | 2 +-
tests/qapi-schema/struct-member-name-clash.err | 2 ++
.../{args-name-clash.json => struct-member-name-clash.json} | 2 +-
.../{args-name-clash.out => struct-member-name-clash.out} | 0
5 files changed, 4 insertions(+), 4 deletions(-)
delete mode 100644 tests/qapi-schema/args-name-clash.err
create mode 100644 tests/qapi-schema/struct-member-name-clash.err
rename tests/qapi-schema/{args-name-clash.json => struct-member-name-clash.json} (64%)
rename tests/qapi-schema/{args-name-clash.out => struct-member-name-clash.out} (100%)
diff --git a/tests/qapi-schema/args-name-clash.err b/tests/qapi-schema/args-name-clash.err
deleted file mode 100644
index 3e04817bc0..0000000000
--- a/tests/qapi-schema/args-name-clash.err
+++ /dev/null
@@ -1,2 +0,0 @@
-args-name-clash.json: In command 'oops':
-args-name-clash.json:4: parameter 'a_b' collides with parameter 'a-b'
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 4e7635f0a8..8ba6917132 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -30,7 +30,6 @@ schemas = [
'args-member-array-bad.json',
'args-member-case.json',
'args-member-unknown.json',
- 'args-name-clash.json',
'args-union.json',
'args-unknown.json',
'bad-base.json',
@@ -177,6 +176,7 @@ schemas = [
'struct-member-if-invalid.json',
'struct-member-invalid-dict.json',
'struct-member-invalid.json',
+ 'struct-member-name-clash.json',
'trailing-comma-list.json',
'trailing-comma-object.json',
'type-bypass-bad-gen.json',
diff --git a/tests/qapi-schema/struct-member-name-clash.err b/tests/qapi-schema/struct-member-name-clash.err
new file mode 100644
index 0000000000..6ac042d59d
--- /dev/null
+++ b/tests/qapi-schema/struct-member-name-clash.err
@@ -0,0 +1,2 @@
+struct-member-name-clash.json: In struct 'Oops':
+struct-member-name-clash.json:4: member 'a_b' collides with member 'a-b'
diff --git a/tests/qapi-schema/args-name-clash.json b/tests/qapi-schema/struct-member-name-clash.json
similarity index 64%
rename from tests/qapi-schema/args-name-clash.json
rename to tests/qapi-schema/struct-member-name-clash.json
index 61423cb893..3fb69cc2ce 100644
--- a/tests/qapi-schema/args-name-clash.json
+++ b/tests/qapi-schema/struct-member-name-clash.json
@@ -1,4 +1,4 @@
# C member name collision
# Reject members that clash when mapped to C names (we would have two 'a_b'
# members).
-{ 'command': 'oops', 'data': { 'a-b': 'str', 'a_b': 'str' } }
+{ 'struct': 'Oops', 'data': { 'a-b': 'str', 'a_b': 'str' } }
diff --git a/tests/qapi-schema/args-name-clash.out b/tests/qapi-schema/struct-member-name-clash.out
similarity index 100%
rename from tests/qapi-schema/args-name-clash.out
rename to tests/qapi-schema/struct-member-name-clash.out
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 26/29] qapi: Enforce struct member naming rules
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (24 preceding siblings ...)
2021-03-23 21:56 ` [PULL 25/29] tests/qapi-schema: Switch member name clash test to struct Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 27/29] qapi: Enforce enum " Markus Armbruster
` (3 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Struct members, including command arguments, event data, and union
inline base members, should use '-', not '_'. Enforce this. Fix the
fixable offenders (all in tests/), and add the remainder to pragma
member-name-exceptions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-27-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
docs/devel/qapi-code-gen.txt | 3 ++-
qapi/pragma.json | 17 +++++++++++++++++
qga/qapi-schema.json | 4 ++++
scripts/qapi/expr.py | 5 +++--
tests/qapi-schema/qapi-schema-test.json | 8 ++++++--
tests/qapi-schema/qapi-schema-test.out | 4 ++--
tests/qapi-schema/struct-member-name-clash.err | 2 +-
tests/qapi-schema/struct-member-name-clash.json | 1 +
8 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 10e9a0f829..c1cb6f987d 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -168,7 +168,8 @@ Pragma 'command-returns-exceptions' takes a list of commands that may
violate the rules on permitted return types. Default is none.
Pragma 'member-name-exceptions' takes a list of types whose member
-names may contain uppercase letters. Default is none.
+names may contain uppercase letters, and '_' instead of '-'. Default
+is none.
=== Enumeration types ===
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 339f067943..f422a1a2ac 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -31,10 +31,27 @@
# Externally visible types whose member names may use uppercase
'member-name-exceptions': [ # visible in:
'ACPISlotType', # query-acpi-ospm-status
+ 'AcpiTableOptions', # -acpitable
+ 'BlkdebugSetStateOptions', # blockdev-add, -blockdev
+ 'BlockDeviceInfo', # query-block
+ 'BlockDeviceStats', # query-blockstats
+ 'BlockDeviceTimedStats', # query-blockstats
+ 'BlockIOThrottle', # block_set_io_throttle
+ 'BlockInfo', # query-block
'BlockdevVmdkAdapterType', # blockdev-create (to match VMDK spec)
'BlockdevVmdkSubformat', # blockdev-create (to match VMDK spec)
+ 'ColoCompareProperties', # object_add, -object
+ 'FilterMirrorProperties', # object_add, -object
+ 'FilterRedirectorProperties', # object_add, -object
+ 'FilterRewriterProperties', # object_add, -object
+ 'InputLinuxProperties', # object_add, -object
+ 'NetdevTapOptions', # netdev_add, query-netdev, -netdev
+ 'PciBusInfo', # query-pci
+ 'PciDeviceInfo', # query-pci
+ 'PciMemoryRegion', # query-pci
'QapiErrorClass', # QMP error replies
'UuidInfo', # query-uuid
+ 'VncClientInfo', # query-vnc, query-vnc-servers, ...
'X86CPURegister32' # qom-get of x86 CPU properties
# feature-words, filtered-features
] } }
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 2b08b761c2..fb17eebde3 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -19,6 +19,10 @@
# Whitelists to permit QAPI rule violations; think twice before you
# add to them!
{ 'pragma': {
+ # Types whose member names may use '_'
+ 'member-name-exceptions': [
+ 'GuestAgentInfo'
+ ],
# Commands allowed to return a non-dictionary:
'command-returns-exceptions': [
'guest-file-open',
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 9193e68763..ba9f7ad350 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -184,7 +184,7 @@ def check_type(value, info, source,
raise QAPISemError(info,
"%s should be an object or type name" % source)
- permit_upper = allow_dict in info.pragma.member_name_exceptions
+ permissive = allow_dict in info.pragma.member_name_exceptions
# value is a dictionary, check that each member is okay
for (key, arg) in value.items():
@@ -192,7 +192,8 @@ def check_type(value, info, source,
if key.startswith('*'):
key = key[1:]
check_name_lower(key, info, key_source,
- permit_upper, permit_underscore=True)
+ permit_upper=permissive,
+ permit_underscore=permissive)
if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
raise QAPISemError(info, "%s uses reserved name" % key_source)
check_keys(arg, info, key_source, ['type'], ['if', 'features'])
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index e635db4a35..387678acbb 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -6,6 +6,10 @@
# Whitelists to permit QAPI rule violations
{ 'pragma': {
+ # Types whose member names may use '_'
+ 'member-name-exceptions': [
+ 'UserDefA'
+ ],
# Commands allowed to return a non-dictionary:
'command-returns-exceptions': [
'guest-get-time',
@@ -231,7 +235,7 @@
'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
{ 'command': 'test-if-union-cmd',
- 'data': { 'union_cmd_arg': 'TestIfUnion' },
+ 'data': { 'union-cmd-arg': 'TestIfUnion' },
'if': 'defined(TEST_IF_UNION)' }
{ 'alternate': 'TestIfAlternate', 'data':
@@ -240,7 +244,7 @@
'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' }
{ 'command': 'test-if-alternate-cmd',
- 'data': { 'alt_cmd_arg': 'TestIfAlternate' },
+ 'data': { 'alt-cmd-arg': 'TestIfAlternate' },
'if': 'defined(TEST_IF_ALT)' }
{ 'command': 'test-if-cmd',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 3f1ea345fd..51efe5d7cd 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -320,7 +320,7 @@ object TestIfUnion
if ['defined(TEST_IF_UNION_BAR)']
if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
object q_obj_test-if-union-cmd-arg
- member union_cmd_arg: TestIfUnion optional=False
+ member union-cmd-arg: TestIfUnion optional=False
if ['defined(TEST_IF_UNION)']
command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
@@ -332,7 +332,7 @@ alternate TestIfAlternate
if ['defined(TEST_IF_ALT_BAR)']
if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)']
object q_obj_test-if-alternate-cmd-arg
- member alt_cmd_arg: TestIfAlternate optional=False
+ member alt-cmd-arg: TestIfAlternate optional=False
if ['defined(TEST_IF_ALT)']
command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None
gen=True success_response=True boxed=False oob=False preconfig=False
diff --git a/tests/qapi-schema/struct-member-name-clash.err b/tests/qapi-schema/struct-member-name-clash.err
index 6ac042d59d..7e53a605d2 100644
--- a/tests/qapi-schema/struct-member-name-clash.err
+++ b/tests/qapi-schema/struct-member-name-clash.err
@@ -1,2 +1,2 @@
struct-member-name-clash.json: In struct 'Oops':
-struct-member-name-clash.json:4: member 'a_b' collides with member 'a-b'
+struct-member-name-clash.json:5: member 'a_b' collides with member 'a-b'
diff --git a/tests/qapi-schema/struct-member-name-clash.json b/tests/qapi-schema/struct-member-name-clash.json
index 3fb69cc2ce..571acf05ce 100644
--- a/tests/qapi-schema/struct-member-name-clash.json
+++ b/tests/qapi-schema/struct-member-name-clash.json
@@ -1,4 +1,5 @@
# C member name collision
# Reject members that clash when mapped to C names (we would have two 'a_b'
# members).
+{ 'pragma': { 'member-name-exceptions': [ 'Oops' ] } }
{ 'struct': 'Oops', 'data': { 'a-b': 'str', 'a_b': 'str' } }
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 27/29] qapi: Enforce enum member naming rules
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (25 preceding siblings ...)
2021-03-23 21:56 ` [PULL 26/29] qapi: Enforce struct member naming rules Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 28/29] qapi: Enforce union and alternate branch " Markus Armbruster
` (2 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Enum members should use '-', not '_'. Enforce this. Fix the fixable
offenders (all in tests/), and add the remainder to pragma
member-name-exceptions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-28-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/pragma.json | 8 ++++++++
scripts/qapi/expr.py | 5 +++--
tests/qapi-schema/enum-clash-member.err | 2 +-
tests/qapi-schema/enum-clash-member.json | 1 +
4 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/qapi/pragma.json b/qapi/pragma.json
index f422a1a2ac..b4e17167e1 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -32,12 +32,15 @@
'member-name-exceptions': [ # visible in:
'ACPISlotType', # query-acpi-ospm-status
'AcpiTableOptions', # -acpitable
+ 'BlkdebugEvent', # blockdev-add, -blockdev
'BlkdebugSetStateOptions', # blockdev-add, -blockdev
'BlockDeviceInfo', # query-block
'BlockDeviceStats', # query-blockstats
'BlockDeviceTimedStats', # query-blockstats
'BlockIOThrottle', # block_set_io_throttle
'BlockInfo', # query-block
+ 'BlockdevAioOptions', # blockdev-add, -blockdev
+ 'BlockdevDriver', # blockdev-add, query-blockstats, ...
'BlockdevVmdkAdapterType', # blockdev-create (to match VMDK spec)
'BlockdevVmdkSubformat', # blockdev-create (to match VMDK spec)
'ColoCompareProperties', # object_add, -object
@@ -46,10 +49,15 @@
'FilterRewriterProperties', # object_add, -object
'InputLinuxProperties', # object_add, -object
'NetdevTapOptions', # netdev_add, query-netdev, -netdev
+ 'ObjectType', # object-add, -object
+ 'PCIELinkSpeed', # internal only
'PciBusInfo', # query-pci
'PciDeviceInfo', # query-pci
'PciMemoryRegion', # query-pci
+ 'QKeyCode', # send-key, input-sent-event
'QapiErrorClass', # QMP error replies
+ 'SshHostKeyCheckMode', # blockdev-add, -blockdev
+ 'SysEmuTarget', # query-cpu-fast, query-target
'UuidInfo', # query-uuid
'VncClientInfo', # query-vnc, query-vnc-servers, ...
'X86CPURegister32' # qom-get of x86 CPU properties
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index ba9f7ad350..d968609c48 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -229,7 +229,7 @@ def check_enum(expr, info):
if prefix is not None and not isinstance(prefix, str):
raise QAPISemError(info, "'prefix' must be a string")
- permit_upper = name in info.pragma.member_name_exceptions
+ permissive = name in info.pragma.member_name_exceptions
members[:] = [m if isinstance(m, dict) else {'name': m}
for m in members]
@@ -243,7 +243,8 @@ def check_enum(expr, info):
if member_name[0].isdigit():
member_name = 'd' + member_name # Hack: hide the digit
check_name_lower(member_name, info, source,
- permit_upper, permit_underscore=True)
+ permit_upper=permissive,
+ permit_underscore=permissive)
check_if(member, info, source)
diff --git a/tests/qapi-schema/enum-clash-member.err b/tests/qapi-schema/enum-clash-member.err
index 5986571427..e4eb102ae2 100644
--- a/tests/qapi-schema/enum-clash-member.err
+++ b/tests/qapi-schema/enum-clash-member.err
@@ -1,2 +1,2 @@
enum-clash-member.json: In enum 'MyEnum':
-enum-clash-member.json:2: value 'one_two' collides with value 'one-two'
+enum-clash-member.json:3: value 'one_two' collides with value 'one-two'
diff --git a/tests/qapi-schema/enum-clash-member.json b/tests/qapi-schema/enum-clash-member.json
index b6928b8bfd..82bcbf724b 100644
--- a/tests/qapi-schema/enum-clash-member.json
+++ b/tests/qapi-schema/enum-clash-member.json
@@ -1,2 +1,3 @@
# we reject enums where members will clash when mapped to C enum
+{ 'pragma': { 'member-name-exceptions': [ 'MyEnum' ] } }
{ 'enum': 'MyEnum', 'data': [ 'one-two', 'one_two' ] }
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 28/29] qapi: Enforce union and alternate branch naming rules
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (26 preceding siblings ...)
2021-03-23 21:56 ` [PULL 27/29] qapi: Enforce enum " Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 21:56 ` [PULL 29/29] block: Remove monitor command block_passwd Markus Armbruster
2021-03-23 23:46 ` [PULL 00/29] QAPI patches patches for 2021-03-23 Peter Maydell
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell
Union branch names should use '-', not '_'. Enforce this. The only
offenders are in tests/. Fix them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323094025.3569441-29-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message typo fixed]
---
scripts/qapi/expr.py | 4 ++--
tests/qapi-schema/alternate-clash.err | 2 +-
tests/qapi-schema/alternate-clash.json | 6 ++++--
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/qapi-schema-test.out | 4 ++--
tests/qapi-schema/union-clash-branches.err | 2 +-
tests/qapi-schema/union-clash-branches.json | 6 ++++--
7 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index d968609c48..540b3982b1 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -274,7 +274,7 @@ def check_union(expr, info):
for (key, value) in members.items():
source = "'data' member '%s'" % key
if discriminator is None:
- check_name_lower(key, info, source, permit_underscore=True)
+ check_name_lower(key, info, source)
# else: name is in discriminator enum, which gets checked
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
@@ -288,7 +288,7 @@ def check_alternate(expr, info):
raise QAPISemError(info, "'data' must not be empty")
for (key, value) in members.items():
source = "'data' member '%s'" % key
- check_name_lower(key, info, source, permit_underscore=True)
+ check_name_lower(key, info, source)
check_keys(value, info, source, ['type'], ['if'])
check_if(value, info, source)
check_type(value['type'], info, source)
diff --git a/tests/qapi-schema/alternate-clash.err b/tests/qapi-schema/alternate-clash.err
index 0fe02f2c99..caa2d42e3f 100644
--- a/tests/qapi-schema/alternate-clash.err
+++ b/tests/qapi-schema/alternate-clash.err
@@ -1,2 +1,2 @@
alternate-clash.json: In alternate 'Alt1':
-alternate-clash.json:4: branch 'a_b' collides with branch 'a-b'
+alternate-clash.json:6: name of 'data' member 'a_b' must not use uppercase or '_'
diff --git a/tests/qapi-schema/alternate-clash.json b/tests/qapi-schema/alternate-clash.json
index 039c4be658..87f061a74a 100644
--- a/tests/qapi-schema/alternate-clash.json
+++ b/tests/qapi-schema/alternate-clash.json
@@ -1,5 +1,7 @@
# Alternate branch name collision
-# Reject an alternate that would result in a collision in generated C
-# names (this would try to generate two union members named 'a_b').
+# Naming rules make collision impossible (even with the pragma). If
+# that wasn't the case, then we'd get a collision in generated C: two
+# union members a_b.
+{ 'pragma': { 'member-name-exceptions': [ 'Alt1' ] } }
{ 'alternate': 'Alt1',
'data': { 'a-b': 'bool', 'a_b': 'int' } }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 387678acbb..84b9d41f15 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -231,7 +231,7 @@
{ 'union': 'TestIfUnion', 'data':
{ 'foo': 'TestStruct',
- 'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
+ 'bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} },
'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' }
{ 'command': 'test-if-union-cmd',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 51efe5d7cd..e0b8a5f0b6 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -309,14 +309,14 @@ object q_obj_TestStruct-wrapper
member data: TestStruct optional=False
enum TestIfUnionKind
member foo
- member union_bar
+ member bar
if ['defined(TEST_IF_UNION_BAR)']
if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
object TestIfUnion
member type: TestIfUnionKind optional=False
tag type
case foo: q_obj_TestStruct-wrapper
- case union_bar: q_obj_str-wrapper
+ case bar: q_obj_str-wrapper
if ['defined(TEST_IF_UNION_BAR)']
if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)']
object q_obj_test-if-union-cmd-arg
diff --git a/tests/qapi-schema/union-clash-branches.err b/tests/qapi-schema/union-clash-branches.err
index 73bbc2cabd..ef53645728 100644
--- a/tests/qapi-schema/union-clash-branches.err
+++ b/tests/qapi-schema/union-clash-branches.err
@@ -1,2 +1,2 @@
union-clash-branches.json: In union 'TestUnion':
-union-clash-branches.json:4: branch 'a_b' collides with branch 'a-b'
+union-clash-branches.json:6: name of 'data' member 'a_b' must not use uppercase or '_'
diff --git a/tests/qapi-schema/union-clash-branches.json b/tests/qapi-schema/union-clash-branches.json
index 3bece8c948..7bdda0b0da 100644
--- a/tests/qapi-schema/union-clash-branches.json
+++ b/tests/qapi-schema/union-clash-branches.json
@@ -1,5 +1,7 @@
# Union branch name collision
-# Reject a union that would result in a collision in generated C names (this
-# would try to generate two members 'a_b').
+# Naming rules make collision impossible (even with the pragma). If
+# that wasn't the case, then we'd get collisions in generated C: two
+# union members a_b, and two enum members TEST_UNION_A_B.
+{ 'pragma': { 'member-name-exceptions': [ 'TestUnion' ] } }
{ 'union': 'TestUnion',
'data': { 'a-b': 'int', 'a_b': 'str' } }
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* [PULL 29/29] block: Remove monitor command block_passwd
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (27 preceding siblings ...)
2021-03-23 21:56 ` [PULL 28/29] qapi: Enforce union and alternate branch " Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
2021-03-23 23:46 ` [PULL 00/29] QAPI patches patches for 2021-03-23 Peter Maydell
29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, Daniel P . Berrangé
Command block_passwd always fails since
Commit c01c214b69 "block: remove all encryption handling APIs"
(v2.10.0) turned block_passwd into a stub that always fails, and
hardcoded encryption_key_missing to false in query-named-block-nodes
and query-block.
Commit ad1324e044 "block: remove 'encryption_key_missing' flag from
QAPI" just landed. Complete the cleanup job: remove block_passwd.
Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210323101951.3686029-1-armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
qapi/block-core.json | 14 --------------
qapi/pragma.json | 1 -
block/monitor/block-hmp-cmds.c | 10 ----------
blockdev.c | 8 --------
hmp-commands.hx | 15 ---------------
5 files changed, 48 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1c3f1deb03..6d227924d0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1207,20 +1207,6 @@
##
{ 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
-##
-# @block_passwd:
-#
-# This command sets the password of a block device that has not been open
-# with a password and requires one.
-#
-# This command is now obsolete and will always return an error since 2.10
-#
-##
-{ 'command': 'block_passwd',
- 'data': { '*device': 'str',
- '*node-name': 'str',
- 'password': 'str' } }
-
##
# @block_resize:
#
diff --git a/qapi/pragma.json b/qapi/pragma.json
index b4e17167e1..3bc0335d1f 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -6,7 +6,6 @@
# Commands allowed to return a non-dictionary:
'command-name-exceptions': [
'add_client',
- 'block_passwd',
'block_resize',
'block_set_io_throttle',
'client_migrate_info',
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 75d7fa9510..ebf1033f31 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -515,16 +515,6 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, error);
}
-void hmp_block_passwd(Monitor *mon, const QDict *qdict)
-{
- const char *device = qdict_get_str(qdict, "device");
- const char *password = qdict_get_str(qdict, "password");
- Error *err = NULL;
-
- qmp_block_passwd(true, device, false, NULL, password, &err);
- hmp_handle_error(mon, err);
-}
-
void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/blockdev.c b/blockdev.c
index cf70bb4e43..621cc3b7c4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2407,14 +2407,6 @@ exit:
job_txn_unref(block_job_txn);
}
-void qmp_block_passwd(bool has_device, const char *device,
- bool has_node_name, const char *node_name,
- const char *password, Error **errp)
-{
- error_setg(errp,
- "Setting block passwords directly is no longer supported");
-}
-
BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
const char *name,
Error **errp)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9b88c45174..435c591a1c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1493,21 +1493,6 @@ SRST
used by another monitor command.
ERST
- {
- .name = "block_passwd",
- .args_type = "device:B,password:s",
- .params = "block_passwd device password",
- .help = "set the password of encrypted block devices",
- .cmd = hmp_block_passwd,
- },
-
-SRST
-``block_passwd`` *device* *password*
- Set the encrypted device *device* password to *password*
-
- This command is now obsolete and will always return an error since 2.10
-ERST
-
{
.name = "block_set_io_throttle",
.args_type = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
--
2.26.3
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [PULL 00/29] QAPI patches patches for 2021-03-23
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
` (28 preceding siblings ...)
2021-03-23 21:56 ` [PULL 29/29] block: Remove monitor command block_passwd Markus Armbruster
@ 2021-03-23 23:46 ` Peter Maydell
29 siblings, 0 replies; 31+ messages in thread
From: Peter Maydell @ 2021-03-23 23:46 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On Tue, 23 Mar 2021 at 21:57, Markus Armbruster <armbru@redhat.com> wrote:
>
> The following changes since commit 9950da284fa5e2ea9ab57d87e05b693fb60c79ce:
>
> Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20210322-2' into staging (2021-03-23 15:30:46 +0000)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-03-23
>
> for you to fetch changes up to bdabafc6836edc0f34732cac473899cb4e77a296:
>
> block: Remove monitor command block_passwd (2021-03-23 22:31:56 +0100)
>
> ----------------------------------------------------------------
> QAPI patches patches for 2021-03-23
>
> ----------------------------------------------------------------
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 31+ messages in thread