[libvirt] [PATCH 1/3] virNetDevOpenvswitchInterfaceStats: Optimize for speed

[Michal Privoznik]

We run 'ovs-vsctl' nine times (first to find if interface is
there and then eight times = for each stats member separately).
This is very inefficient. I've found a way to run it once and
with a bit of help from virJSON module we can parse out stats
we need.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/util/virnetdevopenvswitch.c | 111 +++++++++++++++++++++-----------
 1 file changed, 74 insertions(+), 37 deletions(-)

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index c99ecfbf15..0fe64bedab 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -28,6 +28,7 @@
 #include "virmacaddr.h"
 #include "virstring.h"
 #include "virlog.h"
+#include "virjson.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -311,58 +312,94 @@ int
 virNetDevOpenvswitchInterfaceStats(const char *ifname,
                                    virDomainInterfaceStatsPtr stats)
 {
-    char *tmp;
-    bool gotStats = false;
     VIR_AUTOPTR(virCommand) cmd = NULL;
     VIR_AUTOFREE(char *) output = NULL;
+    VIR_AUTOPTR(virJSONValue) jsonStats = NULL;
+    virJSONValuePtr jsonMap = NULL;
+    size_t i;
 
-    /* Just ensure the interface exists in ovs */
     cmd = virCommandNew(OVSVSCTL);
     virNetDevOpenvswitchAddTimeout(cmd);
-    virCommandAddArgList(cmd, "get", "Interface", ifname, "name", NULL);
+    virCommandAddArgList(cmd, "--if-exists", "--format=list", "--data=json",
+                         "--no-headings", "--columns=statistics", "list",
+                         "Interface", ifname, NULL);
     virCommandSetOutputBuffer(cmd, &output);
 
-    if (virCommandRun(cmd, NULL) < 0) {
+    /* The above command returns either:
+     * 1) empty string if @ifname doesn't exist, or
+     * 2) a JSON array, for instance:
+     *    ["map",[["collisions",0],["rx_bytes",0],["rx_crc_err",0],["rx_dropped",0],
+     *    ["rx_errors",0],["rx_frame_err",0],["rx_over_err",0],["rx_packets",0],
+     *    ["tx_bytes",12406],["tx_dropped",0],["tx_errors",0],["tx_packets",173]]]
+     */
+
+    if (virCommandRun(cmd, NULL) < 0 ||
+        STREQ_NULLABLE(output, "")) {
         /* no ovs-vsctl or interface 'ifname' doesn't exists in ovs */
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Interface not found"));
         return -1;
     }
 
-#define GET_STAT(name, member) \
-    do { \
-        VIR_FREE(output); \
-        virCommandFree(cmd); \
-        cmd = virCommandNew(OVSVSCTL); \
-        virNetDevOpenvswitchAddTimeout(cmd); \
-        virCommandAddArgList(cmd, "--if-exists", "get", "Interface", \
-                             ifname, "statistics:" name, NULL); \
-        virCommandSetOutputBuffer(cmd, &output); \
-        if (virCommandRun(cmd, NULL) < 0 || !output || !*output || *output == '\n') { \
-            stats->member = -1; \
-        } else { \
-            if (virStrToLong_ll(output, &tmp, 10, &stats->member) < 0 || \
-                *tmp != '\n') { \
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \
-                               _("Fail to parse ovs-vsctl output")); \
-                return -1; \
-            } \
-            gotStats = true; \
-        } \
-    } while (0)
+    if (!(jsonStats = virJSONValueFromString(output)) ||
+        !virJSONValueIsArray(jsonStats) ||
+        !(jsonMap = virJSONValueArrayGet(jsonStats, 1))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Unable to parse ovs-vsctl output"));
+        return -1;
+    }
 
-    /* The TX/RX fields appear to be swapped here
-     * because this is the host view. */
-    GET_STAT("rx_bytes", tx_bytes);
-    GET_STAT("rx_packets", tx_packets);
-    GET_STAT("rx_errors", tx_errs);
-    GET_STAT("rx_dropped", tx_drop);
-    GET_STAT("tx_bytes", rx_bytes);
-    GET_STAT("tx_packets", rx_packets);
-    GET_STAT("tx_errors", rx_errs);
-    GET_STAT("tx_dropped", rx_drop);
+    stats->rx_bytes = stats->rx_packets = stats->rx_errs = stats->rx_drop = -1;
+    stats->tx_bytes = stats->tx_packets = stats->tx_errs = stats->tx_drop = -1;
 
-    if (!gotStats) {
+    for (i = 0; i < virJSONValueArraySize(jsonMap); i++) {
+        virJSONValuePtr item = virJSONValueArrayGet(jsonMap, i);
+        virJSONValuePtr jsonKey;
+        virJSONValuePtr jsonVal;
+        const char *key;
+        long long val;
+
+        if (!item ||
+            (!(jsonKey = virJSONValueArrayGet(item, 0))) ||
+            (!(jsonVal = virJSONValueArrayGet(item, 1))) ||
+            (!(key = virJSONValueGetString(jsonKey))) ||
+            (virJSONValueGetNumberLong(jsonVal, &val) < 0)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Malformed ovs-vsctl output"));
+            return -1;
+        }
+
+        /* The TX/RX fields appear to be swapped here
+         * because this is the host view. */
+        if (STREQ(key, "rx_bytes")) {
+            stats->tx_bytes = val;
+        } else if (STREQ(key, "rx_packets")) {
+            stats->tx_packets = val;
+        } else if (STREQ(key, "rx_errors")) {
+            stats->tx_errs = val;
+        } else if (STREQ(key, "rx_dropped")) {
+            stats->tx_drop = val;
+        } else if (STREQ(key, "tx_bytes")) {
+            stats->rx_bytes = val;
+        } else if (STREQ(key, "tx_packets")) {
+            stats->rx_packets = val;
+        } else if (STREQ(key, "tx_errors")) {
+            stats->rx_errs = val;
+        } else if (STREQ(key, "tx_dropped")) {
+            stats->rx_drop = val;
+        } else {
+            VIR_DEBUG("Unused ovs-vsctl stat key=%s val=%lld", key, val);
+        }
+    }
+
+    if (stats->rx_bytes == -1 &&
+        stats->rx_packets == -1 &&
+        stats->rx_errs == -1 &&
+        stats->rx_drop == -1 &&
+        stats->tx_bytes == -1 &&
+        stats->tx_packets == -1 &&
+        stats->tx_errs == -1 &&
+        stats->tx_drop == -1) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Interface doesn't have any statistics"));
         return -1;
-- 
2.21.0