camel_to_upper() converts its argument from camel case to upper case
with '_' between words. Used for generated enumeration constant
prefixes.
When some of the words are spelled all caps, where exactly to insert
'_' is guesswork. camel_to_upper()'s guesses are bad enough in places
to make people override them with a 'prefix' in the schema.
Rewrite it to guess better:
1. Insert '_' after a non-upper case character followed by an upper
case character:
OneTwo -> ONE_TWO
One2Three -> ONE2_THREE
2. Insert '_' before the last upper case character followed by a
non-upper case character:
ACRONYMWord -> ACRONYM_Word
Except at the beginning (as in OneTwo above), or when there is
already one:
AbCd -> AB_CD
This changes the default enumeration constant prefix for a number of
enums. Generated enumeration constants change only where the default
is not overridden with 'prefix'.
The following enumerations without a 'prefix' change:
enum old camel_to_upper()
new camel_to_upper()
------------------------------------------------------------------
DisplayGLMode DISPLAYGL_MODE
DISPLAY_GL_MODE
EbpfProgramID EBPF_PROGRAMID
EBPF_PROGRAM_ID
HmatLBDataType HMATLB_DATA_TYPE
HMAT_LB_DATA_TYPE
HmatLBMemoryHierarchy HMATLB_MEMORY_HIERARCHY
HMAT_LB_MEMORY_HIERARCHY
MultiFDCompression MULTIFD_COMPRESSION
MULTI_FD_COMPRESSION
OffAutoPCIBAR OFF_AUTOPCIBAR
OFF_AUTO_PCIBAR
QCryptoBlockFormat Q_CRYPTO_BLOCK_FORMAT
QCRYPTO_BLOCK_FORMAT
QCryptoBlockLUKSKeyslotState Q_CRYPTO_BLOCKLUKS_KEYSLOT_STATE
QCRYPTO_BLOCK_LUKS_KEYSLOT_STATE
QKeyCode Q_KEY_CODE
QKEY_CODE
XDbgBlockGraphNodeType X_DBG_BLOCK_GRAPH_NODE_TYPE
XDBG_BLOCK_GRAPH_NODE_TYPE
TestUnionEnumA TEST_UNION_ENUMA
TEST_UNION_ENUM_A
Add a 'prefix' so generated code doesn't change now. Subsequent
commits will remove most of them again. Two will remain:
MULTIFD_COMPRESSION, because migration code generally spells "multifd"
that way, and Q_KEY_CODE, because that one is baked into
subprojects/keycodemapdb/tools/keymap-gen.
The following enumerations with a 'prefix' change so that the prefix
is now superfluous:
enum old camel_to_upper()
new camel_to_upper() [equal to prefix]
------------------------------------------------------------------
BlkdebugIOType BLKDEBUGIO_TYPE
BLKDEBUG_IO_TYPE
QCryptoTLSCredsEndpoint Q_CRYPTOTLS_CREDS_ENDPOINT
QCRYPTO_TLS_CREDS_ENDPOINT
QCryptoSecretFormat Q_CRYPTO_SECRET_FORMAT
QCRYPTO_SECRET_FORMAT
QCryptoCipherMode Q_CRYPTO_CIPHER_MODE
QCRYPTO_CIPHER_MODE
QCryptodevBackendType Q_CRYPTODEV_BACKEND_TYPE
QCRYPTODEV_BACKEND_TYPE
QType [builtin] Q_TYPE
QTYPE
Drop these prefixes.
The following enumerations with a 'prefix' change without making the
'prefix' superfluous:
enum old camel_to_upper()
new camel_to_upper() [equal to prefix]
prefix
------------------------------------------------------------------
CpuS390Entitlement CPUS390_ENTITLEMENT
CPU_S390_ENTITLEMENT
S390_CPU_ENTITLEMENT
CpuS390Polarization CPUS390_POLARIZATION
CPU_S390_POLARIZATION
S390_CPU_POLARIZATION
CpuS390State CPUS390_STATE
CPU_S390_STATE
S390_CPU_STATE
QAuthZListFormat Q_AUTHZ_LIST_FORMAT
QAUTH_Z_LIST_FORMAT
QAUTHZ_LIST_FORMAT
QAuthZListPolicy Q_AUTHZ_LIST_POLICY
QAUTH_Z_LIST_POLICY
QAUTHZ_LIST_POLICY
QCryptoAkCipherAlgorithm Q_CRYPTO_AK_CIPHER_ALGORITHM
QCRYPTO_AK_CIPHER_ALGORITHM
QCRYPTO_AKCIPHER_ALG
QCryptoAkCipherKeyType Q_CRYPTO_AK_CIPHER_KEY_TYPE
QCRYPTO_AK_CIPHER_KEY_TYPE
QCRYPTO_AKCIPHER_KEY_TYPE
QCryptoCipherAlgorithm Q_CRYPTO_CIPHER_ALGORITHM
QCRYPTO_CIPHER_ALGORITHM
QCRYPTO_CIPHER_ALG
QCryptoHashAlgorithm Q_CRYPTO_HASH_ALGORITHM
QCRYPTO_HASH_ALGORITHM
QCRYPTO_HASH_ALG
QCryptoIVGenAlgorithm Q_CRYPTOIV_GEN_ALGORITHM
QCRYPTO_IV_GEN_ALGORITHM
QCRYPTO_IVGEN_ALG
QCryptoRSAPaddingAlgorithm Q_CRYPTORSA_PADDING_ALGORITHM
QCRYPTO_RSA_PADDING_ALGORITHM
QCRYPTO_RSA_PADDING_ALG
QCryptodevBackendAlgType Q_CRYPTODEV_BACKEND_ALG_TYPE
QCRYPTODEV_BACKEND_ALG_TYPE
QCRYPTODEV_BACKEND_ALG
QCryptodevBackendServiceType Q_CRYPTODEV_BACKEND_SERVICE_TYPE
QCRYPTODEV_BACKEND_SERVICE_TYPE
QCRYPTODEV_BACKEND_SERVICE
Subsequent commits will tweak things to remove most of these prefixes.
Only QAUTHZ_LIST_FORMAT and QAUTHZ_LIST_POLICY will remain.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/block-core.json | 3 +-
qapi/common.json | 1 +
qapi/crypto.json | 6 ++--
qapi/cryptodev.json | 1 -
qapi/ebpf.json | 1 +
qapi/machine.json | 1 +
qapi/migration.json | 1 +
qapi/ui.json | 2 ++
scripts/qapi/common.py | 42 ++++++++++++++----------
scripts/qapi/schema.py | 2 +-
tests/qapi-schema/alternate-array.out | 1 -
tests/qapi-schema/comments.out | 1 -
tests/qapi-schema/doc-good.out | 1 -
tests/qapi-schema/empty.out | 1 -
tests/qapi-schema/include-repetition.out | 1 -
tests/qapi-schema/include-simple.out | 1 -
tests/qapi-schema/indented-expr.out | 1 -
tests/qapi-schema/qapi-schema-test.json | 1 +
tests/qapi-schema/qapi-schema-test.out | 2 +-
19 files changed, 37 insertions(+), 33 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f400b334c8..897bc7e0e7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2011,6 +2011,7 @@
# Since: 4.0
##
{ 'enum': 'XDbgBlockGraphNodeType',
+ 'prefix': 'X_DBG_BLOCK_GRAPH_NODE_TYPE', # TODO drop
'data': [ 'block-backend', 'block-job', 'block-driver' ] }
##
@@ -3746,7 +3747,7 @@
#
# Since: 4.1
##
-{ 'enum': 'BlkdebugIOType', 'prefix': 'BLKDEBUG_IO_TYPE',
+{ 'enum': 'BlkdebugIOType',
'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush',
'block-status' ] }
diff --git a/qapi/common.json b/qapi/common.json
index 7558ce5430..25726d3113 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -92,6 +92,7 @@
# Since: 2.12
##
{ 'enum': 'OffAutoPCIBAR',
+ 'prefix': 'OFF_AUTOPCIBAR', # TODO drop
'data': [ 'off', 'auto', 'bar0', 'bar1', 'bar2', 'bar3', 'bar4', 'bar5' ] }
##
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 39b191e8a2..e2d77c3fb3 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -20,7 +20,6 @@
# Since: 2.5
##
{ 'enum': 'QCryptoTLSCredsEndpoint',
- 'prefix': 'QCRYPTO_TLS_CREDS_ENDPOINT',
'data': ['client', 'server']}
##
@@ -36,7 +35,6 @@
# Since: 2.6
##
{ 'enum': 'QCryptoSecretFormat',
- 'prefix': 'QCRYPTO_SECRET_FORMAT',
'data': ['raw', 'base64']}
##
@@ -123,7 +121,6 @@
# Since: 2.6
##
{ 'enum': 'QCryptoCipherMode',
- 'prefix': 'QCRYPTO_CIPHER_MODE',
'data': ['ecb', 'cbc', 'xts', 'ctr']}
##
@@ -160,7 +157,7 @@
# Since: 2.6
##
{ 'enum': 'QCryptoBlockFormat',
-# 'prefix': 'QCRYPTO_BLOCK_FORMAT',
+ 'prefix': 'Q_CRYPTO_BLOCK_FORMAT', # TODO drop
'data': ['qcow', 'luks']}
##
@@ -363,6 +360,7 @@
# Since: 5.1
##
{ 'enum': 'QCryptoBlockLUKSKeyslotState',
+ 'prefix': 'Q_CRYPTO_BLOCKLUKS_KEYSLOT_STATE', # TODO drop
'data': [ 'active', 'inactive' ] }
##
diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
index 68289f4984..60f8fe8e4a 100644
--- a/qapi/cryptodev.json
+++ b/qapi/cryptodev.json
@@ -48,7 +48,6 @@
# Since: 8.0
##
{ 'enum': 'QCryptodevBackendType',
- 'prefix': 'QCRYPTODEV_BACKEND_TYPE',
'data': ['builtin', 'vhost-user', 'lkcf']}
##
diff --git a/qapi/ebpf.json b/qapi/ebpf.json
index e500b5a744..166a9d0eb0 100644
--- a/qapi/ebpf.json
+++ b/qapi/ebpf.json
@@ -42,6 +42,7 @@
# Since: 9.0
##
{ 'enum': 'EbpfProgramID',
+ 'prefix': 'EBPF_PROGRAMID', # TODO drop
'if': 'CONFIG_EBPF',
'data': [ { 'name': 'rss' } ] }
diff --git a/qapi/machine.json b/qapi/machine.json
index fcfd249e2d..5514450e12 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -712,6 +712,7 @@
# Since: 5.0
##
{ 'enum': 'HmatLBDataType',
+ 'prefix': 'HMATLB_DATA_TYPE', # TODO drop
'data': [ 'access-latency', 'read-latency', 'write-latency',
'access-bandwidth', 'read-bandwidth', 'write-bandwidth' ] }
diff --git a/qapi/migration.json b/qapi/migration.json
index 073b67c052..1058d69971 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -571,6 +571,7 @@
# Since: 5.0
##
{ 'enum': 'MultiFDCompression',
+ 'prefix': 'MULTIFD_COMPRESSION',
'data': [ 'none', 'zlib',
{ 'name': 'zstd', 'if': 'CONFIG_ZSTD' },
{ 'name': 'qpl', 'if': 'CONFIG_QPL' },
diff --git a/qapi/ui.json b/qapi/ui.json
index 5daca5168c..31c42821f6 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -948,6 +948,7 @@
# Since: 1.3
##
{ 'enum': 'QKeyCode',
+ 'prefix': 'Q_KEY_CODE',
'data': [ 'unmapped',
'shift', 'shift_r', 'alt', 'alt_r', 'ctrl',
'ctrl_r', 'menu', 'esc', '1', '2', '3', '4', '5', '6', '7', '8',
@@ -1395,6 +1396,7 @@
# Since: 3.0
##
{ 'enum' : 'DisplayGLMode',
+ 'prefix' : 'DISPLAYGL_MODE', # TODO drop
'data' : [ 'off', 'on', 'core', 'es' ] }
##
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 737b059e62..7081dcd943 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -40,22 +40,28 @@ def camel_to_upper(value: str) -> str:
ENUM_Name2 -> ENUM_NAME2
ENUM24_Name -> ENUM24_NAME
"""
- c_fun_str = c_name(value, False)
- if value.isupper():
- return c_fun_str
+ ret = value[0]
+ upc = value[0].isupper()
- new_name = ''
- length = len(c_fun_str)
- for i in range(length):
- char = c_fun_str[i]
- # When char is upper case and no '_' appears before, do more checks
- if char.isupper() and (i > 0) and c_fun_str[i - 1] != '_':
- if i < length - 1 and c_fun_str[i + 1].islower():
- new_name += '_'
- elif c_fun_str[i - 1].isdigit():
- new_name += '_'
- new_name += char
- return new_name.lstrip('_').upper()
+ # Copy remainder of ``value`` to ``ret`` with '_' inserted
+ for ch in value[1:]:
+ if ch.isupper() == upc:
+ pass
+ elif upc:
+ # ``ret`` ends in upper case, next char isn't: insert '_'
+ # before the last upper case char unless there is one
+ # already, or it's at the beginning
+ if len(ret) > 2 and ret[-2] != '_':
+ ret = ret[:-1] + '_' + ret[-1]
+ else:
+ # ``ret`` doesn't end in upper case, next char is: insert
+ # '_' before it
+ if ret[-1] != '_':
+ ret += '_'
+ ret += ch
+ upc = ch.isupper()
+
+ return c_name(ret.upper())
def c_enum_const(type_name: str,
@@ -68,9 +74,9 @@ def c_enum_const(type_name: str,
:param const_name: The name of this constant.
:param prefix: Optional, prefix that overrides the type_name.
"""
- if prefix is not None:
- type_name = prefix
- return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper()
+ if prefix is None:
+ prefix = camel_to_upper(type_name)
+ return prefix + '_' + c_name(const_name, False).upper()
def c_name(name: str, protect: bool = True) -> str:
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index d65c35f6ee..e97c978d38 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -1249,7 +1249,7 @@ def _def_predefineds(self) -> None:
[{'name': n} for n in qtypes], None)
self._def_definition(QAPISchemaEnumType(
- 'QType', None, None, None, None, qtype_values, 'QTYPE'))
+ 'QType', None, None, None, None, qtype_values, None))
def _make_features(
self,
diff --git a/tests/qapi-schema/alternate-array.out b/tests/qapi-schema/alternate-array.out
index a657d85738..2f30973ac3 100644
--- a/tests/qapi-schema/alternate-array.out
+++ b/tests/qapi-schema/alternate-array.out
@@ -1,7 +1,6 @@
module ./builtin
object q_empty
enum QType
- prefix QTYPE
member none
member qnull
member qnum
diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index ce4f6a4f0f..937070c2c4 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,7 +1,6 @@
module ./builtin
object q_empty
enum QType
- prefix QTYPE
member none
member qnull
member qnum
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 6d24f1127b..ec277be91e 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -1,7 +1,6 @@
module ./builtin
object q_empty
enum QType
- prefix QTYPE
member none
member qnull
member qnum
diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
index 3feb3f69d3..d1981f8586 100644
--- a/tests/qapi-schema/empty.out
+++ b/tests/qapi-schema/empty.out
@@ -1,7 +1,6 @@
module ./builtin
object q_empty
enum QType
- prefix QTYPE
member none
member qnull
member qnum
diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
index 16dbd9b819..c564d27862 100644
--- a/tests/qapi-schema/include-repetition.out
+++ b/tests/qapi-schema/include-repetition.out
@@ -1,7 +1,6 @@
module ./builtin
object q_empty
enum QType
- prefix QTYPE
member none
member qnull
member qnum
diff --git a/tests/qapi-schema/include-simple.out b/tests/qapi-schema/include-simple.out
index 48e923bfbc..ec8200ab18 100644
--- a/tests/qapi-schema/include-simple.out
+++ b/tests/qapi-schema/include-simple.out
@@ -1,7 +1,6 @@
module ./builtin
object q_empty
enum QType
- prefix QTYPE
member none
member qnull
member qnum
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index 6a30ded3fa..a7c22c3eef 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -1,7 +1,6 @@
module ./builtin
object q_empty
enum QType
- prefix QTYPE
member none
member qnull
member qnum
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 8ca977c49d..0f5f54e621 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -119,6 +119,7 @@
'data': [ 'value-a', 'value-b' ] }
{ 'enum': 'TestUnionEnumA',
+ 'prefix': 'TEST_UNION_ENUMA', # TODO drop
'data': [ 'value-a1', 'value-a2' ] }
{ 'struct': 'TestUnionTypeA1',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index e2f0981348..add7346f49 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -1,7 +1,6 @@
module ./builtin
object q_empty
enum QType
- prefix QTYPE
member none
member qnull
member qnum
@@ -109,6 +108,7 @@ enum TestUnionEnum
member value-a
member value-b
enum TestUnionEnumA
+ prefix TEST_UNION_ENUMA
member value-a1
member value-a2
object TestUnionTypeA1
--
2.45.0Am 30.07.2024 um 10:10 hat Markus Armbruster geschrieben: > camel_to_upper() converts its argument from camel case to upper case > with '_' between words. Used for generated enumeration constant > prefixes. > > When some of the words are spelled all caps, where exactly to insert > '_' is guesswork. camel_to_upper()'s guesses are bad enough in places > to make people override them with a 'prefix' in the schema. > > Rewrite it to guess better: > > 1. Insert '_' after a non-upper case character followed by an upper > case character: > > OneTwo -> ONE_TWO > One2Three -> ONE2_THREE > > 2. Insert '_' before the last upper case character followed by a > non-upper case character: > > ACRONYMWord -> ACRONYM_Word > > Except at the beginning (as in OneTwo above), or when there is > already one: > > AbCd -> AB_CD Maybe it's just me, but the exception "at the beginning" (in the sense of "after the first character") seems to be exactly where I thought "that looks strange" while going through your list below. In particular, I'd expect X_DBG_* instead of XDBG_*. I also thought that the Q_* spelling made more sense, though this might be less clear. But in case of doubt, less exceptions seems like a good choice. > + # Copy remainder of ``value`` to ``ret`` with '_' inserted > + for ch in value[1:]: > + if ch.isupper() == upc: > + pass > + elif upc: > + # ``ret`` ends in upper case, next char isn't: insert '_' > + # before the last upper case char unless there is one > + # already, or it's at the beginning > + if len(ret) > 2 and ret[-2] != '_': > + ret = ret[:-1] + '_' + ret[-1] I think in the code this means I would have expected len(ret) >= 2. Kevin
Kevin Wolf <kwolf@redhat.com> writes: > Am 30.07.2024 um 10:10 hat Markus Armbruster geschrieben: >> camel_to_upper() converts its argument from camel case to upper case >> with '_' between words. Used for generated enumeration constant >> prefixes. >> >> When some of the words are spelled all caps, where exactly to insert >> '_' is guesswork. camel_to_upper()'s guesses are bad enough in places >> to make people override them with a 'prefix' in the schema. >> >> Rewrite it to guess better: >> >> 1. Insert '_' after a non-upper case character followed by an upper >> case character: >> >> OneTwo -> ONE_TWO >> One2Three -> ONE2_THREE >> >> 2. Insert '_' before the last upper case character followed by a >> non-upper case character: >> >> ACRONYMWord -> ACRONYM_Word >> >> Except at the beginning (as in OneTwo above), or when there is >> already one: >> >> AbCd -> AB_CD > > Maybe it's just me, but the exception "at the beginning" (in the sense > of "after the first character") seems to be exactly where I thought > "that looks strange" while going through your list below. By "except at the beginning", I mean don't map "One" to "_ONE". > In particular, > I'd expect X_DBG_* instead of XDBG_*. What's the intent of the X in the XDbgFOO types? Signify unstable? If yes: we don't do that elsewhere. Type names are not part of the external interface. We never used an X prefix for names of unstable types. We use an x- prefix for names of unstable commands, arguments and members, but even that is optional today. Feature flag @unstable is the source of truth. The XDbgFOO appear to be used just by x-debug-query-block-graph, which has feature @unstable. If the XDBG name bothers you, we can strip the X prefix from the type names. Happy to do that in this series. > I also thought that the Q_* > spelling made more sense, though this might be less clear. The crypto subsystem spells its prefix qcrypto_, QCRYPTO_, and QCrypto. Before this series, it forces QAPI to generate QCRYPTO_ with 'prefix' with two exceptions, probably oversights. > But in case > of doubt, less exceptions seems like a good choice. Agree. I want to be able to predict generated names :) >> + # Copy remainder of ``value`` to ``ret`` with '_' inserted >> + for ch in value[1:]: >> + if ch.isupper() == upc: >> + pass >> + elif upc: >> + # ``ret`` ends in upper case, next char isn't: insert '_' >> + # before the last upper case char unless there is one >> + # already, or it's at the beginning >> + if len(ret) > 2 and ret[-2] != '_': >> + ret = ret[:-1] + '_' + ret[-1] > > I think in the code this means I would have expected len(ret) >= 2. I'm not sure I understand what you mean. With len(ret) > 2, we map "QType" to "QTYPE". With len(ret) >= 2, we'd map it to "Q_TYPE".
On Tue, Jul 30, 2024 at 10:10:15AM +0200, Markus Armbruster wrote: > camel_to_upper() converts its argument from camel case to upper case > with '_' between words. Used for generated enumeration constant > prefixes. > > When some of the words are spelled all caps, where exactly to insert > '_' is guesswork. camel_to_upper()'s guesses are bad enough in places > to make people override them with a 'prefix' in the schema. > > Rewrite it to guess better: > > 1. Insert '_' after a non-upper case character followed by an upper > case character: > > OneTwo -> ONE_TWO > One2Three -> ONE2_THREE > > 2. Insert '_' before the last upper case character followed by a > non-upper case character: > > ACRONYMWord -> ACRONYM_Word > > Except at the beginning (as in OneTwo above), or when there is > already one: > > AbCd -> AB_CD > > This changes the default enumeration constant prefix for a number of > enums. Generated enumeration constants change only where the default > is not overridden with 'prefix'. > > The following enumerations without a 'prefix' change: > > enum old camel_to_upper() > new camel_to_upper() > ------------------------------------------------------------------ > DisplayGLMode DISPLAYGL_MODE > DISPLAY_GL_MODE > EbpfProgramID EBPF_PROGRAMID > EBPF_PROGRAM_ID > HmatLBDataType HMATLB_DATA_TYPE > HMAT_LB_DATA_TYPE > HmatLBMemoryHierarchy HMATLB_MEMORY_HIERARCHY > HMAT_LB_MEMORY_HIERARCHY > MultiFDCompression MULTIFD_COMPRESSION > MULTI_FD_COMPRESSION > OffAutoPCIBAR OFF_AUTOPCIBAR > OFF_AUTO_PCIBAR > QCryptoBlockFormat Q_CRYPTO_BLOCK_FORMAT > QCRYPTO_BLOCK_FORMAT > QCryptoBlockLUKSKeyslotState Q_CRYPTO_BLOCKLUKS_KEYSLOT_STATE > QCRYPTO_BLOCK_LUKS_KEYSLOT_STATE > QKeyCode Q_KEY_CODE > QKEY_CODE > XDbgBlockGraphNodeType X_DBG_BLOCK_GRAPH_NODE_TYPE > XDBG_BLOCK_GRAPH_NODE_TYPE > TestUnionEnumA TEST_UNION_ENUMA > TEST_UNION_ENUM_A > > Add a 'prefix' so generated code doesn't change now. Subsequent > commits will remove most of them again. Two will remain: > MULTIFD_COMPRESSION, because migration code generally spells "multifd" > that way, and Q_KEY_CODE, because that one is baked into > subprojects/keycodemapdb/tools/keymap-gen. > > The following enumerations with a 'prefix' change so that the prefix > is now superfluous: > > enum old camel_to_upper() > new camel_to_upper() [equal to prefix] > ------------------------------------------------------------------ > BlkdebugIOType BLKDEBUGIO_TYPE > BLKDEBUG_IO_TYPE > QCryptoTLSCredsEndpoint Q_CRYPTOTLS_CREDS_ENDPOINT > QCRYPTO_TLS_CREDS_ENDPOINT > QCryptoSecretFormat Q_CRYPTO_SECRET_FORMAT > QCRYPTO_SECRET_FORMAT > QCryptoCipherMode Q_CRYPTO_CIPHER_MODE > QCRYPTO_CIPHER_MODE > QCryptodevBackendType Q_CRYPTODEV_BACKEND_TYPE > QCRYPTODEV_BACKEND_TYPE > QType [builtin] Q_TYPE > QTYPE > > Drop these prefixes. > > The following enumerations with a 'prefix' change without making the > 'prefix' superfluous: > > enum old camel_to_upper() > new camel_to_upper() [equal to prefix] > prefix > ------------------------------------------------------------------ > CpuS390Entitlement CPUS390_ENTITLEMENT > CPU_S390_ENTITLEMENT > S390_CPU_ENTITLEMENT > CpuS390Polarization CPUS390_POLARIZATION > CPU_S390_POLARIZATION > S390_CPU_POLARIZATION > CpuS390State CPUS390_STATE > CPU_S390_STATE > S390_CPU_STATE > QAuthZListFormat Q_AUTHZ_LIST_FORMAT > QAUTH_Z_LIST_FORMAT > QAUTHZ_LIST_FORMAT > QAuthZListPolicy Q_AUTHZ_LIST_POLICY > QAUTH_Z_LIST_POLICY > QAUTHZ_LIST_POLICY > QCryptoAkCipherAlgorithm Q_CRYPTO_AK_CIPHER_ALGORITHM > QCRYPTO_AK_CIPHER_ALGORITHM > QCRYPTO_AKCIPHER_ALG > QCryptoAkCipherKeyType Q_CRYPTO_AK_CIPHER_KEY_TYPE > QCRYPTO_AK_CIPHER_KEY_TYPE > QCRYPTO_AKCIPHER_KEY_TYPE > QCryptoCipherAlgorithm Q_CRYPTO_CIPHER_ALGORITHM > QCRYPTO_CIPHER_ALGORITHM > QCRYPTO_CIPHER_ALG > QCryptoHashAlgorithm Q_CRYPTO_HASH_ALGORITHM > QCRYPTO_HASH_ALGORITHM > QCRYPTO_HASH_ALG > QCryptoIVGenAlgorithm Q_CRYPTOIV_GEN_ALGORITHM > QCRYPTO_IV_GEN_ALGORITHM > QCRYPTO_IVGEN_ALG > QCryptoRSAPaddingAlgorithm Q_CRYPTORSA_PADDING_ALGORITHM > QCRYPTO_RSA_PADDING_ALGORITHM > QCRYPTO_RSA_PADDING_ALG > QCryptodevBackendAlgType Q_CRYPTODEV_BACKEND_ALG_TYPE > QCRYPTODEV_BACKEND_ALG_TYPE > QCRYPTODEV_BACKEND_ALG > QCryptodevBackendServiceType Q_CRYPTODEV_BACKEND_SERVICE_TYPE > QCRYPTODEV_BACKEND_SERVICE_TYPE > QCRYPTODEV_BACKEND_SERVICE > > Subsequent commits will tweak things to remove most of these prefixes. > Only QAUTHZ_LIST_FORMAT and QAUTHZ_LIST_POLICY will remain. IIUC from above those two result in QAUTH_Z_LIST_FORMAT QAUTH_Z_LIST_POLICY Is it possible to add a 3rd rule * Single uppercase letter folds into the previous word or are there valid cases where we have a single uppercase that we want to preserve ? It sure would be nice to eliminate the 'prefix' concept, that we've clearly over-used, if we can kill the only 2 remaining examples. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > qapi/block-core.json | 3 +- > qapi/common.json | 1 + > qapi/crypto.json | 6 ++-- > qapi/cryptodev.json | 1 - > qapi/ebpf.json | 1 + > qapi/machine.json | 1 + > qapi/migration.json | 1 + > qapi/ui.json | 2 ++ > scripts/qapi/common.py | 42 ++++++++++++++---------- > scripts/qapi/schema.py | 2 +- > tests/qapi-schema/alternate-array.out | 1 - > tests/qapi-schema/comments.out | 1 - > tests/qapi-schema/doc-good.out | 1 - > tests/qapi-schema/empty.out | 1 - > tests/qapi-schema/include-repetition.out | 1 - > tests/qapi-schema/include-simple.out | 1 - > tests/qapi-schema/indented-expr.out | 1 - > tests/qapi-schema/qapi-schema-test.json | 1 + > tests/qapi-schema/qapi-schema-test.out | 2 +- > 19 files changed, 37 insertions(+), 33 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Avihai, there's a question for you on VfioMigrationState.
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Tue, Jul 30, 2024 at 10:10:15AM +0200, Markus Armbruster wrote:
>> camel_to_upper() converts its argument from camel case to upper case
>> with '_' between words. Used for generated enumeration constant
>> prefixes.
>>
>> When some of the words are spelled all caps, where exactly to insert
>> '_' is guesswork. camel_to_upper()'s guesses are bad enough in places
>> to make people override them with a 'prefix' in the schema.
>>
>> Rewrite it to guess better:
>>
>> 1. Insert '_' after a non-upper case character followed by an upper
>> case character:
>>
>> OneTwo -> ONE_TWO
>> One2Three -> ONE2_THREE
>>
>> 2. Insert '_' before the last upper case character followed by a
>> non-upper case character:
>>
>> ACRONYMWord -> ACRONYM_Word
>>
>> Except at the beginning (as in OneTwo above), or when there is
>> already one:
>>
>> AbCd -> AB_CD
>>
>> This changes the default enumeration constant prefix for a number of
>> enums. Generated enumeration constants change only where the default
>> is not overridden with 'prefix'.
>>
>> The following enumerations without a 'prefix' change:
>>
>> enum old camel_to_upper()
>> new camel_to_upper()
>> ------------------------------------------------------------------
>> DisplayGLMode DISPLAYGL_MODE
>> DISPLAY_GL_MODE
>> EbpfProgramID EBPF_PROGRAMID
>> EBPF_PROGRAM_ID
>> HmatLBDataType HMATLB_DATA_TYPE
>> HMAT_LB_DATA_TYPE
>> HmatLBMemoryHierarchy HMATLB_MEMORY_HIERARCHY
>> HMAT_LB_MEMORY_HIERARCHY
>> MultiFDCompression MULTIFD_COMPRESSION
>> MULTI_FD_COMPRESSION
>> OffAutoPCIBAR OFF_AUTOPCIBAR
>> OFF_AUTO_PCIBAR
>> QCryptoBlockFormat Q_CRYPTO_BLOCK_FORMAT
>> QCRYPTO_BLOCK_FORMAT
>> QCryptoBlockLUKSKeyslotState Q_CRYPTO_BLOCKLUKS_KEYSLOT_STATE
>> QCRYPTO_BLOCK_LUKS_KEYSLOT_STATE
>> QKeyCode Q_KEY_CODE
>> QKEY_CODE
>> XDbgBlockGraphNodeType X_DBG_BLOCK_GRAPH_NODE_TYPE
>> XDBG_BLOCK_GRAPH_NODE_TYPE
>> TestUnionEnumA TEST_UNION_ENUMA
>> TEST_UNION_ENUM_A
>>
>> Add a 'prefix' so generated code doesn't change now. Subsequent
>> commits will remove most of them again. Two will remain:
>> MULTIFD_COMPRESSION, because migration code generally spells "multifd"
>> that way, and Q_KEY_CODE, because that one is baked into
>> subprojects/keycodemapdb/tools/keymap-gen.
>>
>> The following enumerations with a 'prefix' change so that the prefix
>> is now superfluous:
>>
>> enum old camel_to_upper()
>> new camel_to_upper() [equal to prefix]
>> ------------------------------------------------------------------
>> BlkdebugIOType BLKDEBUGIO_TYPE
>> BLKDEBUG_IO_TYPE
>> QCryptoTLSCredsEndpoint Q_CRYPTOTLS_CREDS_ENDPOINT
>> QCRYPTO_TLS_CREDS_ENDPOINT
>> QCryptoSecretFormat Q_CRYPTO_SECRET_FORMAT
>> QCRYPTO_SECRET_FORMAT
>> QCryptoCipherMode Q_CRYPTO_CIPHER_MODE
>> QCRYPTO_CIPHER_MODE
>> QCryptodevBackendType Q_CRYPTODEV_BACKEND_TYPE
>> QCRYPTODEV_BACKEND_TYPE
>> QType [builtin] Q_TYPE
>> QTYPE
>>
>> Drop these prefixes.
>>
>> The following enumerations with a 'prefix' change without making the
>> 'prefix' superfluous:
>>
>> enum old camel_to_upper()
>> new camel_to_upper() [equal to prefix]
>> prefix
>> ------------------------------------------------------------------
>> CpuS390Entitlement CPUS390_ENTITLEMENT
>> CPU_S390_ENTITLEMENT
>> S390_CPU_ENTITLEMENT
>> CpuS390Polarization CPUS390_POLARIZATION
>> CPU_S390_POLARIZATION
>> S390_CPU_POLARIZATION
>> CpuS390State CPUS390_STATE
>> CPU_S390_STATE
>> S390_CPU_STATE
>> QAuthZListFormat Q_AUTHZ_LIST_FORMAT
>> QAUTH_Z_LIST_FORMAT
>> QAUTHZ_LIST_FORMAT
>> QAuthZListPolicy Q_AUTHZ_LIST_POLICY
>> QAUTH_Z_LIST_POLICY
>> QAUTHZ_LIST_POLICY
>> QCryptoAkCipherAlgorithm Q_CRYPTO_AK_CIPHER_ALGORITHM
>> QCRYPTO_AK_CIPHER_ALGORITHM
>> QCRYPTO_AKCIPHER_ALG
>> QCryptoAkCipherKeyType Q_CRYPTO_AK_CIPHER_KEY_TYPE
>> QCRYPTO_AK_CIPHER_KEY_TYPE
>> QCRYPTO_AKCIPHER_KEY_TYPE
>> QCryptoCipherAlgorithm Q_CRYPTO_CIPHER_ALGORITHM
>> QCRYPTO_CIPHER_ALGORITHM
>> QCRYPTO_CIPHER_ALG
>> QCryptoHashAlgorithm Q_CRYPTO_HASH_ALGORITHM
>> QCRYPTO_HASH_ALGORITHM
>> QCRYPTO_HASH_ALG
>> QCryptoIVGenAlgorithm Q_CRYPTOIV_GEN_ALGORITHM
>> QCRYPTO_IV_GEN_ALGORITHM
>> QCRYPTO_IVGEN_ALG
>> QCryptoRSAPaddingAlgorithm Q_CRYPTORSA_PADDING_ALGORITHM
>> QCRYPTO_RSA_PADDING_ALGORITHM
>> QCRYPTO_RSA_PADDING_ALG
>> QCryptodevBackendAlgType Q_CRYPTODEV_BACKEND_ALG_TYPE
>> QCRYPTODEV_BACKEND_ALG_TYPE
>> QCRYPTODEV_BACKEND_ALG
>> QCryptodevBackendServiceType Q_CRYPTODEV_BACKEND_SERVICE_TYPE
>> QCRYPTODEV_BACKEND_SERVICE_TYPE
>> QCRYPTODEV_BACKEND_SERVICE
>>
>> Subsequent commits will tweak things to remove most of these prefixes.
>> Only QAUTHZ_LIST_FORMAT and QAUTHZ_LIST_POLICY will remain.
>
> IIUC from above those two result in
>
> QAUTH_Z_LIST_FORMAT
> QAUTH_Z_LIST_POLICY
>
> Is it possible to add a 3rd rule
>
> * Single uppercase letter folds into the previous word
I guess we could.
> or are there valid cases where we have a single uppercase
> that we want to preserve ?
Not now, but I'd prefer to leave predictions to economists.
> It sure would be nice to eliminate the 'prefix' concept,
> that we've clearly over-used, if we can kill the only 2
> remaining examples.
There are a few more, actually. After this series and outside tests:
enum default prefix camel_to_upper()
prefix override
------------------------------------------------------------------
BlkdebugEvent BLKDEBUG_EVENT
BLKDBG
IscsiHeaderDigest ISCSI_HEADER_DIGEST
QAPI_ISCSI_HEADER_DIGEST
MultiFDCompression MULTI_FD_COMPRESSION
MULTIFD_COMPRESSION
QAuthZListFormat QAUTH_Z_LIST_FORMAT
QAUTHZ_LIST_FORMAT
QAuthZListPolicy QAUTH_Z_LIST_POLICY
QAUTHZ_LIST_POLICY
QKeyCode QKEY_CODE
Q_KEY_CODE
VfioMigrationState VFIO_MIGRATION_STATE
QAPI_VFIO_MIGRATION_STATE
Reasons for 'prefix', and what could be done instead of 'prefix':
* BlkdebugEvent: shorten the prefix.
Could live with the longer names instead. Some 90 occurences...
* IscsiHeaderDigest
QAPI version of enum iscsi_header_digest from libiscsi's
iscsi/iscsi.h. We use 'prefix' to avoid name clashes.
Could rename the type to QapiIscsiHeaderDigest instead.
* MultiFDCompression
Migration code consistently uses prefixes multifd_, MULTIFD_, and
MultiFD_.
Could rename the type to MultifdCompression instead, but that just
moves the inconsistency to the type name.
* QAuthZListFormat and QAuthZListPolicy
The authz code consistently uses QAuthZ.
Could make camel_to_upper() avoid the lone Z instead (and hope that'll
remain what we want).
* QKeyCode
Q_KEY_CODE is baked into subprojects/keycodemapdb/tools/keymap-gen.
Could adjust the subproject instead.
* VfioMigrationState
Can't see why this one has a prefix. Avihai, can you enlighten me?
Daniel, thoughts?
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi/block-core.json | 3 +-
>> qapi/common.json | 1 +
>> qapi/crypto.json | 6 ++--
>> qapi/cryptodev.json | 1 -
>> qapi/ebpf.json | 1 +
>> qapi/machine.json | 1 +
>> qapi/migration.json | 1 +
>> qapi/ui.json | 2 ++
>> scripts/qapi/common.py | 42 ++++++++++++++----------
>> scripts/qapi/schema.py | 2 +-
>> tests/qapi-schema/alternate-array.out | 1 -
>> tests/qapi-schema/comments.out | 1 -
>> tests/qapi-schema/doc-good.out | 1 -
>> tests/qapi-schema/empty.out | 1 -
>> tests/qapi-schema/include-repetition.out | 1 -
>> tests/qapi-schema/include-simple.out | 1 -
>> tests/qapi-schema/indented-expr.out | 1 -
>> tests/qapi-schema/qapi-schema-test.json | 1 +
>> tests/qapi-schema/qapi-schema-test.out | 2 +-
>> 19 files changed, 37 insertions(+), 33 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Thanks!
On 30/07/2024 15:22, Markus Armbruster wrote: > External email: Use caution opening links or attachments > > > Avihai, there's a question for you on VfioMigrationState. > > Daniel P. Berrangé <berrange@redhat.com> writes: > >> On Tue, Jul 30, 2024 at 10:10:15AM +0200, Markus Armbruster wrote: >>> camel_to_upper() converts its argument from camel case to upper case >>> with '_' between words. Used for generated enumeration constant >>> prefixes. >>> >>> When some of the words are spelled all caps, where exactly to insert >>> '_' is guesswork. camel_to_upper()'s guesses are bad enough in places >>> to make people override them with a 'prefix' in the schema. >>> >>> Rewrite it to guess better: >>> >>> 1. Insert '_' after a non-upper case character followed by an upper >>> case character: >>> >>> OneTwo -> ONE_TWO >>> One2Three -> ONE2_THREE >>> >>> 2. Insert '_' before the last upper case character followed by a >>> non-upper case character: >>> >>> ACRONYMWord -> ACRONYM_Word >>> >>> Except at the beginning (as in OneTwo above), or when there is >>> already one: >>> >>> AbCd -> AB_CD >>> >>> This changes the default enumeration constant prefix for a number of >>> enums. Generated enumeration constants change only where the default >>> is not overridden with 'prefix'. >>> >>> The following enumerations without a 'prefix' change: >>> >>> enum old camel_to_upper() >>> new camel_to_upper() >>> ------------------------------------------------------------------ >>> DisplayGLMode DISPLAYGL_MODE >>> DISPLAY_GL_MODE >>> EbpfProgramID EBPF_PROGRAMID >>> EBPF_PROGRAM_ID >>> HmatLBDataType HMATLB_DATA_TYPE >>> HMAT_LB_DATA_TYPE >>> HmatLBMemoryHierarchy HMATLB_MEMORY_HIERARCHY >>> HMAT_LB_MEMORY_HIERARCHY >>> MultiFDCompression MULTIFD_COMPRESSION >>> MULTI_FD_COMPRESSION >>> OffAutoPCIBAR OFF_AUTOPCIBAR >>> OFF_AUTO_PCIBAR >>> QCryptoBlockFormat Q_CRYPTO_BLOCK_FORMAT >>> QCRYPTO_BLOCK_FORMAT >>> QCryptoBlockLUKSKeyslotState Q_CRYPTO_BLOCKLUKS_KEYSLOT_STATE >>> QCRYPTO_BLOCK_LUKS_KEYSLOT_STATE >>> QKeyCode Q_KEY_CODE >>> QKEY_CODE >>> XDbgBlockGraphNodeType X_DBG_BLOCK_GRAPH_NODE_TYPE >>> XDBG_BLOCK_GRAPH_NODE_TYPE >>> TestUnionEnumA TEST_UNION_ENUMA >>> TEST_UNION_ENUM_A >>> >>> Add a 'prefix' so generated code doesn't change now. Subsequent >>> commits will remove most of them again. Two will remain: >>> MULTIFD_COMPRESSION, because migration code generally spells "multifd" >>> that way, and Q_KEY_CODE, because that one is baked into >>> subprojects/keycodemapdb/tools/keymap-gen. >>> >>> The following enumerations with a 'prefix' change so that the prefix >>> is now superfluous: >>> >>> enum old camel_to_upper() >>> new camel_to_upper() [equal to prefix] >>> ------------------------------------------------------------------ >>> BlkdebugIOType BLKDEBUGIO_TYPE >>> BLKDEBUG_IO_TYPE >>> QCryptoTLSCredsEndpoint Q_CRYPTOTLS_CREDS_ENDPOINT >>> QCRYPTO_TLS_CREDS_ENDPOINT >>> QCryptoSecretFormat Q_CRYPTO_SECRET_FORMAT >>> QCRYPTO_SECRET_FORMAT >>> QCryptoCipherMode Q_CRYPTO_CIPHER_MODE >>> QCRYPTO_CIPHER_MODE >>> QCryptodevBackendType Q_CRYPTODEV_BACKEND_TYPE >>> QCRYPTODEV_BACKEND_TYPE >>> QType [builtin] Q_TYPE >>> QTYPE >>> >>> Drop these prefixes. >>> >>> The following enumerations with a 'prefix' change without making the >>> 'prefix' superfluous: >>> >>> enum old camel_to_upper() >>> new camel_to_upper() [equal to prefix] >>> prefix >>> ------------------------------------------------------------------ >>> CpuS390Entitlement CPUS390_ENTITLEMENT >>> CPU_S390_ENTITLEMENT >>> S390_CPU_ENTITLEMENT >>> CpuS390Polarization CPUS390_POLARIZATION >>> CPU_S390_POLARIZATION >>> S390_CPU_POLARIZATION >>> CpuS390State CPUS390_STATE >>> CPU_S390_STATE >>> S390_CPU_STATE >>> QAuthZListFormat Q_AUTHZ_LIST_FORMAT >>> QAUTH_Z_LIST_FORMAT >>> QAUTHZ_LIST_FORMAT >>> QAuthZListPolicy Q_AUTHZ_LIST_POLICY >>> QAUTH_Z_LIST_POLICY >>> QAUTHZ_LIST_POLICY >>> QCryptoAkCipherAlgorithm Q_CRYPTO_AK_CIPHER_ALGORITHM >>> QCRYPTO_AK_CIPHER_ALGORITHM >>> QCRYPTO_AKCIPHER_ALG >>> QCryptoAkCipherKeyType Q_CRYPTO_AK_CIPHER_KEY_TYPE >>> QCRYPTO_AK_CIPHER_KEY_TYPE >>> QCRYPTO_AKCIPHER_KEY_TYPE >>> QCryptoCipherAlgorithm Q_CRYPTO_CIPHER_ALGORITHM >>> QCRYPTO_CIPHER_ALGORITHM >>> QCRYPTO_CIPHER_ALG >>> QCryptoHashAlgorithm Q_CRYPTO_HASH_ALGORITHM >>> QCRYPTO_HASH_ALGORITHM >>> QCRYPTO_HASH_ALG >>> QCryptoIVGenAlgorithm Q_CRYPTOIV_GEN_ALGORITHM >>> QCRYPTO_IV_GEN_ALGORITHM >>> QCRYPTO_IVGEN_ALG >>> QCryptoRSAPaddingAlgorithm Q_CRYPTORSA_PADDING_ALGORITHM >>> QCRYPTO_RSA_PADDING_ALGORITHM >>> QCRYPTO_RSA_PADDING_ALG >>> QCryptodevBackendAlgType Q_CRYPTODEV_BACKEND_ALG_TYPE >>> QCRYPTODEV_BACKEND_ALG_TYPE >>> QCRYPTODEV_BACKEND_ALG >>> QCryptodevBackendServiceType Q_CRYPTODEV_BACKEND_SERVICE_TYPE >>> QCRYPTODEV_BACKEND_SERVICE_TYPE >>> QCRYPTODEV_BACKEND_SERVICE >>> >>> Subsequent commits will tweak things to remove most of these prefixes. >>> Only QAUTHZ_LIST_FORMAT and QAUTHZ_LIST_POLICY will remain. >> IIUC from above those two result in >> >> QAUTH_Z_LIST_FORMAT >> QAUTH_Z_LIST_POLICY >> >> Is it possible to add a 3rd rule >> >> * Single uppercase letter folds into the previous word > I guess we could. > >> or are there valid cases where we have a single uppercase >> that we want to preserve ? > Not now, but I'd prefer to leave predictions to economists. > >> It sure would be nice to eliminate the 'prefix' concept, >> that we've clearly over-used, if we can kill the only 2 >> remaining examples. > There are a few more, actually. After this series and outside tests: > > enum default prefix camel_to_upper() > prefix override > ------------------------------------------------------------------ > BlkdebugEvent BLKDEBUG_EVENT > BLKDBG > IscsiHeaderDigest ISCSI_HEADER_DIGEST > QAPI_ISCSI_HEADER_DIGEST > MultiFDCompression MULTI_FD_COMPRESSION > MULTIFD_COMPRESSION > QAuthZListFormat QAUTH_Z_LIST_FORMAT > QAUTHZ_LIST_FORMAT > QAuthZListPolicy QAUTH_Z_LIST_POLICY > QAUTHZ_LIST_POLICY > QKeyCode QKEY_CODE > Q_KEY_CODE > VfioMigrationState VFIO_MIGRATION_STATE > QAPI_VFIO_MIGRATION_STATE > > Reasons for 'prefix', and what could be done instead of 'prefix': > > * BlkdebugEvent: shorten the prefix. > > Could live with the longer names instead. Some 90 occurences... > > * IscsiHeaderDigest > > QAPI version of enum iscsi_header_digest from libiscsi's > iscsi/iscsi.h. We use 'prefix' to avoid name clashes. > > Could rename the type to QapiIscsiHeaderDigest instead. > > * MultiFDCompression > > Migration code consistently uses prefixes multifd_, MULTIFD_, and > MultiFD_. > > Could rename the type to MultifdCompression instead, but that just > moves the inconsistency to the type name. > > * QAuthZListFormat and QAuthZListPolicy > > The authz code consistently uses QAuthZ. > > Could make camel_to_upper() avoid the lone Z instead (and hope that'll > remain what we want). > > * QKeyCode > > Q_KEY_CODE is baked into subprojects/keycodemapdb/tools/keymap-gen. > > Could adjust the subproject instead. > > * VfioMigrationState > > Can't see why this one has a prefix. Avihai, can you enlighten me? linux-headers/linux/vfio.h defines enum vfio_device_mig_state with values VFIO_DEVICE_STATE_STOP etc. I used the QAPI prefix to emphasize this is a QAPI entity rather than a VFIO entity. Thanks. > > Daniel, thoughts? > >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> --- >>> qapi/block-core.json | 3 +- >>> qapi/common.json | 1 + >>> qapi/crypto.json | 6 ++-- >>> qapi/cryptodev.json | 1 - >>> qapi/ebpf.json | 1 + >>> qapi/machine.json | 1 + >>> qapi/migration.json | 1 + >>> qapi/ui.json | 2 ++ >>> scripts/qapi/common.py | 42 ++++++++++++++---------- >>> scripts/qapi/schema.py | 2 +- >>> tests/qapi-schema/alternate-array.out | 1 - >>> tests/qapi-schema/comments.out | 1 - >>> tests/qapi-schema/doc-good.out | 1 - >>> tests/qapi-schema/empty.out | 1 - >>> tests/qapi-schema/include-repetition.out | 1 - >>> tests/qapi-schema/include-simple.out | 1 - >>> tests/qapi-schema/indented-expr.out | 1 - >>> tests/qapi-schema/qapi-schema-test.json | 1 + >>> tests/qapi-schema/qapi-schema-test.out | 2 +- >>> 19 files changed, 37 insertions(+), 33 deletions(-) >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > Thanks! >
Avihai Horon <avihaih@nvidia.com> writes: > On 30/07/2024 15:22, Markus Armbruster wrote: >> >> Avihai, there's a question for you on VfioMigrationState. >> >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >>> On Tue, Jul 30, 2024 at 10:10:15AM +0200, Markus Armbruster wrote: [...] >> * VfioMigrationState >> >> Can't see why this one has a prefix. Avihai, can you enlighten me? > > linux-headers/linux/vfio.h defines enum vfio_device_mig_state with values VFIO_DEVICE_STATE_STOP etc. It does not define any VFIO_DEVICE_STATE_*, though. > I used the QAPI prefix to emphasize this is a QAPI entity rather than a VFIO entity. We define about two dozen symbols starting with VFIO_, and several hundreds starting with vfio_. What makes this enumeration type different so its members need emphasis? [...]
On 31/07/2024 8:12, Markus Armbruster wrote: > External email: Use caution opening links or attachments > > > Avihai Horon <avihaih@nvidia.com> writes: > >> On 30/07/2024 15:22, Markus Armbruster wrote: >>> Avihai, there's a question for you on VfioMigrationState. >>> >>> Daniel P. Berrangé <berrange@redhat.com> writes: >>> >>>> On Tue, Jul 30, 2024 at 10:10:15AM +0200, Markus Armbruster wrote: > [...] > >>> * VfioMigrationState >>> >>> Can't see why this one has a prefix. Avihai, can you enlighten me? >> linux-headers/linux/vfio.h defines enum vfio_device_mig_state with values VFIO_DEVICE_STATE_STOP etc. > It does not define any VFIO_DEVICE_STATE_*, though. > >> I used the QAPI prefix to emphasize this is a QAPI entity rather than a VFIO entity. > We define about two dozen symbols starting with VFIO_, and several > hundreds starting with vfio_. What makes this enumeration type > different so its members need emphasis? Right. I thought it would be clearer with the QAPI prefix because VFIO_DEVICE_STATE_* and VFIO_MIGRATION_STATE_* have similar values. But it's not a must. If you want to reduce prefix usage, go ahead, I don't have a strong opinion about it. > > [...] >
Avihai Horon <avihaih@nvidia.com> writes: > On 31/07/2024 8:12, Markus Armbruster wrote: >> External email: Use caution opening links or attachments >> >> >> Avihai Horon <avihaih@nvidia.com> writes: >> >>> On 30/07/2024 15:22, Markus Armbruster wrote: >>>> Avihai, there's a question for you on VfioMigrationState. >>>> >>>> Daniel P. Berrangé <berrange@redhat.com> writes: >>>> >>>>> On Tue, Jul 30, 2024 at 10:10:15AM +0200, Markus Armbruster wrote: >> [...] >> >>>> * VfioMigrationState >>>> >>>> Can't see why this one has a prefix. Avihai, can you enlighten me? >>> >>> linux-headers/linux/vfio.h defines enum vfio_device_mig_state with values VFIO_DEVICE_STATE_STOP etc. >> >> It does not define any VFIO_DEVICE_STATE_*, though. >> >>> I used the QAPI prefix to emphasize this is a QAPI entity rather than a VFIO entity. >> >> We define about two dozen symbols starting with VFIO_, and several >> hundreds starting with vfio_. What makes this enumeration type >> different so its members need emphasis? > > Right. I thought it would be clearer with the QAPI prefix because VFIO_DEVICE_STATE_* and VFIO_MIGRATION_STATE_* have similar values. > > But it's not a must. If you want to reduce prefix usage, go ahead, I don't have a strong opinion about it. Thanks!
© 2016 - 2024 Red Hat, Inc.