[PATCH 1/3] qemu_monitor: add qemuMonitorQueryStats

[Martin Kletzander]

On Fri, Jul 22, 2022 at 02:44:30PM +0200, Kristina Hanicova wrote:
>On Thu, Jul 14, 2022 at 7:54 AM Amneesh Singh <natto at weirdnatto.in> wrote:
>
>> Related: https://gitlab.com/libvirt/libvirt/-/issues/276
>>
>> This patch adds an API for the "query-stats" QMP command.
>>
>> The query returns a JSON containing the statistics based on the target,
>> which can either be vCPU or VM, and the providers. The API deserializes
>> the query result into an array of GHashMaps, which can later be used to
>> extract all the query statistics. GHashMaps are used to avoid traversing
>> the entire array to find the statistics you are looking for. This would
>> be a singleton array if the target is a VM since the returned JSON is
>> also a singleton array in that case.
>>
>> Signed-off-by: Amneesh Singh <natto at weirdnatto.in>
>> ---
>>  src/qemu/qemu_monitor.c      |  70 +++++++++++++++++++
>>  src/qemu/qemu_monitor.h      |  45 ++++++++++++
>>  src/qemu/qemu_monitor_json.c | 130 +++++++++++++++++++++++++++++++++++
>>  src/qemu/qemu_monitor_json.h |   6 ++
>>  4 files changed, 251 insertions(+)
>>

[...]

>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index 95267ec6..344636e1 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -1554,3 +1554,48 @@ qemuMonitorChangeMemoryRequestedSize(qemuMonitor
>> *mon,
>>  int
>>  qemuMonitorMigrateRecover(qemuMonitor *mon,
>>                            const char *uri);
>> +
>> +typedef enum {
>> +    QEMU_MONITOR_QUERY_STATS_TARGET_VM,
>> +    QEMU_MONITOR_QUERY_STATS_TARGET_VCPU,
>> +    QEMU_MONITOR_QUERY_STATS_TARGET_LAST,
>> +} qemuMonitorQueryStatsTargetType;
>> +
>> +VIR_ENUM_DECL(qemuMonitorQueryStatsTarget);
>> +
>> +typedef enum {
>> +    QEMU_MONITOR_QUERY_STATS_NAME_HALT_POLL_SUCCESS_NS,
>> +    QEMU_MONITOR_QUERY_STATS_NAME_HALT_POLL_FAIL_NS,
>> +    QEMU_MONITOR_QUERY_STATS_NAME_LAST,
>> +} qemuMonitorQueryStatsNameType;
>> +
>> +VIR_ENUM_DECL(qemuMonitorQueryStatsName);
>> +
>> +typedef enum {
>> +    QEMU_MONITOR_QUERY_STATS_PROVIDER_KVM,
>> +    QEMU_MONITOR_QUERY_STATS_PROVIDER_LAST,
>> +} qemuMonitorQueryStatsProviderType;
>> +
>> +VIR_ENUM_DECL(qemuMonitorQueryStatsProvider);
>> +
>> +typedef struct _qemuMonitorQueryStatsProvider
>> qemuMonitorQueryStatsProvider;
>> +struct _qemuMonitorQueryStatsProvider {
>> +    qemuMonitorQueryStatsProviderType provider;
>>
>
>It is a bit confusing for me to have a variable 'provider' in the
>'provider' structure.
>Later you use provider->provider which is not very readable.
>I would prefer 'type' (maybe even 'providerType') instead.
>

I think I suggested `type` in previous review.

I'll fix this up and the other comments (mine as well) before pushing unless
there is something bigger in the last patch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220722/c120ce4b/attachment.sig>