From: Davide Marro <dmarro@redhat.com> QEMU uses uint64 values for block latency histogram boundaries, while libvirt currently stores and parses them as unsigned int values. This limits histogram boundaries to UINT32_MAX nanoseconds and prevents using larger values supported by QEMU. Use unsigned long long for latency histogram boundaries throughout the domain configuration and QEMU monitor code. Update the XML schema and parser accordingly, and serialize the values using the 64-bit JSON number helper. Extend the XML and QEMU monitor tests with a 5000000000 nanosecond boundary to verify values above UINT32_MAX are preserved. Resolves: https://gitlab.com/libvirt/libvirt/-/work_items/919 Signed-off-by: Davide Marro <dmarro@redhat.com> --- src/conf/domain_conf.c | 12 ++--- src/conf/domain_conf.h | 10 ++-- src/conf/schemas/domaincommon.rng | 2 +- src/qemu/qemu_monitor.c | 10 ++-- src/qemu/qemu_monitor.h | 10 ++-- src/qemu/qemu_monitor_json.c | 14 +++--- src/qemu/qemu_monitor_json.h | 10 ++-- tests/qemumonitorjsontest.c | 46 ++++++++++++++++--- ...isk-statistics-intervals.x86_64-latest.xml | 1 + .../disk-statistics-intervals.xml | 1 + 10 files changed, 75 insertions(+), 41 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1327793b0a..94a1f8bd8e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8427,7 +8427,7 @@ virDomainDiskDefDriverParseXMLHistogramOne(virDomainDiskDef *def, xmlNodePtr cur) { g_autofree char *histogram_type = NULL; - unsigned int **histogram_config = NULL; + unsigned long long **histogram_config = NULL; g_autoptr(GPtrArray) binNodes = virXMLNodeGetSubelementList(cur, "bin"); size_t nbins = 0; size_t i; @@ -8463,12 +8463,12 @@ virDomainDiskDefDriverParseXMLHistogramOne(virDomainDiskDef *def, return -1; } - *histogram_config = g_new0(unsigned int, binNodes->len + 1); + *histogram_config = g_new0(unsigned long long, binNodes->len + 1); for (i = 0; i < binNodes->len; i++) { - unsigned int val; + unsigned long long val; - if (virXMLPropUInt(g_ptr_array_index(binNodes, i), + if (virXMLPropULongLong(g_ptr_array_index(binNodes, i), "start", 10, VIR_XML_PROP_REQUIRED, &val) < 0) @@ -24571,7 +24571,7 @@ virDomainDiskDefFormatThrottleFilters(virBuffer *buf, static void virDomainDiskDefFormatDriverHistogram(virBuffer *buf, const char *type, - unsigned int *bins) + unsigned long long *bins) { g_auto(virBuffer) histogramAttrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) histogramChildBuf = VIR_BUFFER_INIT_CHILD(buf); @@ -24586,7 +24586,7 @@ virDomainDiskDefFormatDriverHistogram(virBuffer *buf, virBufferAddLit(&histogramChildBuf, "<bin start='0'/>\n"); for (; *bins > 0; bins++) - virBufferAsprintf(&histogramChildBuf, "<bin start='%u'/>\n", *bins); + virBufferAsprintf(&histogramChildBuf, "<bin start='%llu'/>\n", *bins); virXMLFormatElement(buf, "latency-histogram", &histogramAttrBuf, &histogramChildBuf); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f7cbe7bf1d..eadb5a49a4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -599,11 +599,11 @@ struct _virDomainDiskDef { unsigned int *statistics; /* Optional, zero terminated list of intervals to collect statistics for */ /* optional zero terminated lists of bin boundaries for latency histograms */ - unsigned int *histogram_boundaries; - unsigned int *histogram_boundaries_read; - unsigned int *histogram_boundaries_write; - unsigned int *histogram_boundaries_zone; - unsigned int *histogram_boundaries_flush; + unsigned long long *histogram_boundaries; + unsigned long long *histogram_boundaries_read; + unsigned long long *histogram_boundaries_write; + unsigned long long *histogram_boundaries_zone; + unsigned long long *histogram_boundaries_flush; virDomainDiskDetectZeroes detect_zeroes; virTristateSwitch discard_no_unref; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 0c0a3597a9..d1b10c1c5f 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2845,7 +2845,7 @@ <oneOrMore> <element name='bin'> <attribute name='start'> - <ref name="unsignedInt"/> + <ref name="unsignedLong"/> </attribute> </element> </oneOrMore> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 21df06ebd8..2c86cd1971 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4558,11 +4558,11 @@ qemuMonitorBlockdevSetActive(qemuMonitor *mon, int qemuMonitorBlockLatencyHistogramSet(qemuMonitor *mon, const char *id, - unsigned int *boundaries, - unsigned int *boundaries_read, - unsigned int *boundaries_write, - unsigned int *boundaries_zone, - unsigned int *boundaries_flush) + unsigned long long *boundaries, + unsigned long long *boundaries_read, + unsigned long long *boundaries_write, + unsigned long long *boundaries_zone, + unsigned long long *boundaries_flush) { QEMU_CHECK_MONITOR(mon); VIR_DEBUG("id='%s'", id); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index e560618bf4..e9b6a25f0c 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1988,11 +1988,11 @@ qemuMonitorBlockdevSetActive(qemuMonitor *mon, int qemuMonitorBlockLatencyHistogramSet(qemuMonitor *mon, const char *id, - unsigned int *boundaries, - unsigned int *boundaries_read, - unsigned int *boundaries_write, - unsigned int *boundaries_zone, - unsigned int *boundaries_flush); + unsigned long long *boundaries, + unsigned long long *boundaries_read, + unsigned long long *boundaries_write, + unsigned long long *boundaries_zone, + unsigned long long *boundaries_flush); int qemuMonitorAnnounceSelf(qemuMonitor *mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7e3249cd47..8e88dc9442 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9176,7 +9176,7 @@ qemuMonitorJSONBlockdevSetActive(qemuMonitor *mon, static virJSONValue * -qemuMonitorJSONBlockLatencyHistogramBoundary(unsigned int *bound) +qemuMonitorJSONBlockLatencyHistogramBoundary(unsigned long long *bound) { g_autoptr(virJSONValue) ret = virJSONValueNewArray(); @@ -9184,7 +9184,7 @@ qemuMonitorJSONBlockLatencyHistogramBoundary(unsigned int *bound) return NULL; for (; *bound > 0; bound++) { - g_autoptr(virJSONValue) val = virJSONValueNewNumberUint(*bound); + g_autoptr(virJSONValue) val = virJSONValueNewNumberUlong(*bound); /* the only error is if the first argument is not an array */ ignore_value(virJSONValueArrayAppend(ret, &val)); @@ -9197,11 +9197,11 @@ qemuMonitorJSONBlockLatencyHistogramBoundary(unsigned int *bound) int qemuMonitorJSONBlockLatencyHistogramSet(qemuMonitor *mon, const char *id, - unsigned int *boundaries, - unsigned int *boundaries_read, - unsigned int *boundaries_write, - unsigned int *boundaries_zone, - unsigned int *boundaries_flush) + unsigned long long *boundaries, + unsigned long long *boundaries_read, + unsigned long long *boundaries_write, + unsigned long long *boundaries_zone, + unsigned long long *boundaries_flush) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 3b7b04a11d..63eded7ecb 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -818,11 +818,11 @@ qemuMonitorJSONBlockdevSetActive(qemuMonitor *mon, int qemuMonitorJSONBlockLatencyHistogramSet(qemuMonitor *mon, const char *id, - unsigned int *boundaries, - unsigned int *boundaries_read, - unsigned int *boundaries_write, - unsigned int *boundaries_zone, - unsigned int *boundaries_flush); + unsigned long long *boundaries, + unsigned long long *boundaries_read, + unsigned long long *boundaries_write, + unsigned long long *boundaries_zone, + unsigned long long *boundaries_flush); int qemuMonitorJSONAnnounceSelf(qemuMonitor *mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 70f03afb81..a963029c69 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1130,13 +1130,45 @@ GEN_TEST_FUNC(qemuMonitorJSONSetAction, GEN_TEST_FUNC(qemuMonitorJSONSetLaunchSecurityState, "sev_secret_header", "sev_secret", 0, true) -unsigned int testHistogramBoundaries[] = {10, 30, 50, 0}; -GEN_TEST_FUNC(qemuMonitorJSONBlockLatencyHistogramSet, "devid", - testHistogramBoundaries, - testHistogramBoundaries, - testHistogramBoundaries, - testHistogramBoundaries, - testHistogramBoundaries) +static int +testQemuMonitorJSONqemuMonitorJSONBlockLatencyHistogramSet(const void *opaque) +{ + const testQemuMonitorJSONSimpleFuncData *data = opaque; + virDomainXMLOption *xmlopt = data->xmlopt; + unsigned long long boundaries[] = {10, 30, 5000000000ULL, 0}; + g_autoptr(qemuMonitorTest) test = NULL; + + if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) + return -1; + + if (qemuMonitorTestAddItemVerbatim(test, + "{" + "\"execute\":\"block-latency-histogram-set\"," + "\"arguments\":{" + "\"id\":\"devid\"," + "\"boundaries\":[10,30,5000000000]," + "\"boundaries-read\":[10,30,5000000000]," + "\"boundaries-write\":[10,30,5000000000]," + "\"boundaries-zap\":[10,30,5000000000]," + "\"boundaries-flush\":[10,30,5000000000]" + "}," + "\"id\":\"libvirt-1\"" + "}", + NULL, + "{\"return\":{}}") < 0) + return -1; + + if (qemuMonitorJSONBlockLatencyHistogramSet(qemuMonitorTestGetMonitor(test), + "devid", + boundaries, + boundaries, + boundaries, + boundaries, + boundaries) < 0) + return -1; + + return 0; +} static int testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) diff --git a/tests/qemuxmlconfdata/disk-statistics-intervals.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-statistics-intervals.x86_64-latest.xml index d02f954073..d245eedb5b 100644 --- a/tests/qemuxmlconfdata/disk-statistics-intervals.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/disk-statistics-intervals.x86_64-latest.xml @@ -26,6 +26,7 @@ <bin start='0'/> <bin start='20'/> <bin start='30'/> + <bin start='5000000000'/> </latency-histogram> </statistics> </driver> diff --git a/tests/qemuxmlconfdata/disk-statistics-intervals.xml b/tests/qemuxmlconfdata/disk-statistics-intervals.xml index 5f9e9470d7..8b1e7481b2 100644 --- a/tests/qemuxmlconfdata/disk-statistics-intervals.xml +++ b/tests/qemuxmlconfdata/disk-statistics-intervals.xml @@ -23,6 +23,7 @@ <bin start='0'/> <bin start='20'/> <bin start='30'/> + <bin start='5000000000'/> </latency-histogram> </statistics> </driver> -- 2.54.0 (Apple Git-157)