[PATCH 0/5] Various (json related) cleanups

Some more patches from old branches that I didn't get around finishing before. Peter Krempa (5): virbitmap: Allow NULL bitmap in functions returning index of a set/clear bit qemuMonitorJSONQueryStats: Simplify logic to construct 'provider_list' qemu_monitor_json: Replace simplify fetching Array from JSON object qemu: agent: Use virJSONValueObjectGetArray qemuDomainGetStatsVcpu: Refactor cleanup src/qemu/qemu_agent.c | 29 ++++--------------- src/qemu/qemu_driver.c | 22 ++++++--------- src/qemu/qemu_monitor_json.c | 54 ++++++++++++++---------------------- src/util/virbitmap.c | 9 ++++++ src/util/virbitmap.h | 9 ++---- 5 files changed, 46 insertions(+), 77 deletions(-) -- 2.39.1

virBitmapNextSetBit/virBitmapLastSetBit/virBitmapNextClearBit can be used for iteration of a bitmap. Allow NULL bitmap so that iteration of a bitmap can be simplified in certain cases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 9 +++++++++ src/util/virbitmap.h | 9 +++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 5b9204cbd7..690ac592ba 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -824,6 +824,9 @@ virBitmapNextSetBit(virBitmap *bitmap, size_t nb; unsigned long bits; + if (!bitmap) + return -1; + if (pos < 0) pos = -1; @@ -863,6 +866,9 @@ virBitmapLastSetBit(virBitmap *bitmap) ssize_t sz; unsigned long bits; + if (!bitmap) + return -1; + /* If bitmap is empty then there is no set bit */ if (bitmap->map_len == 0) return -1; @@ -916,6 +922,9 @@ virBitmapNextClearBit(virBitmap *bitmap, size_t nb; unsigned long bits; + if (!bitmap) + return -1; + if (pos < 0) pos = -1; diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index e2314904b0..9f954f5ee7 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -111,14 +111,11 @@ bool virBitmapIsAllSet(virBitmap *bitmap) bool virBitmapIsAllClear(virBitmap *bitmap) ATTRIBUTE_NONNULL(1); -ssize_t virBitmapNextSetBit(virBitmap *bitmap, ssize_t pos) - ATTRIBUTE_NONNULL(1); +ssize_t virBitmapNextSetBit(virBitmap *bitmap, ssize_t pos); -ssize_t virBitmapLastSetBit(virBitmap *bitmap) - ATTRIBUTE_NONNULL(1); +ssize_t virBitmapLastSetBit(virBitmap *bitmap); -ssize_t virBitmapNextClearBit(virBitmap *bitmap, ssize_t pos) - ATTRIBUTE_NONNULL(1); +ssize_t virBitmapNextClearBit(virBitmap *bitmap, ssize_t pos); size_t virBitmapCountBits(virBitmap *bitmap) ATTRIBUTE_NONNULL(1); -- 2.39.1

Simplify construction of a single provider by using virJSONValueObjectAdd and restructuring the code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index db99017555..01e2aaa2cf 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8828,36 +8828,32 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon, g_autoptr(virJSONValue) reply = NULL; g_autoptr(virJSONValue) vcpu_list = NULL; g_autoptr(virJSONValue) provider_list = NULL; - size_t i; if (providers) { provider_list = virJSONValueNewArray(); for (i = 0; i < providers->len; i++) { - g_autoptr(virJSONValue) provider_obj = virJSONValueNewObject(); qemuMonitorQueryStatsProvider *provider = providers->pdata[i]; - const char *type_str = qemuMonitorQueryStatsProviderTypeToString(provider->type); - virBitmap *names = provider->names; - - if (virJSONValueObjectAppendString(provider_obj, "provider", type_str) < 0) - return NULL; - - if (!virBitmapIsAllClear(names)) { - g_autoptr(virJSONValue) provider_names = virJSONValueNewArray(); - ssize_t curBit = -1; + g_autoptr(virJSONValue) provider_obj = NULL; + g_autoptr(virJSONValue) provider_names = NULL; + ssize_t curBit = -1; - while ((curBit = virBitmapNextSetBit(names, curBit)) != -1) { - const char *name = qemuMonitorQueryStatsNameTypeToString(curBit); + while ((curBit = virBitmapNextSetBit(provider->names, curBit)) != -1) { + if (!provider_names) + provider_names = virJSONValueNewArray(); - if (virJSONValueArrayAppendString(provider_names, name) < 0) - return NULL; - } - - if (virJSONValueObjectAppend(provider_obj, "names", &provider_names) < 0) + if (virJSONValueArrayAppendString(provider_names, + qemuMonitorQueryStatsNameTypeToString(curBit)) < 0) return NULL; } + if (virJSONValueObjectAdd(&provider_obj, + "s:provider", qemuMonitorQueryStatsProviderTypeToString(provider->type), + "A:names", &provider_names, + NULL) < 0) + return NULL; + if (virJSONValueArrayAppend(provider_list, &provider_obj) < 0) return NULL; } -- 2.39.1

Replace instances of virJSONValueObjectGet + virJSONValueIsArray by virJSONValueObjectGetArray. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 01e2aaa2cf..d05ca2932f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3634,11 +3634,9 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValue *msg, "in query-rx-filter response")); return -1; } - if ((!(table = virJSONValueObjectGet(entry, "unicast-table"))) || - (!virJSONValueIsArray(table))) { + if ((!(table = virJSONValueObjectGetArray(entry, "unicast-table")))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing or invalid 'unicast-table' array " - "in query-rx-filter response")); + _("Missing or invalid 'unicast-table' array in query-rx-filter response")); return -1; } nTable = virJSONValueArraySize(table); @@ -3675,11 +3673,9 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValue *msg, "in query-rx-filter response")); return -1; } - if ((!(table = virJSONValueObjectGet(entry, "multicast-table"))) || - (!virJSONValueIsArray(table))) { + if ((!(table = virJSONValueObjectGetArray(entry, "multicast-table")))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing or invalid 'multicast-table' array " - "in query-rx-filter response")); + _("Missing or invalid 'multicast-table' array in query-rx-filter response")); return -1; } nTable = virJSONValueArraySize(table); @@ -3709,11 +3705,9 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValue *msg, "in query-rx-filter response")); return -1; } - if ((!(table = virJSONValueObjectGet(entry, "vlan-table"))) || - (!virJSONValueIsArray(table))) { + if ((!(table = virJSONValueObjectGetArray(entry, "vlan-table")))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing or invalid 'vlan-table' array " - "in query-rx-filter response")); + _("Missing or invalid 'vlan-table' array in query-rx-filter response")); return -1; } nTable = virJSONValueArraySize(table); @@ -8722,9 +8716,7 @@ qemuMonitorJSONExtractQueryStatsSchema(virJSONValue *json) if (!virJSONValueIsObject(obj)) continue; - stats = virJSONValueObjectGetArray(obj, "stats"); - - if (!virJSONValueIsArray(stats)) + if (!(stats = virJSONValueObjectGetArray(obj, "stats"))) continue; target_str = virJSONValueObjectGetString(obj, "target"); -- 2.39.1

Replace virJSONValueObjectGet + virJSONValueIsArray by the single API which returns only an array. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index fa2c0bf915..f20bc4e6a7 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1789,12 +1789,6 @@ qemuAgentGetFSInfoFillDisks(virJSONValue *jsondisks, size_t ndisks; size_t i; - if (!virJSONValueIsArray(jsondisks)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Malformed guest-get-fsinfo 'disk' data array")); - return -1; - } - ndisks = virJSONValueArraySize(jsondisks); if (ndisks) @@ -1846,15 +1840,9 @@ qemuAgentGetFSInfo(qemuAgent *agent, report_unsupported)) < 0) return rc; - if (!(data = virJSONValueObjectGet(reply, "return"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest-get-fsinfo reply was missing return data")); - goto cleanup; - } - - if (!virJSONValueIsArray(data)) { + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Malformed guest-get-fsinfo data array")); + _("guest-get-fsinfo reply was missing or not an array")); goto cleanup; } @@ -1934,9 +1922,9 @@ qemuAgentGetFSInfo(qemuAgent *agent, info_ret[i]->total_bytes = -1; } - if (!(disk = virJSONValueObjectGet(entry, "disk"))) { + if (!(disk = virJSONValueObjectGetArray(entry, "disk"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("'disk' missing in reply of guest-get-fsinfo")); + _("'disk' missing or not an array in reply of guest-get-fsinfo")); goto cleanup; } @@ -2067,16 +2055,9 @@ qemuAgentGetInterfaceAddresses(virDomainInterfacePtr **ifaces_ret, /* as well as IP address which - moreover - * can be presented multiple times */ - ip_addr_arr = virJSONValueObjectGet(iface_obj, "ip-addresses"); - if (!ip_addr_arr) + if (!(ip_addr_arr = virJSONValueObjectGetArray(iface_obj, "ip-addresses"))) return 0; - if (!virJSONValueIsArray(ip_addr_arr)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Malformed ip-addresses array")); - return -1; - } - /* If current iface already exists, continue with the count */ addrs_count = iface->naddrs; -- 2.39.1

