[PATCH 0/9] qemu: add support for block latency histogram collection
This patchset introduces configuration of the block latency histogram feature in qemu as well as exposes the histograms via bulk domain stats. The feature was in qemu for a long time so no capabilities are needed. Peter Krempa (9): qemu: Extract disk setup done via QMP into a separate helper qemu: process: Rename 'qemuProcessSetupDiskThrottling' to 'qemuProcessSetupDisks' qemu: monitor: Extract block latency histogram stats into 'qemuBlockStats' Expose latency histograms via 'virConnectGetAllDomainStats' qemu: monitor: Add handlers for 'block-latency-histogram-set' docs: formatdomain: Fix indentation of docs for <disk><driver><statistics> element docs: formatdomain: Reword section about the '<statistics>' element under disk driver Introduce support for disk operation latency histogram collection qemu: Setup disk latency histograms on startup/hotplug/update docs/formatdomain.rst | 77 ++++++++-- docs/manpages/virsh.rst | 7 + include/libvirt/libvirt-domain.h | 113 +++++++++++++++ src/conf/domain_conf.c | 133 +++++++++++++++++- src/conf/domain_conf.h | 7 + src/conf/schemas/domaincommon.rng | 37 ++++- src/qemu/qemu_domain.c | 17 +++ src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 43 ++++++ src/qemu/qemu_hotplug.c | 52 ++++--- src/qemu/qemu_monitor.c | 41 ++++++ src/qemu/qemu_monitor.h | 27 ++++ src/qemu/qemu_monitor_json.c | 115 +++++++++++++++ src/qemu/qemu_monitor_json.h | 9 ++ src/qemu/qemu_process.c | 63 ++++++--- src/qemu/qemu_process.h | 3 + tests/qemumonitorjsontest.c | 9 ++ ...isk-statistics-intervals.x86_64-latest.xml | 29 ++++ .../disk-statistics-intervals.xml | 25 ++++ 19 files changed, 748 insertions(+), 62 deletions(-) -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> Introduce 'qemuProcessSetupDiskPropsRuntime' helper function which will collect all code used for runtime setup of a disk. This is currently old-style throttling. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 23 +++++------------------ src/qemu/qemu_process.c | 37 +++++++++++++++++++++++++++---------- src/qemu/qemu_process.h | 3 +++ 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f786248e70..ffd7795b66 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -567,13 +567,8 @@ qemuDomainChangeMediaBlockdev(virDomainObj *vm, } /* set throttling for the new image */ - if (rc == 0 && - !virStorageSourceIsEmpty(newsrc) && - qemuDiskConfigBlkdeviotuneEnabled(disk)) { - rc = qemuMonitorSetBlockIoThrottle(priv->mon, - diskPriv->qomName, - &disk->blkdeviotune); - } + if (rc == 0) + rc = qemuProcessSetupDiskPropsRuntime(priv->mon, disk); if (rc == 0) rc = qemuMonitorBlockdevTrayClose(priv->mon, diskPriv->qomName); @@ -793,21 +788,13 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm, if (rc == 0) rc = qemuMonitorAddDeviceProps(priv->mon, &devprops); - /* Setup throttling of disk via block_set_io_throttle QMP command. This - * is a hack until the 'throttle' blockdev driver will support modification - * of the trhottle group. See also qemuProcessSetupDiskThrottlingBlockdev. - * As there isn't anything sane to do if this fails, let's just return - * success. - */ if (rc == 0) { qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); g_autoptr(GHashTable) blockinfo = NULL; - if (qemuDiskConfigBlkdeviotuneEnabled(disk)) { - if (qemuMonitorSetBlockIoThrottle(priv->mon, diskPriv->qomName, - &disk->blkdeviotune) < 0) - VIR_WARN("failed to set blkdeviotune for '%s' of '%s'", disk->dst, vm->def->name); - } + /* There isn't anything sane to do if this fails (rollback would + * require hot-unplug), let's just return success. */ + ignore_value(qemuProcessSetupDiskPropsRuntime(priv->mon, disk)); if ((blockinfo = qemuMonitorGetBlockInfo(priv->mon))) { struct qemuDomainDiskInfo *diskinfo; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c5b2a5fda8..8708c2d66b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7936,6 +7936,32 @@ qemuProcessGenID(virDomainObj *vm, } +/** + * qemuProcessSetupDiskPropsRuntime: + * @mon: qemu monitor object + * @disk: disk definition + * + * This function expects that caller already entered 'monitor' context. + * + * Sets up disk properties which are only possible to be set in runtime. + */ +int +qemuProcessSetupDiskPropsRuntime(qemuMonitor *mon, + virDomainDiskDef *disk) +{ + if (virStorageSourceIsEmpty(disk->src)) + return 0; + + if (qemuDiskConfigBlkdeviotuneEnabled(disk) && + qemuMonitorSetBlockIoThrottle(mon, + QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName, + &disk->blkdeviotune) < 0) + return -1; + + return 0; +} + + /** * qemuProcessSetupDiskThrottling: * @@ -7959,16 +7985,7 @@ qemuProcessSetupDiskThrottling(virDomainObj *vm, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDef *disk = vm->def->disks[i]; - /* Setting throttling for empty drives fails */ - if (virStorageSourceIsEmpty(disk->src)) - continue; - - if (!qemuDiskConfigBlkdeviotuneEnabled(disk)) - continue; - - if (qemuMonitorSetBlockIoThrottle(qemuDomainGetMonitor(vm), - QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName, - &disk->blkdeviotune) < 0) + if (qemuProcessSetupDiskPropsRuntime(qemuDomainGetMonitor(vm), disk) < 0) goto cleanup; } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 426e11d79e..df78b00abb 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -146,6 +146,9 @@ int qemuProcessPrepareHostStorageSourceChain(virDomainObj *vm, int qemuProcessPrepareHostStorageDisk(virDomainObj *vm, virDomainDiskDef *disk); +int qemuProcessSetupDiskPropsRuntime(qemuMonitor *mon, + virDomainDiskDef *disk); + int qemuProcessDeleteThreadContext(virDomainObj *vm); int qemuProcessLaunch(virConnectPtr conn, -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> Rename the runtime disk option setup function to be universal. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8708c2d66b..411c5b3b88 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7963,21 +7963,19 @@ qemuProcessSetupDiskPropsRuntime(qemuMonitor *mon, /** - * qemuProcessSetupDiskThrottling: + * qemuProcessSetupDisks: * - * Sets up disk trottling for -blockdev via block_set_io_throttle monitor - * command. This hack should be replaced by proper use of the 'throttle' - * blockdev driver in qemu once it will support changing of the throttle group. - * Same hack is done in qemuDomainAttachDiskGeneric. + * Sets up disk settings available only at runtime: + * - trottling for -blockdev via block_set_io_throttle QMP command */ static int -qemuProcessSetupDiskThrottling(virDomainObj *vm, - virDomainAsyncJob asyncJob) +qemuProcessSetupDisks(virDomainObj *vm, + virDomainAsyncJob asyncJob) { size_t i; int ret = -1; - VIR_DEBUG("Setting up disk throttling for -blockdev via block_set_io_throttle"); + VIR_DEBUG("Setting up disk config via runtime commands"); if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0) return -1; @@ -8574,7 +8572,7 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessSetupBalloon(vm, asyncJob) < 0) goto cleanup; - if (qemuProcessSetupDiskThrottling(vm, asyncJob) < 0) + if (qemuProcessSetupDisks(vm, asyncJob) < 0) goto cleanup; /* Since CPUs were not started yet, the balloon could not return the memory -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> Extract the 'rd_latency_histogram', 'wr_latency_histogram', 'zone_append_latency_histogram', and 'flush_latency_histogram' stats objects into our internal data. Rather than storing 'boundaries' between bins we store them as start points. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 20 +++++++++++++ src/qemu/qemu_monitor.h | 18 ++++++++++++ src/qemu/qemu_monitor_json.c | 55 ++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 504500c864..cdd08004fb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1986,6 +1986,26 @@ qemuBlockStatsFinalize(GObject *object) g_free(stats->limits); g_free(stats->timed_stats); + if (stats->histogram_read) { + g_free(stats->histogram_read->bins); + g_free(stats->histogram_read); + } + + if (stats->histogram_write) { + g_free(stats->histogram_write->bins); + g_free(stats->histogram_write); + } + + if (stats->histogram_zone) { + g_free(stats->histogram_zone->bins); + g_free(stats->histogram_zone); + } + + if (stats->histogram_flush) { + g_free(stats->histogram_flush->bins); + g_free(stats->histogram_flush); + } + G_OBJECT_CLASS(qemu_block_stats_parent_class)->finalize(object); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index d096f474c1..fb4fe2bc76 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -806,6 +806,18 @@ struct qemuBlockStatsLimits { }; +struct qemuBlockStatsLatencyHistogramBin { + unsigned long long start; + unsigned long long value; +}; + + +struct qemuBlockStatsLatencyHistogram { + struct qemuBlockStatsLatencyHistogramBin *bins; + size_t nbins; +}; + + struct qemuBlockStatsTimed { unsigned long long interval_length; @@ -858,6 +870,12 @@ struct _qemuBlockStats { /* block accounting/timed stats from qemu - one entry per interval configured */ size_t n_timed_stats; struct qemuBlockStatsTimed *timed_stats; + + /* latency histograms */ + struct qemuBlockStatsLatencyHistogram *histogram_read; + struct qemuBlockStatsLatencyHistogram *histogram_write; + struct qemuBlockStatsLatencyHistogram *histogram_zone; + struct qemuBlockStatsLatencyHistogram *histogram_flush; }; G_DECLARE_FINAL_TYPE(qemuBlockStats, qemu_block_stats, QEMU, BLOCK_STATS, GObject); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a602b1e65b..5736546ec2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2425,6 +2425,52 @@ qemuMonitorJSONBlockStatsCollectDataTimed(virJSONValue *timed_stats, } +static void +qemuMonitorJSONBlockStatsCollectDataLatencyHistogram(virJSONValue *stats, + const char *histogram_field, + struct qemuBlockStatsLatencyHistogram **histogram_data) +{ + virJSONValue *hist; + virJSONValue *hist_bins; + virJSONValue *hist_bounds; + g_autofree struct qemuBlockStatsLatencyHistogramBin *bins = NULL; + size_t nbins = 0; + size_t i; + + if (!(hist = virJSONValueObjectGetObject(stats, histogram_field))) + return; + + if (!(hist_bins = virJSONValueObjectGetArray(hist, "bins")) || + !(hist_bounds = virJSONValueObjectGetArray(hist, "boundaries")) || + virJSONValueArraySize(hist_bins) != (virJSONValueArraySize(hist_bounds) + 1)) { + VIR_DEBUG("malformed latency histogram container"); + return; + } + + nbins = virJSONValueArraySize(hist_bins); + bins = g_new0(struct qemuBlockStatsLatencyHistogramBin, nbins); + + for (i = 0; i < nbins; i++) { + virJSONValue *bin = virJSONValueArrayGet(hist_bins, i); + virJSONValue *bound = NULL; + + if (i > 0) + bound = virJSONValueArrayGet(hist_bounds, i - 1); + + if (!bin || + virJSONValueGetNumberUlong(bin, &(bins[i].value)) < 0 || + (bound && virJSONValueGetNumberUlong(bound, &(bins[i].start)) < 0)) { + VIR_DEBUG("malformed latency histogram container"); + return; + } + } + + *histogram_data = g_new0(struct qemuBlockStatsLatencyHistogram, 1); + (*histogram_data)->bins = g_steal_pointer(&bins); + (*histogram_data)->nbins = nbins; +} + + static qemuBlockStats * qemuMonitorJSONBlockStatsCollectData(virJSONValue *dev, int *nstats) @@ -2469,6 +2515,15 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValue *dev, bstats->wr_highest_offset_valid = true; } + qemuMonitorJSONBlockStatsCollectDataLatencyHistogram(stats, "rd_latency_histogram", + &bstats->histogram_read); + qemuMonitorJSONBlockStatsCollectDataLatencyHistogram(stats, "wr_latency_histogram", + &bstats->histogram_write); + qemuMonitorJSONBlockStatsCollectDataLatencyHistogram(stats, "zone_append_latency_histogram", + &bstats->histogram_zone); + qemuMonitorJSONBlockStatsCollectDataLatencyHistogram(stats, "flush_latency_histogram", + &bstats->histogram_flush); + if ((timed_stats = virJSONValueObjectGetArray(stats, "timed_stats")) && virJSONValueArraySize(timed_stats) > 0) qemuMonitorJSONBlockStatsCollectDataTimed(timed_stats, bstats); -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> Add documentation and constants for constructing the stats field names for latency histograms and expose them in the qemu driver: Example: block.1.latency_histogram.read.bin.count=9 block.1.latency_histogram.read.bin.0.start=0 block.1.latency_histogram.read.bin.0.value=0 block.1.latency_histogram.read.bin.1.start=10 block.1.latency_histogram.read.bin.1.value=0 block.1.latency_histogram.read.bin.2.start=100 block.1.latency_histogram.read.bin.2.value=0 block.1.latency_histogram.read.bin.3.start=1000 block.1.latency_histogram.read.bin.3.value=1047 block.1.latency_histogram.read.bin.4.start=10000 block.1.latency_histogram.read.bin.4.value=2131 block.1.latency_histogram.read.bin.5.start=100000 block.1.latency_histogram.read.bin.5.value=0 block.1.latency_histogram.read.bin.6.start=1000000 block.1.latency_histogram.read.bin.6.value=0 block.1.latency_histogram.read.bin.7.start=10000000 block.1.latency_histogram.read.bin.7.value=0 block.1.latency_histogram.read.bin.8.start=100000000 block.1.latency_histogram.read.bin.8.value=0 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/manpages/virsh.rst | 7 ++ include/libvirt/libvirt-domain.h | 113 +++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 43 ++++++++++++ 3 files changed, 163 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index a9d691824e..ff0cf1a715 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -2811,6 +2811,13 @@ Information listed includes: pending write operations in the defined interval * ``block.<num>.timed_group.<num>.zone_append_queue_depth_avg`` - average number of pending zone append operations in the defined interval +* ``block.<num>.latency_histogram.<type>.bin.count`` - number of bins in + latency histogram. <type> is one of ``read``, ``write``, ``zone_append``, or + ``flush`` +* ``block.<num>.latency_histogram.<type>.bin.<num>.start`` start boundary of + a latency histogram bin in nanoseconds of given operation duration +* ``block.<num>.latency_histogram.<type>.bin.<num>.value`` current number of + events corresponding to the given bin and type *--iothread* returns information about IOThreads on the running guest diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 893359aaae..56c21dcf25 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3896,6 +3896,119 @@ struct _virDomainStatsRecord { */ # define VIR_DOMAIN_STATS_BLOCK_SUFFIX_TIMED_GROUP_SUFFIX_ZONE_APPEND_QUEUE_DEPTH_AVG ".zone_append_queue_depth_avg" +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_READ_PREFIX: + * + * The parameter name prefix to access 'read' latency histograms. Concatenate + * the prefix with either: + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_COUNT + * to get the number of bins in given histogram + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_PREFIX and + * entry number formatted as an unsigned integer and one of the latency + * histogram suffix parameters to compelte a full bin parameter name + * + * Since: 12.1.0 + */ +# define VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_READ_PREFIX ".latency_histogram.read." + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_WRITE_PREFIX: + * + * The parameter name prefix to access 'write' latency histograms. Concatenate + * the prefix with either: + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_COUNT + * to get the number of bins in given histogram + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_PREFIX and + * entry number formatted as an unsigned integer and one of the latency + * histogram suffix parameters to compelte a full bin parameter name + * + * Since: 12.1.0 + */ +# define VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_WRITE_PREFIX ".latency_histogram.write." + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_ZONE_APPEND_PREFIX: + * + * The parameter name prefix to access 'zone_append' latency histograms. Concatenate + * the prefix with either: + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_COUNT + * to get the number of bins in given histogram + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_PREFIX and + * entry number formatted as an unsigned integer and one of the latency + * histogram suffix parameters to compelte a full bin parameter name + * + * Since: 12.1.0 + */ +# define VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_ZONE_APPEND_PREFIX ".latency_histogram.zone_append." + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_FLUSH_PREFIX: + * + * The parameter name prefix to access 'flush' latency histograms. Concatenate + * the prefix with either: + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_COUNT + * to get the number of bins in given histogram + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_PREFIX and + * entry number formatted as an unsigned integer and one of the latency + * histogram suffix parameters to compelte a full bin parameter name + * + * Since: 12.1.0 + */ +# define VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_FLUSH_PREFIX ".latency_histogram.flush." + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_COUNT: + * + * The parameter name suffix to access number of bins in one of the following + * latency histogram types: + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_READ_PREFIX + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_WRITE_PREFIX + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_ZONE_APPEND_PREFIX + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_FLUSH_PREFIX + * + * Number of bins in latency histogram as unsigned long long. + * + * Since: 12.1.0 + */ +# define VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_COUNT "bin.count" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_PREFIX: + * + * The parameter name suffix to access a latency histogram bin in one of the + * following latency histogram types: + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_READ_PREFIX + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_WRITE_PREFIX + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_ZONE_APPEND_PREFIX + * - VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_FLUSH_PREFIX + * + * Concatenate with a bin number as unsigned int and one of the other field + * suffixes to access bin parameters. + * + * Since: 12.1.0 + */ +# define VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_PREFIX "bin." + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_SUFFIX_START: + * + * Start of the current latency histogram bin in nanoseconds as unsigned long long. + * + * Since: 12.1.0 + */ +# define VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_SUFFIX_START ".start" + +/** + * VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_SUFFIX_VALUE: + * + * Current value of the number of occurences of the latency within this bin + * as unsigned long long. + * + * Since: 12.1.0 + */ +# define VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_SUFFIX_VALUE ".value" + + /** * VIR_DOMAIN_STATS_PERF_CMT: * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cdd333c882..d8cc32eee8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17606,6 +17606,36 @@ qemuDomainGetStatsBlockExportBackendStorage(const char *entryname, } +static void +qemuDomainGetStatsBlockExportFrontendLatencyHistogram(struct qemuBlockStatsLatencyHistogram *h, + size_t disk_idx, + const char *prefix_hist, + virTypedParamList *par) +{ + size_t i; + + if (!h) + return; + + virTypedParamListAddULLong(par, h->nbins, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu%s" VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_COUNT, + disk_idx, prefix_hist); + + for (i = 0; i < h->nbins; i++) { + virTypedParamListAddULLong(par, h->bins[i].start, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu%s" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_SUFFIX_START, + disk_idx, prefix_hist, i); + virTypedParamListAddULLong(par, h->bins[i].value, + VIR_DOMAIN_STATS_BLOCK_PREFIX "%zu%s" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_PREFIX "%zu" + VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_SUFFIX_BIN_SUFFIX_VALUE, + disk_idx, prefix_hist, i); + } +} + + static void qemuDomainGetStatsBlockExportFrontend(const char *frontendname, GHashTable *stats, @@ -17730,6 +17760,19 @@ qemuDomainGetStatsBlockExportFrontend(const char *frontendname, idx, i); } } + + qemuDomainGetStatsBlockExportFrontendLatencyHistogram(en->histogram_read, idx, + VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_READ_PREFIX, + par); + qemuDomainGetStatsBlockExportFrontendLatencyHistogram(en->histogram_write, idx, + VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_WRITE_PREFIX, + par); + qemuDomainGetStatsBlockExportFrontendLatencyHistogram(en->histogram_zone, idx, + VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_ZONE_APPEND_PREFIX, + par); + qemuDomainGetStatsBlockExportFrontendLatencyHistogram(en->histogram_flush, idx, + VIR_DOMAIN_STATS_BLOCK_SUFFIX_LATENCY_HISTOGRAM_FLUSH_PREFIX, + par); } -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> Add QMP monitor code for setting up latency histogram configuration. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 21 +++++++++++++ src/qemu/qemu_monitor.h | 9 ++++++ src/qemu/qemu_monitor_json.c | 60 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 9 ++++++ tests/qemumonitorjsontest.c | 9 ++++++ 5 files changed, 108 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index cdd08004fb..3d7477c01c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4612,3 +4612,24 @@ qemuMonitorBlockdevSetActive(qemuMonitor *mon, return qemuMonitorJSONBlockdevSetActive(mon, nodename, active); } + + +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) +{ + QEMU_CHECK_MONITOR(mon); + VIR_DEBUG("id='%s'", id); + + return qemuMonitorJSONBlockLatencyHistogramSet(mon, id, + boundaries, + boundaries_read, + boundaries_write, + boundaries_zone, + boundaries_flush); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index fb4fe2bc76..128b0fa2ce 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1984,3 +1984,12 @@ int qemuMonitorBlockdevSetActive(qemuMonitor *mon, const char *nodename, bool active); + +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); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5736546ec2..8c930ca1bb 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9113,3 +9113,63 @@ qemuMonitorJSONBlockdevSetActive(qemuMonitor *mon, return qemuMonitorJSONCheckError(cmd, reply); } + + +static virJSONValue * +qemuMonitorJSONBlockLatencyHistogramBoundary(unsigned int *bound) +{ + g_autoptr(virJSONValue) ret = virJSONValueNewArray(); + + if (!bound) + return NULL; + + for (; *bound > 0; bound++) { + g_autoptr(virJSONValue) val = virJSONValueNewNumberUint(*bound); + + /* the only error is if the first argument is not an array */ + ignore_value(virJSONValueArrayAppend(ret, &val)); + } + + return g_steal_pointer(&ret); +} + + +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) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + g_autoptr(virJSONValue) bound = NULL; + g_autoptr(virJSONValue) bound_read = NULL; + g_autoptr(virJSONValue) bound_write = NULL; + g_autoptr(virJSONValue) bound_zone = NULL; + g_autoptr(virJSONValue) bound_flush = NULL; + + bound = qemuMonitorJSONBlockLatencyHistogramBoundary(boundaries); + bound_read = qemuMonitorJSONBlockLatencyHistogramBoundary(boundaries_read); + bound_write = qemuMonitorJSONBlockLatencyHistogramBoundary(boundaries_write); + bound_zone = qemuMonitorJSONBlockLatencyHistogramBoundary(boundaries_zone); + bound_flush = qemuMonitorJSONBlockLatencyHistogramBoundary(boundaries_flush); + + if (!(cmd = qemuMonitorJSONMakeCommand("block-latency-histogram-set", + "s:id", id, + "A:boundaries", &bound, + "A:boundaries-read", &bound_read, + "A:boundaries-write", &bound_write, + "A:boundaries-zap", &bound_zone, + "A:boundaries-flush", &bound_flush, + NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + return qemuMonitorJSONCheckError(cmd, reply); +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index db9160eb68..b418f70048 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -815,3 +815,12 @@ int qemuMonitorJSONBlockdevSetActive(qemuMonitor *mon, const char *nodename, bool active); + +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); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index bfe81739a7..1c1aaaa586 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1130,6 +1130,14 @@ 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 testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque) { @@ -2958,6 +2966,7 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONBlockJobCancel); DO_TEST_GEN(qemuMonitorJSONSetAction); DO_TEST_GEN(qemuMonitorJSONSetLaunchSecurityState); + DO_TEST_GEN(qemuMonitorJSONBlockLatencyHistogramSet); DO_TEST(qemuMonitorJSONGetBalloonInfo); DO_TEST(qemuMonitorJSONGetBlockInfo); DO_TEST(qemuMonitorJSONGetAllBlockStatsInfo); -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> All other paragraphs in this section use 2 spaces after hyphen, fix the recently added section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 0fa9d2e08b..b5a8f7b714 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3619,22 +3619,22 @@ paravirtualized driver is specified via the ``disk`` element. </iothreads> </driver> - - The optional ``statistics`` sub-element allows configuring statistics - collection in configurable intervals for the given disk. Intervals are - configured by ``<statistic>`` sub-elements with ``interval`` attribute - configuring the collection window duration in seconds. The statistics - are available via the bulk statistics API. - - Example:: - - <driver name='qemu'> - <statistics> - <statistic interval='1'/> - <statistic interval='10'/> - </statistics> - </driver> + - The optional ``statistics`` sub-element allows configuring statistics + collection in configurable intervals for the given disk. Intervals are + configured by ``<statistic>`` sub-elements with ``interval`` attribute + configuring the collection window duration in seconds. The statistics + are available via the bulk statistics API. + + Example:: + + <driver name='qemu'> + <statistics> + <statistic interval='1'/> + <statistic interval='10'/> + </statistics> + </driver> - :since:`Since 11.9.0 (QEMU 10.2, virtio, ide, scsi disks only)`. + :since:`Since 11.9.0 (QEMU 10.2, virtio, ide, scsi disks only)`. - The optional ``queues`` attribute specifies the number of virt queues for virtio-blk ( :since:`Since 3.9.0` ) or vhost-user-blk -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> Separate the timed statistics group and link to the fields which are returned by it. Prepare the wording for more statistics configs in the future. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index b5a8f7b714..3c7dbf97db 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3619,11 +3619,17 @@ paravirtualized driver is specified via the ``disk`` element. </iothreads> </driver> - - The optional ``statistics`` sub-element allows configuring statistics - collection in configurable intervals for the given disk. Intervals are - configured by ``<statistic>`` sub-elements with ``interval`` attribute - configuring the collection window duration in seconds. The statistics - are available via the bulk statistics API. + - The optional ``statistics`` sub-element allows configuring various optional + statistics collection. + + Statistic values returned under + `VIR_DOMAIN_STATS_BLOCK_SUFFIX_TIMED_GROUP_PREFIX <html/libvirt-libvirt-domain.html#VIR_DOMAIN_STATS_BLOCK_SUFFIX_TIMED_GROUP_PREFIX>`__ + typed parameter prefix returned by the + `virConnectGetAllDomainStats <html/libvirt-libvirt-domain.html#virConnectGetAllDomainStats>`__ + API are collected based on one or more configurable intervals. An interval + of collection is configured by ``<statistic>`` sub-elements with + ``interval`` attribute configuring the collection window duration in + seconds. Example:: -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> Add config and docs allowing enabling latency histogram collection for block device operations. This patch sets up the docs, schema and XML infrastructure. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatdomain.rst | 41 ++++++ src/conf/domain_conf.c | 133 +++++++++++++++++- src/conf/domain_conf.h | 7 + src/conf/schemas/domaincommon.rng | 37 ++++- ...isk-statistics-intervals.x86_64-latest.xml | 29 ++++ .../disk-statistics-intervals.xml | 25 ++++ 6 files changed, 262 insertions(+), 10 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 3c7dbf97db..799c648980 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3642,6 +3642,47 @@ paravirtualized driver is specified via the ``disk`` element. :since:`Since 11.9.0 (QEMU 10.2, virtio, ide, scsi disks only)`. + Block operation latency histogram collection can be configured using + ``<latency-histogram>`` sub-element. The histogram is collected for + the whole runtime of the VM, but can be re-started or reconfigured using + the `virDomainUpdateDeviceFlags <html/libvirt-libvirt-domain.html#virDomainUpdateDeviceFlags>`__ + API. Using the same config re-starts histogram collection. + + The optional ``type`` attribute configures specific operation to collect + the histogram for. Supported types are ``read``, ``write``, ``zone``, and + ``flush``. If the ``type`` attribute is omitted the histogram collection + bins bins apply to all of the aforementioned types, which can be overriden + with specific config. + + The ``<latency-histogram>`` has multiple mandatory ``<bin>`` sub-elements + with mandatory ``start`` attribute configuring the starting boundary of + the histogram bin configured in nanosecods of the operation duration and + the intervals must be properly ordered and non-duplicate. + + Example:: + + <driver name='qemu'> + <statistics> + + <latency-histogram> + <bin start='0'/> + <bin start='1000'/> + <bin start='100000'/> + </latency-histogram> + + [or for specific operation types] + + <latency-histogram type='read'> + <bin start='0'/> + <bin start='1000'/> + <bin start='100000'/> + </latency-histogram> + + </statistics> + </driver> + + :since:`Since 12.1.0`. + - The optional ``queues`` attribute specifies the number of virt queues for virtio-blk ( :since:`Since 3.9.0` ) or vhost-user-blk ( :since:`Since 7.1.0` ) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 02e23f7866..1fc3ef966d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2446,6 +2446,11 @@ virDomainDiskDefFree(virDomainDiskDef *def) virObjectUnref(def->privateData); g_slist_free_full(def->iothreads, (GDestroyNotify) virDomainIothreadMappingDefFree); g_free(def->statistics); + g_free(def->histogram_boundaries); + g_free(def->histogram_boundaries_read); + g_free(def->histogram_boundaries_write); + g_free(def->histogram_boundaries_zone); + g_free(def->histogram_boundaries_flush); if (def->throttlefilters) { size_t i; @@ -8311,6 +8316,91 @@ virDomainIothreadMappingDefParse(xmlNodePtr driverNode, } +static int +virDomainDiskDefDriverParseXMLHistogramOne(virDomainDiskDef *def, + xmlNodePtr cur) +{ + g_autofree char *histogram_type = NULL; + unsigned int **histogram_config = NULL; + g_autoptr(GPtrArray) binNodes = virXMLNodeGetSubelementList(cur, "bin"); + size_t nbins = 0; + size_t i; + + if ((histogram_type = virXMLPropString(cur, "type"))) { + if (STREQ(histogram_type, "read")) { + histogram_config = &def->histogram_boundaries_read; + } else if (STREQ(histogram_type, "write")) { + histogram_config = &def->histogram_boundaries_write; + } else if (STREQ(histogram_type, "zone")) { + histogram_config = &def->histogram_boundaries_zone; + } else if (STREQ(histogram_type, "flush")) { + histogram_config = &def->histogram_boundaries_flush; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown latency_histogram type '%1$s'"), + histogram_type); + return -1; + } + } else { + histogram_config = &def->histogram_boundaries; + } + + if (*histogram_config) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only one latency-histogram of a given type is supported")); + return -1; + } + + if (binNodes->len == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'bin' elements for 'latency-histogram'")); + return -1; + } + + *histogram_config = g_new0(unsigned int, binNodes->len + 1); + + for (i = 0; i < binNodes->len; i++) { + unsigned int val; + + if (virXMLPropUInt(g_ptr_array_index(binNodes, i), + "start", 10, + VIR_XML_PROP_REQUIRED, + &val) < 0) + return -1; + + if (nbins > 0 && + (val == 0 || + val <= (*histogram_config)[nbins-1])) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the values of 'start' attribute of a 'latency-histogram' 'bin' configuration must be sorted and non-overlapping")); + return -1; + } + + if (val > 0) + (*histogram_config)[nbins++] = val; + } + + return 0; +} + + +static int +virDomainDiskDefDriverParseXMLHistograms(virDomainDiskDef *def, + xmlNodePtr cur) +{ + g_autoptr(GPtrArray) histogramNodes = virXMLNodeGetSubelementList(cur, "latency-histogram"); + size_t i; + + for (i = 0; i < histogramNodes->len; i++) { + if (virDomainDiskDefDriverParseXMLHistogramOne(def, + g_ptr_array_index(histogramNodes, i)) < 0) + return -1; + } + + return 0; +} + + static int virDomainDiskDefDriverParseXML(virDomainDiskDef *def, xmlNodePtr cur) @@ -8384,6 +8474,9 @@ virDomainDiskDefDriverParseXML(virDomainDiskDef *def, return -1; } } + + if (virDomainDiskDefDriverParseXMLHistograms(def, statisticsNode) < 0) + return -1; } if (virXMLPropEnum(cur, "detect_zeroes", @@ -23987,12 +24080,37 @@ virDomainDiskDefFormatThrottleFilters(virBuffer *buf, } +static void +virDomainDiskDefFormatDriverHistogram(virBuffer *buf, + const char *type, + unsigned int *bins) +{ + g_auto(virBuffer) histogramAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) histogramChildBuf = VIR_BUFFER_INIT_CHILD(buf); + + if (!bins || bins[0] == 0) + return; + + if (type) + virBufferAsprintf(&histogramAttrBuf, " type='%s'", type); + + /* we dont store the start boundary of the first bin but it's always there */ + virBufferAddLit(&histogramChildBuf, "<bin start='0'/>\n"); + + for (; *bins > 0; bins++) + virBufferAsprintf(&histogramChildBuf, "<bin start='%u'/>\n", *bins); + + virXMLFormatElement(buf, "latency-histogram", &histogramAttrBuf, &histogramChildBuf); +} + + static void virDomainDiskDefFormatDriver(virBuffer *buf, virDomainDiskDef *disk) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + g_auto(virBuffer) statisticsChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); virBufferEscapeString(&attrBuf, " name='%s'", virDomainDiskGetDriver(disk)); @@ -24064,16 +24182,25 @@ virDomainDiskDefFormatDriver(virBuffer *buf, virDomainIothreadMappingDefFormat(&childBuf, disk->iothreads); if (disk->statistics) { - g_auto(virBuffer) statisticsChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf); size_t i; for (i = 0; disk->statistics[i] > 0; i++) virBufferAsprintf(&statisticsChildBuf, "<statistic interval='%u'/>\n", disk->statistics[i]); - - virXMLFormatElement(&childBuf, "statistics", NULL, &statisticsChildBuf); } + virDomainDiskDefFormatDriverHistogram(&statisticsChildBuf, NULL, + disk->histogram_boundaries); + virDomainDiskDefFormatDriverHistogram(&statisticsChildBuf, "read", + disk->histogram_boundaries_read); + virDomainDiskDefFormatDriverHistogram(&statisticsChildBuf, "write", + disk->histogram_boundaries_write); + virDomainDiskDefFormatDriverHistogram(&statisticsChildBuf, "zone", + disk->histogram_boundaries_zone); + virDomainDiskDefFormatDriverHistogram(&statisticsChildBuf, "flush", + disk->histogram_boundaries_flush); + + virXMLFormatElement(&childBuf, "statistics", NULL, &statisticsChildBuf); virXMLFormatElement(buf, "driver", &attrBuf, &childBuf); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 66dc4e3417..b59aca1d6d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -596,6 +596,13 @@ struct _virDomainDiskDef { GSList *iothreads; /* List of virDomainIothreadMappingDef */ 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; + virDomainDiskDetectZeroes detect_zeroes; virTristateSwitch discard_no_unref; char *domain_name; /* backend domain name */ diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 8669d8f791..47b087a1d4 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -2741,13 +2741,36 @@ </optional> <optional> <element name="statistics"> - <zeroOrMore> - <element name="statistic"> - <attribute name="interval"> - <ref name="unsignedInt"/> - </attribute> - </element> - </zeroOrMore> + <interleave> + <zeroOrMore> + <element name="statistic"> + <attribute name="interval"> + <ref name="unsignedInt"/> + </attribute> + </element> + </zeroOrMore> + <zeroOrMore> + <element name="latency-histogram"> + <optional> + <attribute name='type'> + <choice> + <value>read</value> + <value>write</value> + <value>zone</value> + <value>flush</value> + </choice> + </attribute> + </optional> + <oneOrMore> + <element name='bin'> + <attribute name='start'> + <ref name="unsignedInt"/> + </attribute> + </element> + </oneOrMore> + </element> + </zeroOrMore> + </interleave> </element> </optional> </interleave> diff --git a/tests/qemuxmlconfdata/disk-statistics-intervals.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-statistics-intervals.x86_64-latest.xml index 4c55c50ef5..d02f954073 100644 --- a/tests/qemuxmlconfdata/disk-statistics-intervals.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/disk-statistics-intervals.x86_64-latest.xml @@ -22,6 +22,11 @@ <statistics> <statistic interval='3'/> <statistic interval='10'/> + <latency-histogram> + <bin start='0'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> </statistics> </driver> <source file='/var/lib/libvirt/images/iothrtest1.img'/> @@ -33,6 +38,30 @@ <statistics> <statistic interval='5'/> <statistic interval='15'/> + <latency-histogram type='read'> + <bin start='0'/> + <bin start='10'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> + <latency-histogram type='write'> + <bin start='0'/> + <bin start='10'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> + <latency-histogram type='zone'> + <bin start='0'/> + <bin start='10'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> + <latency-histogram type='flush'> + <bin start='0'/> + <bin start='10'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> </statistics> </driver> <source file='/var/lib/libvirt/images/iothrtest2.img'/> diff --git a/tests/qemuxmlconfdata/disk-statistics-intervals.xml b/tests/qemuxmlconfdata/disk-statistics-intervals.xml index f5e801f5a8..5f9e9470d7 100644 --- a/tests/qemuxmlconfdata/disk-statistics-intervals.xml +++ b/tests/qemuxmlconfdata/disk-statistics-intervals.xml @@ -19,6 +19,11 @@ <statistics> <statistic interval='3'/> <statistic interval='10'/> + <latency-histogram> + <bin start='0'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> </statistics> </driver> <source file='/var/lib/libvirt/images/iothrtest1.img'/> @@ -29,6 +34,26 @@ <statistics> <statistic interval='5'/> <statistic interval='15'/> + <latency-histogram type='read'> + <bin start='10'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> + <latency-histogram type='write'> + <bin start='10'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> + <latency-histogram type='zone'> + <bin start='10'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> + <latency-histogram type='flush'> + <bin start='10'/> + <bin start='20'/> + <bin start='30'/> + </latency-histogram> </statistics> </driver> <source file='/var/lib/libvirt/images/iothrtest2.img'/> -- 2.52.0
From: Peter Krempa <pkrempa@redhat.com> Setup the histograms on startup and hotplug of devices via 'qemuProcessSetupDiskPropsRuntime' and facilitate update/reset/disable of histogram collection via 'qemuDomainChangeDiskLive'. The latter allows to use the update device API to either clear the bins or select new bin configuration or disable the histogram altogether without the need for a specific API. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 17 +++++++++++++++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_hotplug.c | 29 +++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 10 ++++++++++ 4 files changed, 59 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8e1ebe7799..bdab117e96 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10463,6 +10463,23 @@ qemuDomainInitializePflashStorageSource(virDomainObj *vm, } +/** + * qemuDomainDiskHasLatencyHistogram: + * @disk: disk definition + * + * Returns whether @disk has any latency histogram settings configured. + */ +bool +qemuDomainDiskHasLatencyHistogram(virDomainDiskDef *disk) +{ + return disk->histogram_boundaries || + disk->histogram_boundaries_read || + disk->histogram_boundaries_write || + disk->histogram_boundaries_zone || + disk->histogram_boundaries_flush; +} + + /** * qemuDomainDiskBlockJobIsSupported: * diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3361e97315..30ca67bf76 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1078,6 +1078,9 @@ int qemuDomainInitializePflashStorageSource(virDomainObj *vm, virQEMUDriverConfig *cfg); +bool +qemuDomainDiskHasLatencyHistogram(virDomainDiskDef *disk); + bool qemuDomainDiskBlockJobIsSupported(virDomainDiskDef *disk); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ffd7795b66..df61f17723 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -7327,6 +7327,35 @@ qemuDomainChangeDiskLive(virDomainObj *vm, dev->data.disk->src = NULL; } + if (qemuDomainDiskHasLatencyHistogram(disk) || + qemuDomainDiskHasLatencyHistogram(orig_disk)) { + int rc; + + qemuDomainObjEnterMonitor(vm); + rc = qemuMonitorBlockLatencyHistogramSet(qemuDomainGetMonitor(vm), + QEMU_DOMAIN_DISK_PRIVATE(orig_disk)->qomName, + disk->histogram_boundaries, + disk->histogram_boundaries_read, + disk->histogram_boundaries_write, + disk->histogram_boundaries_zone, + disk->histogram_boundaries_flush); + qemuDomainObjExitMonitor(vm); + + if (rc < 0) + return -1; + + g_clear_pointer(&orig_disk->histogram_boundaries, g_free); + g_clear_pointer(&orig_disk->histogram_boundaries_read, g_free); + g_clear_pointer(&orig_disk->histogram_boundaries_write, g_free); + g_clear_pointer(&orig_disk->histogram_boundaries_zone, g_free); + g_clear_pointer(&orig_disk->histogram_boundaries_flush, g_free); + orig_disk->histogram_boundaries = g_steal_pointer(&disk->histogram_boundaries); + orig_disk->histogram_boundaries_read = g_steal_pointer(&disk->histogram_boundaries_read); + orig_disk->histogram_boundaries_write = g_steal_pointer(&disk->histogram_boundaries_write); + orig_disk->histogram_boundaries_zone = g_steal_pointer(&disk->histogram_boundaries_zone); + orig_disk->histogram_boundaries_flush = g_steal_pointer(&disk->histogram_boundaries_flush); + } + /* in case when we aren't updating disk source we update startup policy here */ orig_disk->startupPolicy = dev->data.disk->startupPolicy; orig_disk->snapshot = dev->data.disk->snapshot; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 411c5b3b88..3d9db188fc 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7958,6 +7958,16 @@ qemuProcessSetupDiskPropsRuntime(qemuMonitor *mon, &disk->blkdeviotune) < 0) return -1; + if (qemuDomainDiskHasLatencyHistogram(disk) && + qemuMonitorBlockLatencyHistogramSet(mon, + QEMU_DOMAIN_DISK_PRIVATE(disk)->qomName, + disk->histogram_boundaries, + disk->histogram_boundaries_read, + disk->histogram_boundaries_write, + disk->histogram_boundaries_zone, + disk->histogram_boundaries_flush) < 0) + return -1; + return 0; } -- 2.52.0
On Mon, Feb 02, 2026 at 16:06:37 +0100, Peter Krempa via Devel wrote:
This patchset introduces configuration of the block latency histogram feature in qemu as well as exposes the histograms via bulk domain stats.
The feature was in qemu for a long time so no capabilities are needed.
Peter Krempa (9): qemu: Extract disk setup done via QMP into a separate helper qemu: process: Rename 'qemuProcessSetupDiskThrottling' to 'qemuProcessSetupDisks' qemu: monitor: Extract block latency histogram stats into 'qemuBlockStats' Expose latency histograms via 'virConnectGetAllDomainStats' qemu: monitor: Add handlers for 'block-latency-histogram-set' docs: formatdomain: Fix indentation of docs for <disk><driver><statistics> element docs: formatdomain: Reword section about the '<statistics>' element under disk driver Introduce support for disk operation latency histogram collection qemu: Setup disk latency histograms on startup/hotplug/update
Ping?
On 2/2/26 16:06, Peter Krempa via Devel wrote:
This patchset introduces configuration of the block latency histogram feature in qemu as well as exposes the histograms via bulk domain stats.
The feature was in qemu for a long time so no capabilities are needed.
Peter Krempa (9): qemu: Extract disk setup done via QMP into a separate helper qemu: process: Rename 'qemuProcessSetupDiskThrottling' to 'qemuProcessSetupDisks' qemu: monitor: Extract block latency histogram stats into 'qemuBlockStats' Expose latency histograms via 'virConnectGetAllDomainStats' qemu: monitor: Add handlers for 'block-latency-histogram-set' docs: formatdomain: Fix indentation of docs for <disk><driver><statistics> element docs: formatdomain: Reword section about the '<statistics>' element under disk driver Introduce support for disk operation latency histogram collection qemu: Setup disk latency histograms on startup/hotplug/update
docs/formatdomain.rst | 77 ++++++++-- docs/manpages/virsh.rst | 7 + include/libvirt/libvirt-domain.h | 113 +++++++++++++++ src/conf/domain_conf.c | 133 +++++++++++++++++- src/conf/domain_conf.h | 7 + src/conf/schemas/domaincommon.rng | 37 ++++- src/qemu/qemu_domain.c | 17 +++ src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 43 ++++++ src/qemu/qemu_hotplug.c | 52 ++++--- src/qemu/qemu_monitor.c | 41 ++++++ src/qemu/qemu_monitor.h | 27 ++++ src/qemu/qemu_monitor_json.c | 115 +++++++++++++++ src/qemu/qemu_monitor_json.h | 9 ++ src/qemu/qemu_process.c | 63 ++++++--- src/qemu/qemu_process.h | 3 + tests/qemumonitorjsontest.c | 9 ++ ...isk-statistics-intervals.x86_64-latest.xml | 29 ++++ .../disk-statistics-intervals.xml | 25 ++++ 19 files changed, 748 insertions(+), 62 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa