[libvirt] [PATCH 0/4] bulk stats: QEMU implementation polishing
This patchset does polishing on the QEMU bulk stats implementation. The main issue this patchset addresses is that, unless a critical error is found, bulk stats should be silent and ignore errors. To do so, virResetLastError() is used in a few places, but this is not enough since errors are logged anyway. A better approach is to avoid to report error entirely. The patchset is organized as follows: - patches 1 to 3 enhances the functions used in the bulk stats path(s) adding a 'bool report' flag, to let the caller optionally suppress error reporting. - patch 4 is a general polishing patch which reduces repetition of the code in the block stats collection. Francesco Romani (4): qemu: make qemuDomainHelperGetVcpus silent make virNetInterfaceStats silent qemu: make qemuMonitorGetAllBlockStatsInfo silent qemu: json monitor: reduce duplicated code src/lxc/lxc_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/qemu/qemu_driver.c | 32 +++++----- src/qemu/qemu_monitor.c | 12 ++-- src/qemu/qemu_monitor.h | 3 +- src/qemu/qemu_monitor_json.c | 136 ++++++++++++++++++++----------------------- src/qemu/qemu_monitor_json.h | 3 +- src/util/virstats.c | 21 ++++--- src/util/virstats.h | 3 +- src/xen/xen_hypervisor.c | 2 +- 10 files changed, 111 insertions(+), 105 deletions(-) -- 1.9.3
The commit 74c066df4d8 introduced an helper to factor a code path which is shared between the existing API and the new bulk stats API. In the bulk stats path errors must be silenced unless critical (e.g. memory allocation failure). To address this need, this patch adds an argument to disable error reporting. --- src/qemu/qemu_driver.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f28082f..dc8d6c3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1381,8 +1381,10 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss, static int -qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, - unsigned char *cpumaps, int maplen) +qemuDomainHelperGetVcpus(virDomainObjPtr vm, + virVcpuInfoPtr info, int maxinfo, + unsigned char *cpumaps, int maplen, + bool report) { int maxcpu, hostcpus; size_t i, v; @@ -1412,8 +1414,10 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, NULL, vm->pid, priv->vcpupids[i]) < 0) { - virReportSystemError(errno, "%s", - _("cannot get vCPU placement & pCPU time")); + if (report) + virReportSystemError(errno, "%s", + _("cannot get vCPU placement " + "& pCPU time")); return -1; } } @@ -1440,8 +1444,9 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, virBitmapFree(map); } } else { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cpu affinity is not available")); + if (report) + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cpu affinity is not available")); return -1; } } @@ -5044,7 +5049,7 @@ qemuDomainGetVcpus(virDomainPtr dom, goto cleanup; } - ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen); + ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen, true); cleanup: if (vm) @@ -17530,8 +17535,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, return -1; if (qemuDomainHelperGetVcpus(dom, cpuinfo, dom->def->vcpus, - NULL, 0) < 0) { - virResetLastError(); + NULL, 0, false) < 0) { ret = 0; /* it's ok to be silent and go ahead */ goto cleanup; } -- 1.9.3
On Fri, Sep 19, 2014 at 11:44:20AM +0200, Francesco Romani wrote:
The commit 74c066df4d8 introduced an helper to factor a code path which is shared between the existing API and the new bulk stats API. In the bulk stats path errors must be silenced unless critical (e.g. memory allocation failure).
To address this need, this patch adds an argument to disable error reporting.
I'm really don't like the approach of adding 'bool reportError' flags to internal functions as it doesn't scale. These flags end up creeping out across more & more of the code base and different callers are going to disagree about which errors should be ignored and which not. Having the callers use virResetError is preferrable IMHO as it avoids polluting countless internal functions with ill defined args. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 09/19/14 12:29, Daniel P. Berrange wrote:
On Fri, Sep 19, 2014 at 11:44:20AM +0200, Francesco Romani wrote:
The commit 74c066df4d8 introduced an helper to factor a code path which is shared between the existing API and the new bulk stats API. In the bulk stats path errors must be silenced unless critical (e.g. memory allocation failure).
To address this need, this patch adds an argument to disable error reporting.
I'm really don't like the approach of adding 'bool reportError' flags to internal functions as it doesn't scale. These flags end up creeping out across more & more of the code base and different callers are going to disagree about which errors should be ignored and which not. Having the callers use virResetError is preferrable IMHO as it avoids polluting
In Eric's last review he advised to change from virResetError to the boolean flag so that logs are not polluted. Without this patch, the code is in the state as you suggest, but it contradicts Eric's stance.
countless internal functions with ill defined args.
Regards, Daniel
Peter
On 09/19/2014 04:57 AM, Peter Krempa wrote:
On 09/19/14 12:29, Daniel P. Berrange wrote:
On Fri, Sep 19, 2014 at 11:44:20AM +0200, Francesco Romani wrote:
The commit 74c066df4d8 introduced an helper to factor a code path which is shared between the existing API and the new bulk stats API. In the bulk stats path errors must be silenced unless critical (e.g. memory allocation failure).
To address this need, this patch adds an argument to disable error reporting.
I'm really don't like the approach of adding 'bool reportError' flags to internal functions as it doesn't scale. These flags end up creeping out across more & more of the code base and different callers are going to disagree about which errors should be ignored and which not. Having the callers use virResetError is preferrable IMHO as it avoids polluting
In Eric's last review he advised to change from virResetError to the boolean flag so that logs are not polluted. Without this patch, the code is in the state as you suggest, but it contradicts Eric's stance.
countless internal functions with ill defined args.
I think it all boils down to a question of how frequently will these functions be called, and how frequently will the logs be populated with messages about an ignored failure. Is this a case where we are better off waiting until the complaints happen? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
virNetInterfaceStats is now used in the bulk stats path, and in this path errors must be silenced unless critical (e.g. memory allocation failure). To address this need, this patch adds an argument to disable error reporting. --- src/lxc/lxc_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/qemu/qemu_driver.c | 6 ++---- src/util/virstats.c | 21 +++++++++++++-------- src/util/virstats.h | 3 ++- src/xen/xen_hypervisor.c | 2 +- 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index c3cd62c..2eaeb22 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3112,7 +3112,7 @@ lxcDomainInterfaceStats(virDomainPtr dom, } if (ret == 0) - ret = virNetInterfaceStats(path, stats); + ret = virNetInterfaceStats(path, stats, true); else virReportError(VIR_ERR_INVALID_ARG, _("Invalid path, '%s' is not a known interface"), path); diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b62273a..2b39f36 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -2010,7 +2010,7 @@ openvzDomainInterfaceStats(virDomainPtr dom, } if (ret == 0) - ret = virNetInterfaceStats(path, stats); + ret = virNetInterfaceStats(path, stats, true); else virReportError(VIR_ERR_INVALID_ARG, _("invalid path, '%s' is not a known interface"), path); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc8d6c3..3ff226f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9884,7 +9884,7 @@ qemuDomainInterfaceStats(virDomainPtr dom, } if (ret == 0) - ret = virNetInterfaceStats(path, stats); + ret = virNetInterfaceStats(path, stats, true); else virReportError(VIR_ERR_INVALID_ARG, _("invalid path, '%s' is not a known interface"), path); @@ -17634,10 +17634,8 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, QEMU_ADD_NAME_PARAM(record, maxparams, "net", i, dom->def->nets[i]->ifname); - if (virNetInterfaceStats(dom->def->nets[i]->ifname, &tmp) < 0) { - virResetLastError(); + if (virNetInterfaceStats(dom->def->nets[i]->ifname, &tmp, false) < 0) continue; - } QEMU_ADD_NET_PARAM(record, maxparams, i, "rx.bytes", tmp.rx_bytes); diff --git a/src/util/virstats.c b/src/util/virstats.c index c4725ed..496393b 100644 --- a/src/util/virstats.c +++ b/src/util/virstats.c @@ -51,7 +51,8 @@ #ifdef __linux__ int virNetInterfaceStats(const char *path, - virDomainInterfaceStatsPtr stats) + virDomainInterfaceStatsPtr stats, + bool report) { int path_len; FILE *fp; @@ -115,14 +116,16 @@ virNetInterfaceStats(const char *path, } VIR_FORCE_FCLOSE(fp); - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("/proc/net/dev: Interface not found")); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("/proc/net/dev: Interface not found")); return -1; } #elif defined(HAVE_GETIFADDRS) && defined(AF_LINK) int virNetInterfaceStats(const char *path, - virDomainInterfaceStatsPtr stats) + virDomainInterfaceStatsPtr stats, + bool report) { struct ifaddrs *ifap, *ifa; struct if_data *ifd; @@ -158,7 +161,7 @@ virNetInterfaceStats(const char *path, } } - if (ret < 0) + if (ret < 0 && report) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Interface not found")); @@ -168,10 +171,12 @@ virNetInterfaceStats(const char *path, #else int virNetInterfaceStats(const char *path ATTRIBUTE_UNUSED, - virDomainInterfaceStatsPtr stats ATTRIBUTE_UNUSED) + virDomainInterfaceStatsPtr stats ATTRIBUTE_UNUSED, + bool report) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("interface stats not implemented on this platform")); + if (report) + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("interface stats not implemented on this platform")); return -1; } diff --git a/src/util/virstats.h b/src/util/virstats.h index d2c6b64..df993cd 100644 --- a/src/util/virstats.h +++ b/src/util/virstats.h @@ -26,6 +26,7 @@ # include "internal.h" extern int virNetInterfaceStats(const char *path, - virDomainInterfaceStatsPtr stats); + virDomainInterfaceStatsPtr stats, + bool report); #endif /* __STATS_LINUX_H__ */ diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index d3d4aea..8b257f7 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1470,7 +1470,7 @@ xenHypervisorDomainInterfaceStats(virDomainDefPtr def, return -1; } - return virNetInterfaceStats(path, stats); + return virNetInterfaceStats(path, stats, true); #else virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("/proc/net/dev: Interface not found")); -- 1.9.3
The commit 290e3c6b07a introduced an helper to factor a code path which is shared between the existing API and the new bulk stats API. In the bulk stats path errors must be silenced unless critical (e.g. memory allocation failure). To address this need, this patch adds an argument to disable error reporting. --- src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_monitor.c | 12 ++++-- src/qemu/qemu_monitor.h | 3 +- src/qemu/qemu_monitor_json.c | 87 ++++++++++++++++++++++++++------------------ src/qemu/qemu_monitor_json.h | 3 +- 5 files changed, 65 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3ff226f..36b96e5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17696,12 +17696,12 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, dom); nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL, - stats, nstats); + stats, nstats, + false); qemuDomainObjExitMonitor(driver, dom); if (nstats < 0) { - virResetLastError(); ret = 0; /* still ok, again go ahead silently */ goto cleanup; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 10f51c5..cac48d7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1765,17 +1765,21 @@ int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, qemuBlockStatsPtr stats, - int nstats) + int nstats, + bool report) { int ret; VIR_DEBUG("mon=%p dev=%s", mon, dev_name); if (mon->json) { ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, - stats, nstats); + stats, nstats, + report); } else { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("unable to query all block stats with this QEMU")); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to query all block stats " + "with this QEMU")); return -1; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 2a43a3c..1eeb7b9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -363,7 +363,8 @@ struct _qemuBlockStats { int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, qemuBlockStatsPtr stats, - int nstats) + int nstats, + bool report) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 857edf1..4810bd3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1749,7 +1749,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, if (flush_total_times) *flush_total_times = -1; - if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, &stats, 1) != 1) + if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, + &stats, 1, true) != 1) goto cleanup; *rd_req = stats.rd_req; @@ -1777,7 +1778,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, qemuBlockStatsPtr bstats, - int nstats) + int nstats, + bool report) { int ret, count; size_t i; @@ -1802,8 +1804,9 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, devices = virJSONValueObjectGet(reply, "return"); if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats reply was missing device list")); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats reply was missing device list")); goto cleanup; } @@ -1812,9 +1815,10 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virJSONValuePtr dev = virJSONValueArrayGet(devices, i); virJSONValuePtr stats; if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats device entry was not " - "in expected format")); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not " + "in expected format")); goto cleanup; } @@ -1824,9 +1828,10 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, if (dev_name) { const char *thisdev = virJSONValueObjectGetString(dev, "device"); if (!thisdev) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats device entry was not " - "in expected format")); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not " + "in expected format")); goto cleanup; } @@ -1843,46 +1848,52 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL || stats->type != VIR_JSON_TYPE_OBJECT) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats stats entry was not " - "in expected format")); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats stats entry was not " + "in expected format")); goto cleanup; } if (virJSONValueObjectGetNumberLong(stats, "rd_bytes", &bstats->rd_bytes) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_bytes"); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "rd_bytes"); goto cleanup; } if (virJSONValueObjectGetNumberLong(stats, "rd_operations", &bstats->rd_req) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_operations"); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "rd_operations"); goto cleanup; } if (virJSONValueObjectHasKey(stats, "rd_total_time_ns") && (virJSONValueObjectGetNumberLong(stats, "rd_total_time_ns", &bstats->rd_total_times) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_total_time_ns"); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "rd_total_time_ns"); goto cleanup; } if (virJSONValueObjectGetNumberLong(stats, "wr_bytes", &bstats->wr_bytes) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_bytes"); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "wr_bytes"); goto cleanup; } if (virJSONValueObjectGetNumberLong(stats, "wr_operations", &bstats->wr_req) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_operations"); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "wr_operations"); goto cleanup; } if (virJSONValueObjectHasKey(stats, "wr_total_time_ns") && @@ -1896,17 +1907,19 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, if (virJSONValueObjectHasKey(stats, "flush_operations") && (virJSONValueObjectGetNumberLong(stats, "flush_operations", &bstats->flush_req) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "flush_operations"); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "flush_operations"); goto cleanup; } if (virJSONValueObjectHasKey(stats, "flush_total_time_ns") && (virJSONValueObjectGetNumberLong(stats, "flush_total_time_ns", &bstats->flush_total_times) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "flush_total_time_ns"); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot read %s statistic"), + "flush_total_time_ns"); goto cleanup; } @@ -1918,8 +1931,10 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, } if (dev_name && !count) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find statistics for device '%s'"), dev_name); + if (report) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find statistics for device '%s'"), + dev_name); goto cleanup; } diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 3daa759..924c79c 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -82,7 +82,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, qemuBlockStatsPtr stats, - int nstats); + int nstats, + bool report); int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams); int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon, -- 1.9.3
This patch replaces repetitive blocks of code with a couple of macros for the sake of clarity. There are no changes in behaviour. --- src/qemu/qemu_monitor_json.c | 129 ++++++++++++++++++------------------------- 1 file changed, 53 insertions(+), 76 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4810bd3..4fa72c9 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1775,6 +1775,25 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, } +#define QEMU_MONITOR_JSON_MALFORMED_ENTRY(kind) do { \ + if (report) \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + _("blockstats %s entry was not " \ + "in expected format"), \ + kind); \ + goto cleanup; \ +} while (0) + + +#define QEMU_MONITOR_JSON_MISSING_STAT(statistic) do { \ + if (report) \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + _("cannot read %s statistic"), \ + statistic); \ + goto cleanup; \ +} while (0) + + int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, qemuBlockStatsPtr bstats, @@ -1814,26 +1833,16 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, for (i = 0; i < virJSONValueArraySize(devices) && count < nstats; i++) { virJSONValuePtr dev = virJSONValueArrayGet(devices, i); virJSONValuePtr stats; - if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats device entry was not " - "in expected format")); - goto cleanup; - } + if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) + QEMU_MONITOR_JSON_MALFORMED_ENTRY("device"); /* If dev_name is specified, we are looking for a specific device, * so we must be stricter. */ if (dev_name) { const char *thisdev = virJSONValueObjectGetString(dev, "device"); - if (!thisdev) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats device entry was not " - "in expected format")); - goto cleanup; - } + if (!thisdev) + QEMU_MONITOR_JSON_MALFORMED_ENTRY("device"); /* New QEMU has separate names for host & guest side of the disk * and libvirt gives the host side a 'drive-' prefix. The passed @@ -1847,81 +1856,44 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, } if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL || - stats->type != VIR_JSON_TYPE_OBJECT) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("blockstats stats entry was not " - "in expected format")); - goto cleanup; - } + stats->type != VIR_JSON_TYPE_OBJECT) + QEMU_MONITOR_JSON_MALFORMED_ENTRY("stats"); if (virJSONValueObjectGetNumberLong(stats, "rd_bytes", - &bstats->rd_bytes) < 0) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_bytes"); - goto cleanup; - } + &bstats->rd_bytes) < 0) + QEMU_MONITOR_JSON_MISSING_STAT("rd_bytes"); + if (virJSONValueObjectGetNumberLong(stats, "rd_operations", - &bstats->rd_req) < 0) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_operations"); - goto cleanup; - } + &bstats->rd_req) < 0) + QEMU_MONITOR_JSON_MISSING_STAT("rd_operations"); + if (virJSONValueObjectHasKey(stats, "rd_total_time_ns") && (virJSONValueObjectGetNumberLong(stats, "rd_total_time_ns", - &bstats->rd_total_times) < 0)) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "rd_total_time_ns"); - goto cleanup; - } + &bstats->rd_total_times) < 0)) + QEMU_MONITOR_JSON_MISSING_STAT("rd_total_time_ns"); + if (virJSONValueObjectGetNumberLong(stats, "wr_bytes", - &bstats->wr_bytes) < 0) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_bytes"); - goto cleanup; - } + &bstats->wr_bytes) < 0) + QEMU_MONITOR_JSON_MISSING_STAT("wr_bytes"); + if (virJSONValueObjectGetNumberLong(stats, "wr_operations", - &bstats->wr_req) < 0) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_operations"); - goto cleanup; - } + &bstats->wr_req) < 0) + QEMU_MONITOR_JSON_MISSING_STAT("wr_operations"); + if (virJSONValueObjectHasKey(stats, "wr_total_time_ns") && (virJSONValueObjectGetNumberLong(stats, "wr_total_time_ns", - &bstats->wr_total_times) < 0)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "wr_total_time_ns"); - goto cleanup; - } + &bstats->wr_total_times) < 0)) + QEMU_MONITOR_JSON_MISSING_STAT("wr_total_time_ns"); + if (virJSONValueObjectHasKey(stats, "flush_operations") && (virJSONValueObjectGetNumberLong(stats, "flush_operations", - &bstats->flush_req) < 0)) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "flush_operations"); - goto cleanup; - } + &bstats->flush_req) < 0)) + QEMU_MONITOR_JSON_MISSING_STAT("flush_operations"); + if (virJSONValueObjectHasKey(stats, "flush_total_time_ns") && (virJSONValueObjectGetNumberLong(stats, "flush_total_time_ns", - &bstats->flush_total_times) < 0)) { - if (report) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s statistic"), - "flush_total_time_ns"); - goto cleanup; - } + &bstats->flush_total_times) < 0)) + QEMU_MONITOR_JSON_MISSING_STAT("flush_total_time_ns"); count++; bstats++; @@ -1947,6 +1919,11 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, } +#undef QEMU_MONITOR_JSON_MISSING_STAT + +#undef QEMU_MONITOR_JSON_MALFORMED_ENTRY + + int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon, int *nparams) { -- 1.9.3
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Francesco Romani -
Peter Krempa