1. Patchew
  2. QEMU
  3. [PATCH v7 0/7] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)
  4. View patch

[PATCH v7 6/7] qapi: Add HV_BALLOON_STATUS_REPORT event and its QMP query command

Maciej S. Szmigiero posted 7 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v7 6/7] qapi: Add HV_BALLOON_STATUS_REPORT event and its QMP query command
Posted by Maciej S. Szmigiero 1 year, 3 months ago 
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>
---
 hw/hyperv/hv-balloon.c | 31 +++++++++++++++++++-
 monitor/monitor.c      |  1 +
 qapi/machine.json      | 64 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/hw/hyperv/hv-balloon.c b/hw/hyperv/hv-balloon.c
index c384f23a3b5e..d3f24721248a 100644
--- a/hw/hyperv/hv-balloon.c
+++ b/hw/hyperv/hv-balloon.c
@@ -1099,7 +1099,36 @@ static void hv_balloon_handle_status_report(HvBalloon *balloon,
     balloon->status_report.available *= HV_BALLOON_PAGE_SIZE;
     balloon->status_report.received = true;
 
-    /* report event */
+    qapi_event_send_hv_balloon_status_report(balloon->status_report.committed,
+                                             balloon->status_report.available);
+}
+
+HvBalloonInfo *qmp_query_hv_balloon_status_report(Error **errp)
+{
+    HvBalloon *balloon;
+    HvBalloonInfo *info;
+
+    balloon = HV_BALLOON(object_resolve_path_type("", TYPE_HV_BALLOON, NULL));
+    if (!balloon) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                  "no %s device present", TYPE_HV_BALLOON);
+        return NULL;
+    }
+
+    if (!balloon->status_report.enabled) {
+        error_setg(errp, "guest memory status reporting not enabled");
+        return NULL;
+    }
+
+    if (!balloon->status_report.received) {
+        error_setg(errp, "no guest memory status report received yet");
+        return NULL;
+    }
+
+    info = g_malloc0(sizeof(*info));
+    info->committed = balloon->status_report.committed;
+    info->available = balloon->status_report.available;
+    return info;
 }
 
 static void hv_balloon_handle_unballoon_response(HvBalloon *balloon,
diff --git a/monitor/monitor.c b/monitor/monitor.c
index dc352f9e9d95..81513c642691 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -315,6 +315,7 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
     [QAPI_EVENT_QUORUM_FAILURE]    = { 1000 * SCALE_MS },
     [QAPI_EVENT_VSERPORT_CHANGE]   = { 1000 * SCALE_MS },
     [QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE] = { 1000 * SCALE_MS },
+    [QAPI_EVENT_HV_BALLOON_STATUS_REPORT] = { 1000 * SCALE_MS },
 };
 
 /*
diff --git a/qapi/machine.json b/qapi/machine.json
index 5ede977cf2bc..f592876964af 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1113,6 +1113,70 @@
 { 'event': 'BALLOON_CHANGE',
   'data': { 'actual': 'int' } }
 
+##
+# @HvBalloonInfo:
+#
+# hv-balloon guest-provided memory status information.
+#
+# @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")
+#
+# Since: 8.2
+##
+{ 'struct': 'HvBalloonInfo', 'data': { 'committed': 'size', 'available': 'size' } }
+
+##
+# @query-hv-balloon-status-report:
+#
+# Returns the hv-balloon driver data contained in the last received "STATUS"
+# message from the guest.
+#
+# Returns:
+# - @HvBalloonInfo on success
+# - If no hv-balloon device is present, DeviceNotFound
+# - If guest memory status reporting not enabled or no guest memory status
+#     report received yet, GenericError
+#
+# Since: 8.2
+#
+# Example:
+#
+# -> { "execute": "query-hv-balloon-status-report" }
+# <- { "return": {
+#          "committed": 816640000,
+#          "available": 3333054464
+#       }
+#    }
+##
+{ 'command': 'query-hv-balloon-status-report', 'returns': 'HvBalloonInfo' }
+
+##
+# @HV_BALLOON_STATUS_REPORT:
+#
+# Emitted when the hv-balloon driver receives a "STATUS" message from
+# the guest.
+#
+# @data - a @HvBalloonInfo equivalent to the one returned by the
+#     'query-hv-balloon-status-report' command.
+#
+# Note: this event is rate-limited.
+#
+# Since: 8.2
+#
+# Example:
+#
+# <- { "event": "HV_BALLOON_STATUS_REPORT",
+#      "data": { "committed": 816640000, "available": 3333054464 },
+#      "timestamp": { "seconds": 1600295492, "microseconds": 661044 } }
+#
+##
+{ 'event': 'HV_BALLOON_STATUS_REPORT',
+  'data': 'HvBalloonInfo' }
+
 ##
 # @MemoryInfo:
 #
Re: [PATCH v7 6/7] qapi: Add HV_BALLOON_STATUS_REPORT event and its QMP query command
Posted by Markus Armbruster 1 year, 2 months ago 
"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.
>
> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>

[...]

>  static void hv_balloon_handle_unballoon_response(HvBalloon *balloon,
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index dc352f9e9d95..81513c642691 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -315,6 +315,7 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
>      [QAPI_EVENT_QUORUM_FAILURE]    = { 1000 * SCALE_MS },
>      [QAPI_EVENT_VSERPORT_CHANGE]   = { 1000 * SCALE_MS },
>      [QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE] = { 1000 * SCALE_MS },
> +    [QAPI_EVENT_HV_BALLOON_STATUS_REPORT] = { 1000 * SCALE_MS },
>  };
>  
>  /*
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 5ede977cf2bc..f592876964af 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1113,6 +1113,70 @@
>  { 'event': 'BALLOON_CHANGE',
>    'data': { 'actual': 'int' } }
>  
> +##
> +# @HvBalloonInfo:
> +#
> +# hv-balloon guest-provided memory status information.
> +#
> +# @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")
> +#
> +# Since: 8.2
> +##
> +{ 'struct': 'HvBalloonInfo', 'data': { 'committed': 'size', 'available': 'size' } }
> +
> +##
> +# @query-hv-balloon-status-report:
> +#
> +# Returns the hv-balloon driver data contained in the last received "STATUS"
> +# message from the guest.
> +#
> +# Returns:
> +# - @HvBalloonInfo on success
> +# - If no hv-balloon device is present, DeviceNotFound
> +# - If guest memory status reporting not enabled or no guest memory status
> +#     report received yet, GenericError

Indentation is off, confusing Sphinx.

Do you actually need to tell the two failures apart?

Do me a favour and break the lines a bit earlier, like

   # Returns the hv-balloon driver data contained in the last received
   # "STATUS" message from the guest.
   #
   # Returns:
   # - @HvBalloonInfo on success
   # - If no hv-balloon device is present, DeviceNotFound
   # - If guest memory status reporting not enabled or no guest memory
   #   status report received yet, GenericError

> +#
> +# Since: 8.2
> +#
> +# Example:
> +#
> +# -> { "execute": "query-hv-balloon-status-report" }
> +# <- { "return": {
> +#          "committed": 816640000,
> +#          "available": 3333054464
> +#       }
> +#    }
> +##
> +{ 'command': 'query-hv-balloon-status-report', 'returns': 'HvBalloonInfo' }
> +
> +##
> +# @HV_BALLOON_STATUS_REPORT:
> +#
> +# Emitted when the hv-balloon driver receives a "STATUS" message from
> +# the guest.
> +#
> +# @data - a @HvBalloonInfo equivalent to the one returned by the

This needs to be

   # @data: ... text ...

to be rendered correctly.

Suggest

   # @data: contents of "STATUS" message received from the guest.

> +#     'query-hv-balloon-status-report' command.
> +#
> +# Note: this event is rate-limited.
> +#
> +# Since: 8.2
> +#
> +# Example:
> +#
> +# <- { "event": "HV_BALLOON_STATUS_REPORT",
> +#      "data": { "committed": 816640000, "available": 3333054464 },
> +#      "timestamp": { "seconds": 1600295492, "microseconds": 661044 } }
> +#
> +##
> +{ 'event': 'HV_BALLOON_STATUS_REPORT',
> +  'data': 'HvBalloonInfo' }
> +
>  ##
>  # @MemoryInfo:
>  #
Re: [PATCH v7 6/7] qapi: Add HV_BALLOON_STATUS_REPORT event and its QMP query command
Posted by Maciej S. Szmigiero 1 year, 2 months ago 
On 25.09.2023 13:49, 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.
>>
>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
> 
> [...]
> 
>>   static void hv_balloon_handle_unballoon_response(HvBalloon *balloon,
>> diff --git a/monitor/monitor.c b/monitor/monitor.c
>> index dc352f9e9d95..81513c642691 100644
>> --- a/monitor/monitor.c
>> +++ b/monitor/monitor.c
>> @@ -315,6 +315,7 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
>>       [QAPI_EVENT_QUORUM_FAILURE]    = { 1000 * SCALE_MS },
>>       [QAPI_EVENT_VSERPORT_CHANGE]   = { 1000 * SCALE_MS },
>>       [QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE] = { 1000 * SCALE_MS },
>> +    [QAPI_EVENT_HV_BALLOON_STATUS_REPORT] = { 1000 * SCALE_MS },
>>   };
>>   
>>   /*
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 5ede977cf2bc..f592876964af 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -1113,6 +1113,70 @@
>>   { 'event': 'BALLOON_CHANGE',
>>     'data': { 'actual': 'int' } }
>>   
>> +##
>> +# @HvBalloonInfo:
>> +#
>> +# hv-balloon guest-provided memory status information.
>> +#
>> +# @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")
>> +#
>> +# Since: 8.2
>> +##
>> +{ 'struct': 'HvBalloonInfo', 'data': { 'committed': 'size', 'available': 'size' } }
>> +
>> +##
>> +# @query-hv-balloon-status-report:
>> +#
>> +# Returns the hv-balloon driver data contained in the last received "STATUS"
>> +# message from the guest.
>> +#
>> +# Returns:
>> +# - @HvBalloonInfo on success
>> +# - If no hv-balloon device is present, DeviceNotFound
>> +# - If guest memory status reporting not enabled or no guest memory status
>> +#     report received yet, GenericError
> 
> Indentation is off, confusing Sphinx.
> 
> Do you actually need to tell the two failures apart?

Technically no, it's just for the API consumer convenience.

> Do me a favour and break the lines a bit earlier, like
> 
>     # Returns the hv-balloon driver data contained in the last received
>     # "STATUS" message from the guest.
>     #
>     # Returns:
>     # - @HvBalloonInfo on success
>     # - If no hv-balloon device is present, DeviceNotFound
>     # - If guest memory status reporting not enabled or no guest memory
>     #   status report received yet, GenericError

Will do.

>> +#
>> +# Since: 8.2
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "query-hv-balloon-status-report" }
>> +# <- { "return": {
>> +#          "committed": 816640000,
>> +#          "available": 3333054464
>> +#       }
>> +#    }
>> +##
>> +{ 'command': 'query-hv-balloon-status-report', 'returns': 'HvBalloonInfo' }
>> +
>> +##
>> +# @HV_BALLOON_STATUS_REPORT:
>> +#
>> +# Emitted when the hv-balloon driver receives a "STATUS" message from
>> +# the guest.
>> +#
>> +# @data - a @HvBalloonInfo equivalent to the one returned by the
> 
> This needs to be
> 
>     # @data: ... text ...
> 
> to be rendered correctly.
> 
> Suggest
> 
>     # @data: contents of "STATUS" message received from the guest.

Will do.

Thanks,
Maciej
Re: [PATCH v7 6/7] qapi: Add HV_BALLOON_STATUS_REPORT event and its QMP query command
Posted by Markus Armbruster 1 year, 2 months ago 
"Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes:

> On 25.09.2023 13:49, 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.
>>>
>>> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>> [...]
>> 
>>>   static void hv_balloon_handle_unballoon_response(HvBalloon *balloon,
>>> diff --git a/monitor/monitor.c b/monitor/monitor.c
>>> index dc352f9e9d95..81513c642691 100644
>>> --- a/monitor/monitor.c
>>> +++ b/monitor/monitor.c
>>> @@ -315,6 +315,7 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
>>>       [QAPI_EVENT_QUORUM_FAILURE]    = { 1000 * SCALE_MS },
>>>       [QAPI_EVENT_VSERPORT_CHANGE]   = { 1000 * SCALE_MS },
>>>       [QAPI_EVENT_MEMORY_DEVICE_SIZE_CHANGE] = { 1000 * SCALE_MS },
>>> +    [QAPI_EVENT_HV_BALLOON_STATUS_REPORT] = { 1000 * SCALE_MS },
>>>   };
>>>     /*
>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>> index 5ede977cf2bc..f592876964af 100644
>>> --- a/qapi/machine.json
>>> +++ b/qapi/machine.json
>>> @@ -1113,6 +1113,70 @@
>>>  { 'event': 'BALLOON_CHANGE',
>>>    'data': { 'actual': 'int' } }
>>> +##
>>> +# @HvBalloonInfo:
>>> +#
>>> +# hv-balloon guest-provided memory status information.
>>> +#
>>> +# @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")
>>> +#
>>> +# Since: 8.2
>>> +##
>>> +{ 'struct': 'HvBalloonInfo', 'data': { 'committed': 'size', 'available': 'size' } }

Long line.  Wrap it like

     { 'struct': 'HvBalloonInfo',
       'data': { 'committed': 'size', 'available': 'size' } }

>>> +
>>> +##
>>> +# @query-hv-balloon-status-report:
>>> +#
>>> +# Returns the hv-balloon driver data contained in the last received "STATUS"
>>> +# message from the guest.
>>> +#
>>> +# Returns:
>>> +# - @HvBalloonInfo on success
>>> +# - If no hv-balloon device is present, DeviceNotFound
>>> +# - If guest memory status reporting not enabled or no guest memory status
>>> +#     report received yet, GenericError
>>
>> Indentation is off, confusing Sphinx.
>>
>> Do you actually need to tell the two failures apart?
>
> Technically no, it's just for the API consumer convenience.

Error classes are remnants of a failed error reporting design ("rich"
error objects), and new code should stick to GenericError.  Exceptions
can be made when there's a proven need for distinguishing errors, or for
consistency, say when extending an existing interface.

The commit burying "rich" errors:

    de253f1491 qmp: switch to the new error format on the wire

Followup commits:

    6ec46ad541 block: Fix block-set-write-threshold not to use funky error class
    f3cf80e805 vnc: Fix QMP change not to use funky error class
    5b347c5410 block: Fix blockdev-backup not to use funky error class
    71568864c4 qapi: Fix up references to long gone error classes

[...]

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.