From: Peter Krempa <[email protected]>

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 <[email protected]>
---
 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

Reply via email to