Automatically free 'cpuinfo' and remove the cleanup label and ret variable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f3f64d3411..6154fe9bfe 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17585,8 +17585,7 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, virDomainVcpuDef *vcpu; qemuDomainVcpuPrivate *vcpupriv; size_t i; - int ret = -1; - virVcpuInfoPtr cpuinfo = NULL; + g_autofree virVcpuInfoPtr cpuinfo = NULL; g_autofree unsigned long long *cpuwait = NULL; g_autofree unsigned long long *cpudelay = NULL; qemuDomainObjPrivate *priv = dom->privateData; @@ -17615,8 +17614,7 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, virDomainDefGetVcpus(dom->def), NULL, 0) < 0) { virResetLastError(); - ret = 0; /* it's ok to be silent and go ahead */ - goto cleanup; + return 0; } if (HAVE_JOB(privflags) && qemuDomainRefreshStatsSchema(dom) == 0) { @@ -17634,7 +17632,7 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, if (virTypedParamListAddInt(params, cpuinfo[i].state, "vcpu.%u.state", cpuinfo[i].number) < 0) - goto cleanup; + return -1; /* stats below are available only if the VM is alive */ if (!virDomainObjIsActive(dom)) @@ -17642,15 +17640,15 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, if (virTypedParamListAddULLong(params, cpuinfo[i].cpuTime, "vcpu.%u.time", cpuinfo[i].number) < 0) - goto cleanup; + return -1; if (virTypedParamListAddULLong(params, cpuwait[i], "vcpu.%u.wait", cpuinfo[i].number) < 0) - goto cleanup; + return -1; if (virTypedParamListAddULLong(params, cpudelay[i], "vcpu.%u.delay", cpuinfo[i].number) < 0) - goto cleanup; + return -1; /* state below is extracted from the individual vcpu structs */ if (!(vcpu = virDomainDefGetVcpu(dom->def, cpuinfo[i].number))) @@ -17663,7 +17661,7 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, vcpupriv->halted == VIR_TRISTATE_BOOL_YES, "vcpu.%u.halted", cpuinfo[i].number) < 0) - goto cleanup; + return -1; } if (!queried_stats) @@ -17675,11 +17673,7 @@ qemuDomainGetStatsVcpu(virQEMUDriver *driver G_GNUC_UNUSED, qemuDomainAddStatsFromHashTable(stats, priv->statsSchema, prefix, params); } - ret = 0; - - cleanup: - VIR_FREE(cpuinfo); - return ret; + return 0; } #define QEMU_ADD_NET_PARAM(params, num, name, value) \ -- 2.39.1

On 2/2/23 17:10, Peter Krempa wrote:
Some more patches from old branches that I didn't get around finishing before.
Peter Krempa (5): virbitmap: Allow NULL bitmap in functions returning index of a set/clear bit qemuMonitorJSONQueryStats: Simplify logic to construct 'provider_list' qemu_monitor_json: Replace simplify fetching Array from JSON object qemu: agent: Use virJSONValueObjectGetArray qemuDomainGetStatsVcpu: Refactor cleanup
src/qemu/qemu_agent.c | 29 ++++--------------- src/qemu/qemu_driver.c | 22 ++++++--------- src/qemu/qemu_monitor_json.c | 54 ++++++++++++++---------------------- src/util/virbitmap.c | 9 ++++++ src/util/virbitmap.h | 9 ++---- 5 files changed, 46 insertions(+), 77 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník
-
Peter Krempa