None of the fields actually return negative values. The internal implementation of BlockAcctStats struct in qemu uses uint64_t and the last place using -1 in libvirt was in the HMP monitor code which was deleted. Change the internal type to unsigned long long and ensure that all public conversions don't overflow. Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- src/qemu/qemu_driver.c | 35 ++++++++++++++++++++++------------- src/qemu/qemu_monitor.h | 16 ++++++++-------- src/qemu/qemu_monitor_json.c | 2 +- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9315b78c48..ab41e51700 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11416,8 +11416,7 @@ qemuDomainBlockStatsGatherTotals(qemuBlockStatsPtr data, qemuBlockStatsPtr total) { #define QEMU_BLOCK_STAT_TOTAL(NAME) \ - if (data->NAME > 0) \ - total->NAME += data->NAME + total->NAME += data->NAME QEMU_BLOCK_STAT_TOTAL(wr_bytes); QEMU_BLOCK_STAT_TOTAL(wr_req); @@ -11573,10 +11572,14 @@ qemuDomainBlockStats(virDomainPtr dom, if (qemuDomainBlocksStatsGather(driver, vm, path, false, &blockstats) < 0) goto endjob; - stats->rd_req = blockstats->rd_req; - stats->rd_bytes = blockstats->rd_bytes; - stats->wr_req = blockstats->wr_req; - stats->wr_bytes = blockstats->wr_bytes; + if (VIR_ASSIGN_IS_OVERFLOW(stats->rd_req, blockstats->rd_req) || + VIR_ASSIGN_IS_OVERFLOW(stats->rd_bytes, blockstats->rd_bytes) || + VIR_ASSIGN_IS_OVERFLOW(stats->wr_req, blockstats->wr_req) || + VIR_ASSIGN_IS_OVERFLOW(stats->wr_bytes, blockstats->wr_bytes)) { + virReportError(VIR_ERR_OVERFLOW, "%s", _("statistic value too large")); + goto endjob; + } + /* qemu doesn't report the error count */ stats->errs = -1; @@ -11638,9 +11641,15 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, nstats = 0; #define QEMU_BLOCK_STATS_ASSIGN_PARAM(VAR, NAME) \ - if (nstats < *nparams && (blockstats->VAR) != -1) { \ + if (nstats < *nparams) { \ + long long tmp; \ + if (VIR_ASSIGN_IS_OVERFLOW(tmp, (blockstats->VAR))) { \ + virReportError(VIR_ERR_OVERFLOW, \ + _("value of '%s' is too large"), NAME); \ + goto endjob; \ + } \ if (virTypedParameterAssign(params + nstats, NAME, \ - VIR_TYPED_PARAM_LLONG, (blockstats->VAR)) < 0) \ + VIR_TYPED_PARAM_LLONG, tmp) < 0) \ goto endjob; \ nstats++; \ } @@ -21490,11 +21499,11 @@ do { \ char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; \ snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, \ "block.%zu.%s", num, name); \ - if (value >= 0 && virTypedParamsAddULLong(&(record)->params, \ - &(record)->nparams, \ - maxparams, \ - param_name, \ - value) < 0) \ + if (virTypedParamsAddULLong(&(record)->params, \ + &(record)->nparams, \ + maxparams, \ + param_name, \ + value) < 0) \ goto cleanup; \ } while (0) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 70000a1c72..321ca2b53a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -651,14 +651,14 @@ virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon); typedef struct _qemuBlockStats qemuBlockStats; typedef qemuBlockStats *qemuBlockStatsPtr; struct _qemuBlockStats { - long long rd_req; - long long rd_bytes; - long long wr_req; - long long wr_bytes; - long long rd_total_times; - long long wr_total_times; - long long flush_req; - long long flush_total_times; + unsigned long long rd_req; + unsigned long long rd_bytes; + unsigned long long wr_req; + unsigned long long wr_bytes; + unsigned long long rd_total_times; + unsigned long long wr_total_times; + unsigned long long flush_req; + unsigned long long flush_total_times; unsigned long long capacity; unsigned long long physical; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9be122a465..26cd9057e4 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2547,7 +2547,7 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev, #define QEMU_MONITOR_BLOCK_STAT_GET(NAME, VAR, MANDATORY) \ if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \ (*nstats)++; \ - if (virJSONValueObjectGetNumberLong(stats, NAME, &VAR) < 0) { \ + if (virJSONValueObjectGetNumberUlong(stats, NAME, &VAR) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ _("cannot read %s statistic"), NAME); \ return NULL; \ -- 2.21.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list