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:

> 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

[...]