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

[Maciej S. Szmigiero]

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