qemu: monitor: return block stats data as a hash to avoid disk mixup

The current block stats code matched up the disk name with the actual
stats by the order in the data returned from qemu. This unfortunately
isn't right as qemu may return the disks in any order. Fix this by
returning a hash of stats and index them by the disk alias.

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6606154eb5812b28cf32e821275c507dcd0a7de9..63fc83f70856e7f410c1018172e1dbedc808a29b 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17922,24 +17922,18 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
 {
     size_t i;
     int ret = -1;
-    int nstats = dom->def->ndisks;
-    qemuBlockStatsPtr stats = NULL;
+    int rc;
+    virHashTablePtr stats = NULL;
     qemuDomainObjPrivatePtr priv = dom->privateData;
 
     if (!HAVE_JOB(privflags) || !virDomainObjIsActive(dom))
         return 0; /* it's ok, just go ahead silently */
 
-    if (VIR_ALLOC_N(stats, nstats) < 0)
-        return -1;
-
     qemuDomainObjEnterMonitor(driver, dom);
-
-    nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, NULL,
-                                             stats, nstats);
-
+    rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats);
     qemuDomainObjExitMonitor(driver, dom);
 
-    if (nstats < 0) {
+    if (rc < 0) {
         virResetLastError();
         ret = 0; /* still ok, again go ahead silently */
         goto cleanup;
@@ -17947,32 +17941,38 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
 
     QEMU_ADD_COUNT_PARAM(record, maxparams, "block", dom->def->ndisks);
 
-    for (i = 0; i < nstats; i++) {
-        QEMU_ADD_NAME_PARAM(record, maxparams,
-                            "block", i, dom->def->disks[i]->dst);
+    for (i = 0; i < dom->def->ndisks; i++) {
+        qemuBlockStats *entry;
+        virDomainDiskDefPtr disk = dom->def->disks[i];
+
+        QEMU_ADD_NAME_PARAM(record, maxparams, "block", i, disk->dst);
+
+        if (!disk->info.alias ||
+            !(entry = virHashLookup(stats, disk->info.alias)))
+            continue;
 
         QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
-                                "rd.reqs", stats[i].rd_req);
+                                "rd.reqs", entry->rd_req);
         QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
-                                "rd.bytes", stats[i].rd_bytes);
+                                "rd.bytes", entry->rd_bytes);
         QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
-                                "rd.times", stats[i].rd_total_times);
+                                "rd.times", entry->rd_total_times);
         QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
-                                "wr.reqs", stats[i].wr_req);
+                                "wr.reqs", entry->wr_req);
         QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
-                                "wr.bytes", stats[i].wr_bytes);
+                                "wr.bytes", entry->wr_bytes);
         QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
-                                "wr.times", stats[i].wr_total_times);
+                                "wr.times", entry->wr_total_times);
         QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
-                                "fl.reqs", stats[i].flush_req);
+                                "fl.reqs", entry->flush_req);
         QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, i,
-                                "fl.times", stats[i].flush_total_times);
+                                "fl.times", entry->flush_total_times);
     }
 
     ret = 0;
 
  cleanup:
-    VIR_FREE(stats);
+    virHashFree(stats);
     return ret;
 }
 

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index c25f00208e0c90c2f8e5f238a3fec24ae900868f..543384dc1c9b34596d0ae07b82bbce4bce60ded8 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1763,23 +1763,17 @@ int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
  */
 int
 qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
-                                const char *dev_name,
-                                qemuBlockStatsPtr stats,
-                                int nstats)
+                                virHashTablePtr *ret_stats)
 {
-    int ret;
-    VIR_DEBUG("mon=%p dev=%s", mon, dev_name);
+    VIR_DEBUG("mon=%p ret_stats=%p", mon, ret_stats);
 
-    if (mon->json) {
-        ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name,
-                                                  stats, nstats);
-    } else {
+    if (!mon->json) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("unable to query all block stats with this QEMU"));
         return -1;
     }
 
-    return ret;
+    return qemuMonitorJSONGetAllBlockStatsInfo(mon, ret_stats);
 }
 
 /* Return 0 and update @nparams with the number of block stats

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 6b91e299085d0e1e2fa3b74cd5251220902f3c99..adf18ab5e0b44c462401981a3c4f96ca770ad912 100644 (file)
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -361,10 +361,8 @@ struct _qemuBlockStats {
 };
 
 int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
-                                    const char *dev_name,
-                                    qemuBlockStatsPtr stats,
-                                    int nstats)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
+                                    virHashTablePtr *ret_stats)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon,
                                          int *nparams);

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a3d7c2c7900fc5c8b4c0c98d23a736508303829d..150ff7639ed31c0c4aa755c9d0c10c0351a2dba7 100644 (file)
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1734,7 +1734,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
                                      long long *flush_total_times,
                                      long long *errs)
 {
-    qemuBlockStats stats;
+    qemuBlockStats *stats;
+    virHashTablePtr blockstats = NULL;
     int ret = -1;
 
     *rd_req = *rd_bytes = -1;
@@ -1749,56 +1750,61 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
     if (flush_total_times)
         *flush_total_times = -1;
 
-    if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, &stats, 1) != 1)
+    if (qemuMonitorJSONGetAllBlockStatsInfo(mon, &blockstats) < 0)
         goto cleanup;
 
-    *rd_req = stats.rd_req;
-    *rd_bytes = stats.rd_bytes;
-    *wr_req = stats.wr_req;
-    *wr_bytes = stats.wr_bytes;
+    if (!(stats = virHashLookup(blockstats, dev_name))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("cannot find statistics for device '%s'"), dev_name);
+        goto cleanup;
+    }
+
+    *rd_req = stats->rd_req;
+    *rd_bytes = stats->rd_bytes;
+    *wr_req = stats->wr_req;
+    *wr_bytes = stats->wr_bytes;
     *errs = -1; /* QEMU does not have this */
 
     if (rd_total_times)
