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@xxxxxxxxxx> 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@xxxxxxxxxx>
---
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@xxxxxxxxxx>
Thanks!
