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

[David Hildenbrand <david@redhat.com>]

On 17.03.20 12:03, Philippe Mathieu-Daudé wrote:
> 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? :)

Will add it to my todo list, so your patch can go in first :)

Thanks

-- 
Thanks,

David / dhildenb