-        *rd_total_times = stats.rd_total_times;
+        *rd_total_times = stats->rd_total_times;
     if (wr_total_times)
-        *wr_total_times = stats.wr_total_times;
+        *wr_total_times = stats->wr_total_times;
     if (flush_req)
-        *flush_req = stats.flush_req;
+        *flush_req = stats->flush_req;
     if (flush_total_times)
-        *flush_total_times = stats.flush_total_times;
+        *flush_total_times = stats->flush_total_times;
 
     ret = 0;
 
  cleanup:
+    virHashFree(blockstats);
     return ret;
 }
 
 
 int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
-                                        const char *dev_name,
-                                        qemuBlockStatsPtr bstats,
-                                        int nstats)
+                                        virHashTablePtr *ret_stats)
 {
-    int ret, count;
+    int ret = -1;
+    int rc;
     size_t i;
-    virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-blockstats",
-                                                     NULL);
+    virJSONValuePtr cmd;
     virJSONValuePtr reply = NULL;
     virJSONValuePtr devices;
+    qemuBlockStatsPtr bstats = NULL;
+    virHashTablePtr hash = NULL;
 
-    if (!cmd)
+    if (!(cmd = qemuMonitorJSONMakeCommand("query-blockstats", NULL)))
         return -1;
 
-    if (!bstats || nstats <= 0)
-        return -1;
+    if (!(hash = virHashCreate(10, virHashValueFree)))
+        goto cleanup;
 
-    ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+    if ((rc = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
+        goto cleanup;
 
-    if (ret == 0)
-        ret = qemuMonitorJSONCheckError(cmd, reply);
-    if (ret < 0)
+    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
         goto cleanup;
-    ret = -1;
 
     devices = virJSONValueObjectGet(reply, "return");
     if (!devices || devices->type != VIR_JSON_TYPE_ARRAY) {
@@ -1807,10 +1813,14 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
         goto cleanup;
     }
 
-    count = 0;
-    for (i = 0; i < virJSONValueArraySize(devices) && count < nstats; i++) {
+    for (i = 0; i < virJSONValueArraySize(devices); i++) {
         virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
         virJSONValuePtr stats;
+        const char *devname;
+
+        if (VIR_ALLOC(bstats) < 0)
+            goto cleanup;
+
         if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("blockstats device entry was not "
@@ -1818,29 +1828,16 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
             goto cleanup;
         }
 
-        /* If dev_name is specified, we are looking for a specific device,
-         * so we must be stricter.
-         */
-        if (dev_name) {
-            const char *thisdev = virJSONValueObjectGetString(dev, "device");
-            if (!thisdev) {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("blockstats device entry was not "
-                                 "in expected format"));
-                goto cleanup;
-            }
-
-            /* New QEMU has separate names for host & guest side of the disk
-             * and libvirt gives the host side a 'drive-' prefix. The passed
-             * in dev_name is the guest side though
-             */
-            if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX))
-                thisdev += strlen(QEMU_DRIVE_HOST_PREFIX);
-
-            if (STRNEQ(thisdev, dev_name))
-                continue;
+        if (!(devname = virJSONValueObjectGetString(dev, "device"))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("blockstats device entry was not "
+                             "in expected format"));
+            goto cleanup;
         }
 
+        if (STRPREFIX(devname, QEMU_DRIVE_HOST_PREFIX))
+            devname += strlen(QEMU_DRIVE_HOST_PREFIX);
+
         if ((stats = virJSONValueObjectGet(dev, "stats")) == NULL ||
             stats->type != VIR_JSON_TYPE_OBJECT) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1910,22 +1907,18 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
             goto cleanup;
         }
 
-        count++;
-        bstats++;
-
-        if (dev_name && count)
-            break;
-    }
-
-    if (dev_name && !count) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("cannot find statistics for device '%s'"), dev_name);
-        goto cleanup;
+        if (virHashAddEntry(hash, devname, bstats) < 0)
+            goto cleanup;
+        bstats = NULL;
     }
 
-    ret = count;
+    *ret_stats = hash;
+    hash = NULL;
+    ret = 0;
 
  cleanup:
+    VIR_FREE(bstats);
+    virHashFree(hash);
     virJSONValueFree(cmd);
     virJSONValueFree(reply);
     return ret;

diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index d366179ea8bcb3a7fd55995002d270651e57c0e0..ef9b552565a5d0325d972c15a61e3fa4b683f2a1 100644 (file)
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -80,9 +80,7 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
                                      long long *flush_total_times,
                                      long long *errs);
 int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
-                                        const char *dev_name,
-                                        qemuBlockStatsPtr stats,
-                                        int nstats);
+                                        virHashTablePtr *ret_stats);
 int qemuMonitorJSONGetBlockStatsParamsNumber(qemuMonitorPtr mon,
                                              int *nparams);
 int qemuMonitorJSONGetBlockExtent(qemuMonitorPtr mon,