diff mbox series

[v6,5/6] qapi: Add HV_BALLOON_STATUS_REPORT event

Message ID 22d53a9cc8756bc39b5d951436dc276fa2c665cc.1689786474.git.maciej.szmigiero@oracle.com
State New
Headers show
Series Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️) | expand

Commit Message

Maciej S. Szmigiero July 20, 2023, 10:13 a.m. UTC
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>

Used by the hv-balloon driver for (optional) guest memory status reports.

Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
---
 qapi/machine.json | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Comments

Markus Armbruster July 25, 2023, 8:04 a.m. UTC | #1
"Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes:

> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>
> Used by the hv-balloon driver for (optional) guest memory status reports.

Inhowfar optional?  What enables / triggers it?

Use case for the event?

Could a status event make sense for other balloon drivers as well?

> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> ---
>  qapi/machine.json | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 5ede977cf2bc..9649616b9ed2 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1113,6 +1113,31 @@
>  { 'event': 'BALLOON_CHANGE',
>    'data': { 'actual': 'int' } }
>  
> +##
> +# @HV_BALLOON_STATUS_REPORT:
> +#
> +# Emitted when the hv-balloon driver receives a "STATUS" message from
> +# the guest.

Aha, the event is triggered by the guest.  It must therefore be
rate-limited, just like BALLOON_CHANGE.  To do that, add it to
monitor_qapi_event_conf[] in monitor/monitor.c, and document it as noted
below.

> +#
> +# @commited: the amount of memory in use inside the guest plus the amount
> +#            of the memory unusable inside the guest (ballooned out,
> +#            offline, etc.)
> +#
> +# @available: the amount of the memory inside the guest available for new
> +#             allocations ("free")

Spelling: committed.  Remember to update the example, too.

Please format like

# @committed: the amount of memory in use inside the guest plus the
#     amount of the memory unusable inside the guest (ballooned out,
#     offline, etc.)
#
# @available: the amount of the memory inside the guest available for
#     new allocations ("free")

to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
to conform to current conventions).

> +#

To document rate-limiting, add:

   # Note: this event is rate-limited.
   #

> +# Since: TBD
> +#
> +# Example:
> +#
> +# <- { "event": "HV_BALLOON_STATUS_REPORT",
> +#      "data": { "commited": 816640000, "available": 3333054464 },
> +#      "timestamp": { "seconds": 1600295492, "microseconds": 661044 } }
> +#
> +##
> +{ 'event': 'HV_BALLOON_STATUS_REPORT',
> +  'data': { 'commited': 'size', 'available': 'size' } }
> +
>  ##
>  # @MemoryInfo:
>  #

An event is commonly paired with a query command, so that QMP clients
can resynchronize state after missing events, e.g. when reconnecting
after a client restart.

query-balloon isn't such a query: it returns less than the event.

If a paired query doesn't make sense, explain why.
Maciej S. Szmigiero July 25, 2023, 6:13 p.m. UTC | #2
On 25.07.2023 10:04, Markus Armbruster wrote:
> "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes:
> 
>> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
>>
>> Used by the hv-balloon driver for (optional) guest memory status reports.
> 
> Inhowfar optional? What enables / triggers it?

They are enabled by "status-report=on" device property, hence they don't
need to be enabled if unwanted.

As you have written below, each status report is generated by the guest
sending a DM_STATUS_REPORT message (which guests do periodically).

> 
> Use case for the event?

To monitor memory state in the guest, for example for some QEMU
auto-ballooning controller.

> Could a status event make sense for other balloon drivers as well?

virtio-balloon has some guest memory stats support, too, but
with important differences, because in virtio-balloon:
1) Stats retrieval is driven by the QEMU process (essentially
polling the guest),

2) There's no notification mechanism for QEMU controller to know
that new stats have arrived from the guest,

3) The list of available individual stats is not constant,
rather it's an array of (TAG, VALUE) pairs.

>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>> ---
>>   qapi/machine.json | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 5ede977cf2bc..9649616b9ed2 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -1113,6 +1113,31 @@
>>   { 'event': 'BALLOON_CHANGE',
>>     'data': { 'actual': 'int' } }
>>   
>> +##
>> +# @HV_BALLOON_STATUS_REPORT:
>> +#
>> +# Emitted when the hv-balloon driver receives a "STATUS" message from
>> +# the guest.
> 
> Aha, the event is triggered by the guest.  It must therefore be
> rate-limited, just like BALLOON_CHANGE.  To do that, add it to
> monitor_qapi_event_conf[] in monitor/monitor.c, and document it as noted
> below.

Ack.

>> +#
>> +# @commited: the amount of memory in use inside the guest plus the amount
>> +#            of the memory unusable inside the guest (ballooned out,
>> +#            offline, etc.)
>> +#
>> +# @available: the amount of the memory inside the guest available for new
>> +#             allocations ("free")
> 
> Spelling: committed.  Remember to update the example, too.

Ack.

> Please format like
> 
> # @committed: the amount of memory in use inside the guest plus the
> #     amount of the memory unusable inside the guest (ballooned out,
> #     offline, etc.)
> #
> # @available: the amount of the memory inside the guest available for
> #     new allocations ("free")
> 
> to blend in with recent commit a937b6aa739 (qapi: Reformat doc comments
> to conform to current conventions).

Ack.

>> +#
> 
> To document rate-limiting, add:
> 
>     # Note: this event is rate-limited.
>     #

Ack.

>> +# Since: TBD
>> +#
>> +# Example:
>> +#
>> +# <- { "event": "HV_BALLOON_STATUS_REPORT",
>> +#      "data": { "commited": 816640000, "available": 3333054464 },
>> +#      "timestamp": { "seconds": 1600295492, "microseconds": 661044 } }
>> +#
>> +##
>> +{ 'event': 'HV_BALLOON_STATUS_REPORT',
>> +  'data': { 'commited': 'size', 'available': 'size' } }
>> +
>>   ##
>>   # @MemoryInfo:
>>   #
> 
> An event is commonly paired with a query command, so that QMP clients
> can resynchronize state after missing events, e.g. when reconnecting
> after a client restart.
> 
> query-balloon isn't such a query: it returns less than the event.
> 
> If a paired query doesn't make sense, explain why.
> 

Will add a query command that returns the last STATUS
event data.

Thanks,
Maciej
diff mbox series

Patch

diff --git a/qapi/machine.json b/qapi/machine.json
index 5ede977cf2bc..9649616b9ed2 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1113,6 +1113,31 @@ 
 { 'event': 'BALLOON_CHANGE',
   'data': { 'actual': 'int' } }
 
+##
+# @HV_BALLOON_STATUS_REPORT:
+#
+# Emitted when the hv-balloon driver receives a "STATUS" message from
+# the guest.
+#
+# @commited: the amount of memory in use inside the guest plus the amount
+#            of the memory unusable inside the guest (ballooned out,
+#            offline, etc.)
+#
+# @available: the amount of the memory inside the guest available for new
+#             allocations ("free")
+#
+# Since: TBD
+#
+# Example:
+#
+# <- { "event": "HV_BALLOON_STATUS_REPORT",
+#      "data": { "commited": 816640000, "available": 3333054464 },
+#      "timestamp": { "seconds": 1600295492, "microseconds": 661044 } }
+#
+##
+{ 'event': 'HV_BALLOON_STATUS_REPORT',
+  'data': { 'commited': 'size', 'available': 'size' } }
+
 ##
 # @MemoryInfo:
 #