qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-6.0 2/6] qapi: Rename KvmInfo to AccelInfo


From: Markus Armbruster
Subject: Re: [PATCH for-6.0 2/6] qapi: Rename KvmInfo to AccelInfo
Date: Fri, 27 Nov 2020 13:08:38 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Roman Bolshakov (r.bolshakov@yadro.com) wrote:
>> There's nothing specific to KVM in the structure. A more generic name
>> would be more appropriate.
>> 
>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
>
> For HMP:
>
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Markus/Eric: Is it OK from QMP to change the type like that?

Type names are not part of the external interface, and can therefore be
changed freely.

>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 11f364fab4..5648d8d24d 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -562,24 +562,24 @@
>>  { 'command': 'inject-nmi' }
>>  
>>  ##
>> -# @KvmInfo:
>> +# @AccelInfo:
>>  #
>> -# Information about support for KVM acceleration
>> +# Information about support for an acceleration

We might add accelerator information that isn't about "is this one
supported?", and then the description becomes misleading.

Suggest

    # Accelerator information

>>  #
>> -# @enabled: true if KVM acceleration is active
>> +# @enabled: true if an acceleration is active

Well, "an acceleration" is always active.  The flag tells us that *this*
accelerator is active.  Suggest

    # @enabled: whether this accelerator is active

>>  #
>> -# @present: true if KVM acceleration is built into this executable
>> +# @present: true if an acceleration is built into this executable

Suggest

    # @present: whether this accelerator is ...


>>  #
>>  # Since: 0.14.0
>>  ##
>> -{ 'struct': 'KvmInfo', 'data': {'enabled': 'bool', 'present': 'bool'} }
>> +{ 'struct': 'AccelInfo', 'data': {'enabled': 'bool', 'present': 'bool'} }
>>  
>>  ##
>>  # @query-kvm:
>>  #
>>  # Returns information about KVM acceleration
>>  #
>> -# Returns: @KvmInfo
>> +# Returns: @AccelInfo
>>  #
>>  # Since: 0.14.0
>>  #
>> @@ -589,14 +589,14 @@
>>  # <- { "return": { "enabled": true, "present": true } }
>>  #
>>  ##
>> -{ 'command': 'query-kvm', 'returns': 'KvmInfo' }
>> +{ 'command': 'query-kvm', 'returns': 'AccelInfo' }
>>  
>>  ##
>>  # @query-accel:
>>  #
>>  # Returns information about an accelerator
>>  #
>> -# Returns: @KvmInfo
>> +# Returns: @AccelInfo
>>  #
>>  # Since: 6.0.0
>>  #
>> @@ -608,7 +608,7 @@
>>  ##
>>  { 'command': 'query-accel',
>>    'data': { 'name': 'str' },
>> -  'returns': 'KvmInfo' }
>> +  'returns': 'AccelInfo' }
>>  
>>  ##
>>  # @NumaOptionsType:
>> -- 
>> 2.29.2
>> 

Adjust the comments, and you may add
Acked-by: Markus Armbruster <armbru@redhat.com>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]