[PATCH v2 12/12] getstats: start crawling backing chain for qemu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Wire up backing chain recursion.  Note that for now, we just use
the same allocation numbers for read-only backing files as what
offline domains would report.  It is not the correct allocation
number for qcow2 over block devices during block-commit, and it
misses out on the fact that qemu also reports read statistics on
backing files that are worth knowing about (seriously - for a
thin-provisioned setup, it would be nice to easily get at a count
of how many reads were serviced from the backing file in relation
to reads serviced by the active layer).  But it is at least
sufficient to prove that the algorithm is working, and to let
other people start coding to the interface while waiting for
later patches that get the correct information.

For a running domain, where one of the two images has a backing
file, I see the traditional output:

$ virsh domstats --block testvm2
Domain: 'testvm2'
  block.count=2
  block.0.name=vda
  block.0.path=/tmp/wrapper.qcow2
  block.0.rd.reqs=1
  block.0.rd.bytes=512
  block.0.rd.times=28858
  block.0.wr.reqs=0
  block.0.wr.bytes=0
  block.0.wr.times=0
  block.0.fl.reqs=0
  block.0.fl.times=0
  block.0.allocation=0
  block.0.capacity=1310720000
  block.0.physical=200704
  block.1.name=vdb
  block.1.path=/dev/sda7
  block.1.rd.reqs=0
  block.1.rd.bytes=0
  block.1.rd.times=0
  block.1.wr.reqs=0
  block.1.wr.bytes=0
  block.1.wr.times=0
  block.1.fl.reqs=0
  block.1.fl.times=0
  block.1.allocation=0
  block.1.capacity=1310720000

vs. the new output:

$ virsh domstats --block --backing testvm2
Domain: 'testvm2'
  block.count=3
  block.0.name=vda
  block.0.path=/tmp/wrapper.qcow2
  block.0.rd.reqs=1
  block.0.rd.bytes=512
  block.0.rd.times=28858
  block.0.wr.reqs=0
  block.0.wr.bytes=0
  block.0.wr.times=0
  block.0.fl.reqs=0
  block.0.fl.times=0
  block.0.allocation=0
  block.0.capacity=1310720000
  block.0.physical=200704
  block.1.name=vda
  block.1.path=/dev/sda6
  block.1.backingIndex=1
  block.1.allocation=1073741824
  block.1.capacity=1310720000
  block.1.physical=1073741824
  block.2.name=vdb
  block.2.path=/dev/sda7
  block.2.rd.reqs=0
  block.2.rd.bytes=0
  block.2.rd.times=0
  block.2.wr.reqs=0
  block.2.wr.bytes=0
  block.2.wr.times=0
  block.2.fl.reqs=0
  block.2.fl.times=0
  block.2.allocation=0
  block.2.capacity=1310720000

