diff mbox series

[01/18] qapi: Smarter camel_to_upper() to reduce need for 'prefix'

Message ID 20240730081032.1246748-2-armbru@redhat.com
State New
Headers show
Series qapi: Reduce use of 'prefix'. | expand

Commit Message

Markus Armbruster July 30, 2024, 8:10 a.m. UTC
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(-)

Comments

Daniel P. Berrangé July 30, 2024, 9:13 a.m. UTC | #1
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
Markus Armbruster July 30, 2024, 12:22 p.m. UTC | #2
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!
Avihai Horon July 30, 2024, 1:33 p.m. UTC | #3
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!
>
Markus Armbruster July 31, 2024, 5:12 a.m. UTC | #4
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?

[...]
Avihai Horon July 31, 2024, 5:59 a.m. UTC | #5
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.

>
> [...]
>
Markus Armbruster July 31, 2024, 6:37 a.m. UTC | #6
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!
Kevin Wolf July 31, 2024, 9:43 a.m. UTC | #7
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. 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
Markus Armbruster Aug. 9, 2024, 9:03 a.m. UTC | #8
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".
diff mbox series

Patch

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