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

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

 



On Wed, Jul 03, 2019 at 09:19:18AM +0200, Michal Privoznik wrote:
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@xxxxxxxxxx>
---
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);

Can we assume these options are present in all the supported versions of
ovs-vsctl?

    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]]]
+     */
+

This example would look better in tests/ where you test this function
against actual output O:-)

+    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;
    }


Jano

Attachment: signature.asc
Description: PGP signature

--
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]

  Powered by Linux