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

Ján Tomko jtomko at redhat.com
Fri Jul 12 16:28:52 UTC 2019


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 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);

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190712/0f72752d/attachment-0001.sig>


More information about the libvir-list mailing list