[libvirt] [PATCH 10/12] qemu: blockstats: Refactor qemuDomainBlockStatsFlags

[Peter Krempa]

Now that qemuDomainBlocksStatsGather provides functions of both
qemuMonitorGetBlockStatsParamsNumber and qemuMonitorGetBlockStatsInfo we
can reuse it and kill a lot of code.

Additionally as a bonus qemuDomainBlockStatsFlags will now support
summary statistics so add a statement to the virsh man page about that.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1142636

man
---
 src/qemu/qemu_driver.c | 82 +++++++++++---------------------------------------
 tools/virsh.pod        |  5 +--
 2 files changed, 21 insertions(+), 66 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4458c52..c35637b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10637,21 +10637,14 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
                           unsigned int flags)
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
-    int idx;
-    int tmp, ret = -1;
     virDomainObjPtr vm;
-    qemuDomainObjPrivatePtr priv;
-    long long rd_req, rd_bytes, wr_req, wr_bytes, rd_total_times;
-    long long wr_total_times, flush_req, flush_total_times;
-    char *diskAlias = NULL;
+    qemuBlockStatsPtr blockstats = NULL;
+    int nstats;
+    int ret = -1;

-    virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);
+    VIR_DEBUG("params=%p, flags=%x", params, flags);

-    if (!*path) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                       _("summary statistics are not supported yet"));
-        return ret;
-    }
+    virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1);

     /* We don't return strings, and thus trivially support this flag.  */
     flags &= ~VIR_TYPED_PARAM_STRING_OKAY;
@@ -10671,64 +10664,26 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
         goto endjob;
     }

-    if (*nparams != 0) {
-        virDomainDiskDefPtr disk = NULL;
-        if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
-            virReportError(VIR_ERR_INVALID_ARG,
-                           _("invalid path: %s"), path);
-            goto endjob;
-        }
-        disk = vm->def->disks[idx];
-
-        if (!disk->info.alias) {
-             virReportError(VIR_ERR_INTERNAL_ERROR,
-                            _("missing disk device alias name for %s"),
-                            disk->dst);
-             goto endjob;
-        }
-        if (VIR_STRDUP(diskAlias, disk->info.alias) < 0)
-            goto endjob;
-    }
-
-    priv = vm->privateData;
-    VIR_DEBUG("priv=%p, params=%p, flags=%x", priv, params, flags);
-
-    qemuDomainObjEnterMonitor(driver, vm);
-    tmp = *nparams;
-    ret = qemuMonitorGetBlockStatsParamsNumber(priv->mon, nparams);
-
-    if (tmp == 0 || ret < 0) {
-        ignore_value(qemuDomainObjExitMonitor(driver, vm));
+    if ((nstats = qemuDomainBlocksStatsGather(driver, vm, path,
+                                              &blockstats)) < 0)
         goto endjob;
-    }
-
-    ret = qemuMonitorGetBlockStatsInfo(priv->mon,
-                                       diskAlias,
-                                       &rd_req,
-                                       &rd_bytes,
-                                       &rd_total_times,
-                                       &wr_req,
-                                       &wr_bytes,
-                                       &wr_total_times,
-                                       &flush_req,
-                                       &flush_total_times);
-
-    if (qemuDomainObjExitMonitor(driver, vm) < 0)
-        ret = -1;

-    if (ret < 0)
+    /* return count of supported stats */
+    if (*nparams == 0) {
+        *nparams = nstats;
+        ret = 0;
         goto endjob;
+    }

-    tmp = 0;
-    ret = -1;
+    nstats = 0;

 /* the following macro shall not be used with side effects statements */
 #define QEMU_BLOCK_STAT(VAR, NAME)                                            \
-    if (tmp < *nparams && (VAR) != -1) {                                      \
-        if (virTypedParameterAssign(params + tmp, NAME, VIR_TYPED_PARAM_LLONG,\
-                                    (VAR)) < 0)                               \
+    if (nstats < *nparams && (blockstats->VAR) != -1) {                       \
+        if (virTypedParameterAssign(params + nstats, NAME,                    \
+                                    VIR_TYPED_PARAM_LLONG, (blockstats->VAR)) < 0) \
             goto endjob;                                                      \
-        tmp++;                                                                \
+        nstats++;                                                             \
     }

     QEMU_BLOCK_STAT(wr_bytes, VIR_DOMAIN_BLOCK_STATS_WRITE_BYTES);
@@ -10748,13 +10703,12 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
     /* Field 'errs' is meaningless for QEMU, won't set it. */

     ret = 0;
-    *nparams = tmp;
+    *nparams = nstats;

  endjob:
     qemuDomainObjEndJob(driver, vm);

  cleanup:
-    VIR_FREE(diskAlias);
     qemuDomObjEndAPI(&vm);
     return ret;
 }
diff --git a/tools/virsh.pod b/tools/virsh.pod
index e65378e..afc380d 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -709,8 +709,9 @@ return an error instead.
 Get device block stats for a running domain.  A I<block-device> corresponds
 to a unique target name (<target dev='name'/>) or source file (<source
 file='name'/>) for one of the disk devices attached to I<domain> (see
-also B<domblklist> for listing these names). On a lxc domain, omitting the
-I<block-device> yields device block stats summarily for the entire domain.
+also B<domblklist> for listing these names). On a lxc or qemu domain,
+omitting the I<block-device> yields device block stats summarily for the
+entire domain.

 Use I<--human> for a more human readable output.

-- 
2.2.2