* src/qemu/qemu_driver.c (QEMU_DOMAIN_STATS_BACKING): New internal
enum bit.
(qemuConnectGetAllDomainStats): Recognize new user flag, and pass
details to...
(qemuDomainGetStatsBlock): ...here, where we can do longer recursion.
(qemuDomainGetStatsOneBlock): Output new field.
* src/qemu/qemu_domain.c (qemuDomainStorageAlias): Tolerate NULL
alias input for offline domain.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
 src/qemu/qemu_domain.c |  3 +++
 src/qemu/qemu_driver.c | 57 +++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 02887cd..73eabee 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2698,6 +2698,9 @@ qemuDomainStorageAlias(const char *device, int depth)
 {
     char *alias;

+    if (!device)
+        return NULL;
+
     if (STRPREFIX(device, QEMU_DRIVE_HOST_PREFIX))
         device += strlen(QEMU_DRIVE_HOST_PREFIX);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8dc5dd5..f70182f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18260,8 +18260,10 @@ qemuDomainGetStatsState(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,


 typedef enum {
-    QEMU_DOMAIN_STATS_HAVE_JOB = (1 << 0), /* job is entered, monitor can be
-                                              accessed */
+    QEMU_DOMAIN_STATS_HAVE_JOB = 1 << 0, /* job is entered, monitor can be
+                                            accessed */
+    QEMU_DOMAIN_STATS_BACKING  = 1 << 1, /* include backing chain in
+                                            block stats */
 } qemuDomainStatsFlags;


@@ -18508,6 +18510,19 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,

 #undef QEMU_ADD_NET_PARAM

+#define QEMU_ADD_BLOCK_PARAM_UI(record, maxparams, num, name, value) \
+    do {                                                             \
+        char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];               \
+        snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,           \
+                 "block.%zu.%s", num, name);                         \
+        if (virTypedParamsAddUInt(&(record)->params,                 \
+                                  &(record)->nparams,                \
+                                  maxparams,                         \
+                                  param_name,                        \
+                                  value) < 0)                        \
+            goto cleanup;                                            \
+    } while (0)
+
 /* expects a LL, but typed parameter must be ULL */
 #define QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, num, name, value) \
 do { \
@@ -18545,20 +18560,24 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
                            virDomainDiskDefPtr disk,
                            virStorageSourcePtr src,
                            size_t block_idx,
+                           unsigned int backing_idx,
                            bool abbreviated,
                            virHashTablePtr stats)
 {
     qemuBlockStats *entry;
     int ret = -1;
+    char *alias = qemuDomainStorageAlias(disk->info.alias, backing_idx);

     QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx,
                         disk->dst);
     if (virStorageSourceIsLocalStorage(src) && src->path)
         QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path",
                             block_idx, src->path);
+    if (backing_idx)
+        QEMU_ADD_BLOCK_PARAM_UI(record, maxparams, block_idx, "backingIndex",
+                                backing_idx);

-    if (abbreviated || !disk->info.alias ||
-        !(entry = virHashLookup(stats, disk->info.alias))) {
+    if (abbreviated || !alias || !(entry = virHashLookup(stats, alias))) {
         if (virStorageSourceIsEmpty(src)) {
             ret = 0;
             goto cleanup;
@@ -18608,6 +18627,7 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,

     ret = 0;
  cleanup:
+    VIR_FREE(alias);
     return ret;
 }

@@ -18627,14 +18647,16 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
     bool abbreviated = false;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     int count_index = -1;
+    size_t visited = 0;
+    bool backing = !!(privflags & QEMU_DOMAIN_STATS_BACKING);

     if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom)) {
         abbreviated = true; /* it's ok, just go ahead silently */
     } else {
         qemuDomainObjEnterMonitor(driver, dom);
-        rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats, false);
+        rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats, backing);
         ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats,
-                                                         false));
+                                                         backing));
         qemuDomainObjExitMonitor(driver, dom);

         if (rc < 0) {
@@ -18651,18 +18673,25 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,

     for (; i < dom->def->ndisks; i++) {
         virDomainDiskDefPtr disk = dom->def->disks[i];
+        virStorageSourcePtr src = disk->src;
+        unsigned int backing_idx = 0;

-        if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
-                                       disk, disk->src, i, abbreviated,
-                                       stats) < 0)
-            goto cleanup;
+        while (src && (!backing_idx || backing)) {
+            if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
+                                           disk, src, visited, backing_idx,
+                                           abbreviated, stats) < 0)
+                goto cleanup;
+            visited++;
+            backing_idx++;
+            src = src->backingStore;
+        }
     }

     ret = 0;

  cleanup:
     if (count_index >= 0)
-        record->params[count_index].value.ui = i;
+        record->params[count_index].value.ui = visited;
     virHashFree(stats);
     virObjectUnref(cfg);
     return ret;
@@ -18803,11 +18832,13 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
     unsigned int domflags = 0;

     if (ndoms)
-        virCheckFlags(VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
+        virCheckFlags(VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
+                      VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
     else
         virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
                       VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
                       VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE |
+                      VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING |
                       VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);

     if (virConnectGetAllDomainStatsEnsureACL(conn) < 0)
@@ -18857,6 +18888,8 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
             domflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
         /* else: without a job it's still possible to gather some data */

+        if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING)
+            domflags |= QEMU_DOMAIN_STATS_BACKING;
         if (qemuDomainGetStats(conn, dom, stats, &tmp, domflags) < 0)
             goto endjob;

-- 
1.9.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]