Re: [PATCH 09/18] qapi/machine: Rename CpuS390* to S390Cpu, and drop 'prefix'

[Markus Armbruster <armbru@redhat.com>]

Zhao Liu <zhao1.liu@intel.com> writes:

> On Tue, Jul 30, 2024 at 10:10:23AM +0200, Markus Armbruster wrote:
>> Date: Tue, 30 Jul 2024 10:10:23 +0200
>> From: Markus Armbruster <armbru@redhat.com>
>> Subject: [PATCH 09/18] qapi/machine: Rename CpuS390* to S390Cpu, and drop
>>  'prefix'
>> 
>> QAPI's 'prefix' feature can make the connection between enumeration
>> type and its constants less than obvious.  It's best used with
>> restraint.
>> 
>> CpuS390Entitlement has a 'prefix' to change the generated enumeration
>> constants' prefix from CPU_S390_POLARIZATION to S390_CPU_POLARIZATION.
>                          ^^^^^^^^^^^^^^^^^^^^^    ^^^^^^^^^^^^^^^^^^^^^
> 			 CPU_S390_ENTITLEMENT     S390_CPU_ENTITLEMENT

Yes.

>> Rename the type to S390CpuEntitlement, so that 'prefix' is not needed.
>> 
>> Likewise change CpuS390Polarization to S390CpuPolarization, and
>> CpuS390State to S390CpuState.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qapi/machine-common.json            |  5 ++---
>>  qapi/machine-target.json            | 11 +++++------
>>  qapi/machine.json                   |  9 ++++-----
>>  qapi/pragma.json                    |  6 +++---
>>  include/hw/qdev-properties-system.h |  2 +-
>>  include/hw/s390x/cpu-topology.h     |  2 +-
>>  target/s390x/cpu.h                  |  2 +-
>>  hw/core/qdev-properties-system.c    |  6 +++---
>>  hw/s390x/cpu-topology.c             |  6 +++---
>>  9 files changed, 23 insertions(+), 26 deletions(-)
>
> [snip]
>
>> diff --git a/qapi/pragma.json b/qapi/pragma.json
>> index 59fbe74b8c..beddea5ca4 100644
>> --- a/qapi/pragma.json
>> +++ b/qapi/pragma.json
>> @@ -47,9 +47,9 @@
>>          'BlockdevSnapshotWrapper',
>>          'BlockdevVmdkAdapterType',
>>          'ChardevBackendKind',
>> -        'CpuS390Entitlement',
>> -        'CpuS390Polarization',
>> -        'CpuS390State',
>> +        'S390CpuEntitlement',
>> +        'S390CpuPolarization',
>> +        'S390CpuState',
>>          'CxlCorErrorType',
>>          'DisplayProtocol',
>>          'DriveBackupWrapper',
>
> It seems to be in alphabetical order. The new names don't follow the
> original order.

You're right.

> Just the above nits,
>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Thanks!