Re: [PATCH v2 3/8] qapi/misc: Restrict balloon-related commands to machine code

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi David,

On 3/16/20 10:05 AM, David Hildenbrand wrote:
> On 16.03.20 01:03, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   qapi/machine.json          | 83 ++++++++++++++++++++++++++++++++++++++
>>   qapi/misc.json             | 83 --------------------------------------
>>   include/sysemu/balloon.h   |  2 +-
>>   balloon.c                  |  2 +-
>>   hw/virtio/virtio-balloon.c |  2 +-
>>   monitor/hmp-cmds.c         |  1 +
>>   6 files changed, 87 insertions(+), 86 deletions(-)
>>
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 07ffc07ba2..c096efbea3 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -915,3 +915,86 @@
>>     'data': 'NumaOptions',
>>     'allow-preconfig': true
>>   }
>> +
>> +##
>> +# @balloon:
>> +#
>> +# Request the balloon driver to change its balloon size.
>> +#
>> +# @value: the target size of the balloon in bytes
> 
> Not related to your patch. The description of most of this stuff is wrong.
> 
> It's not the target size of the balloon, it's the target logical size of
> the VM (logical_vm_size = vm_ram_size - balloon_size)
> 
> -> balloon_size = vm_ram_size - @value
> 
> E.g., "balloon 1024" with a 3G guest means "please inflate the balloon
> to 2048"
> 
>> +#
>> +# Returns: - Nothing on success
>> +#          - If the balloon driver is enabled but not functional because the KVM
>> +#            kernel module cannot support it, KvmMissingCap
>> +#          - If no balloon device is present, DeviceNotActive
>> +#
>> +# Notes: This command just issues a request to the guest.  When it returns,
>> +#        the balloon size may not have changed.  A guest can change the balloon
>> +#        size independent of this command.
>> +#
>> +# Since: 0.14.0
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "balloon", "arguments": { "value": 536870912 } }
>> +# <- { "return": {} }
>> +#
>> +##
>> +{ 'command': 'balloon', 'data': {'value': 'int'} }
>> +
>> +##
>> +# @BalloonInfo:
>> +#
>> +# Information about the guest balloon device.
>> +#
>> +# @actual: the number of bytes the balloon currently contains
> 
> Dito
> 
> @actual is the logical size of the VM (logical_vm_size = vm_ram_size -
> balloon_size)
> 
>> +#
>> +# Since: 0.14.0
>> +#
>> +##
>> +{ 'struct': 'BalloonInfo', 'data': {'actual': 'int' } }
>> +
>> +##
>> +# @query-balloon:
>> +#
>> +# Return information about the balloon device.
>> +#
>> +# Returns: - @BalloonInfo on success
>> +#          - If the balloon driver is enabled but not functional because the KVM
>> +#            kernel module cannot support it, KvmMissingCap
>> +#          - If no balloon device is present, DeviceNotActive
>> +#
>> +# Since: 0.14.0
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "query-balloon" }
>> +# <- { "return": {
>> +#          "actual": 1073741824,
>> +#       }
>> +#    }
>> +#
>> +##
>> +{ 'command': 'query-balloon', 'returns': 'BalloonInfo' }
>> +
>> +##
>> +# @BALLOON_CHANGE:
>> +#
>> +# Emitted when the guest changes the actual BALLOON level. This value is
>> +# equivalent to the @actual field return by the 'query-balloon' command
>> +#
>> +# @actual: actual level of the guest memory balloon in bytes
> 
> Dito
> 
> @actual is the logical size of the VM (vm_ram_size - balloon_size)
> 
> 
> Most probably we want to pull this description fix out. #badinterface

Since you understand how ballooning works, do you mind sending a patch 
with description fixed? :)

Thanks,

Phil.