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

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(a)redhat.com&gt; > --- > 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:-)