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

[Markus Armbruster <armbru@redhat.com>]

"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:
>  #