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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> --- 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.c b/src/qemu/qemu_monitor.c index fda5d2f3..a07e6017 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4541,3 +4541,73 @@ qemuMonitorMigrateRecover(qemuMonitor *mon, return qemuMonitorJSONMigrateRecover(mon, uri); } + +VIR_ENUM_IMPL(qemuMonitorQueryStatsTarget, + QEMU_MONITOR_QUERY_STATS_TARGET_LAST, + "vm", + "vcpu", +); + +VIR_ENUM_IMPL(qemuMonitorQueryStatsName, + QEMU_MONITOR_QUERY_STATS_NAME_LAST, + "halt_poll_success_ns", + "halt_poll_fail_ns" +); + +VIR_ENUM_IMPL(qemuMonitorQueryStatsProvider, + QEMU_MONITOR_QUERY_STATS_PROVIDER_LAST, + "kvm", +); + +void +qemuMonitorQueryStatsProviderFree(qemuMonitorQueryStatsProvider *provider) +{ + virBitmapFree(provider->names); + g_free(provider); +} + +qemuMonitorQueryStatsProvider * +qemuMonitorQueryStatsProviderNew(qemuMonitorQueryStatsProviderType provider_type, + ...) +{ + g_autoptr(qemuMonitorQueryStatsProvider) provider = NULL; + qemuMonitorQueryStatsNameType stat; + va_list name_list; + size_t sentinel = QEMU_MONITOR_QUERY_STATS_NAME_LAST; + + provider = g_new0(qemuMonitorQueryStatsProvider, 1); + provider->provider = provider_type; + provider->names = NULL; + + va_start(name_list, provider_type); + stat = va_arg(name_list, qemuMonitorQueryStatsNameType); + + if (stat != sentinel) {It would be better to compare 'stat' with QEMU_MONITOR_QUERY_STATS_NAME_LAST directly without the additional variable 'sentinel'.+ provider->names = virBitmapNew(QEMU_MONITOR_QUERY_STATS_NAME_LAST); + + while (stat != sentinel) {Same here. This while cycle would be better outside the if case, such as: if (stat != sentinel) provider->names = virBitmapNew(QEMU_MONITOR_QUERY_STATS_NAME_LAST); while (stat != sentinel) { .... } Because the 'while' cycle has the same condition as the 'if' case.+ if (virBitmapSetBit(provider->names, stat) < 0) + return NULL; + stat = va_arg(name_list, qemuMonitorQueryStatsNameType); + } + } + va_end(name_list); + + return g_steal_pointer(&provider); +}
Sorry for so many review e-mails from me on this one patch, but while fixing few
of the nitpicks I managed to rewrite the function quite a bit:
qemuMonitorQueryStatsProvider *provider = g_new0(qemuMonitorQueryStatsProvider, 1);
qemuMonitorQueryStatsNameType stat;
va_list name_list;
/*
* This can be lowered later in case of the enum getting quite large, hence
* the virBitmapSetExpand below.
*/
provider->names = virBitmapNew(QEMU_MONITOR_QUERY_STATS_NAME_LAST);
provider->type = provider_type;
va_start(name_list, provider_type);
while ((stat = va_arg(name_list, qemuMonitorQueryStatsNameType)) !=
QEMU_MONITOR_QUERY_STATS_NAME_LAST)
virBitmapSetBitExpand(provider->names, stat);
va_end(name_list);
return provider;
Let me know (both of you) if you are okay with me using this version.
Thanks
Attachment:
signature.asc
Description: PGP signature
