commit edaf13565 modified the stats retrieval for OVS interfaces to
not fail when one of the fields was unrecognized by the ovs-vsctl
command, but ovs-vsctl was still returning an error, and libvirt was
cluttering the logs with these inconsequential error messages.

This patch modifies the GET_STAT macro to add "--if-exists" to the
ovs-vsctl command, which causes it to return an empty string (and exit
with success) if the requested statistic isn't in its database, thus
eliminating the ugly error messages from the log.

Resolves: https://bugzilla.redhat.com/1683175

Signed-off-by: Laine Stump <laine@laine.org>
---
 src/util/virnetdevopenvswitch.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 4fa3a5742a..e5a45f59ec 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -335,10 +335,10 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
         virCommandFree(cmd); \
         cmd = virCommandNew(OVSVSCTL); \
         virNetDevOpenvswitchAddTimeout(cmd); \
-        virCommandAddArgList(cmd, "get", "Interface", ifname, \
-                             "statistics:" name, NULL); \
+        virCommandAddArgList(cmd, "--if-exists", "get", "Interface", \
+                             ifname, "statistics:" name, NULL); \
         virCommandSetOutputBuffer(cmd, &output); \
-        if (virCommandRun(cmd, NULL) < 0) { \
+        if (virCommandRun(cmd, NULL) < 0 || !*output || *output == '\n') { \
             stats->member = -1; \
         } else { \
             if (virStrToLong_ll(output, &tmp, 10, &stats->member) < 0 || \
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 3/27/19 8:04 PM, Laine Stump wrote:
> commit edaf13565 modified the stats retrieval for OVS interfaces to
> not fail when one of the fields was unrecognized by the ovs-vsctl
> command, but ovs-vsctl was still returning an error, and libvirt was
> cluttering the logs with these inconsequential error messages.
> 
> This patch modifies the GET_STAT macro to add "--if-exists" to the
> ovs-vsctl command, which causes it to return an empty string (and exit
> with success) if the requested statistic isn't in its database, thus
> eliminating the ugly error messages from the log.
> 
> Resolves: https://bugzilla.redhat.com/1683175
> 
> Signed-off-by: Laine Stump <laine@laine.org>
> ---
>   src/util/virnetdevopenvswitch.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
> index 4fa3a5742a..e5a45f59ec 100644
> --- a/src/util/virnetdevopenvswitch.c
> +++ b/src/util/virnetdevopenvswitch.c
> @@ -335,10 +335,10 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
>           virCommandFree(cmd); \
>           cmd = virCommandNew(OVSVSCTL); \
>           virNetDevOpenvswitchAddTimeout(cmd); \
> -        virCommandAddArgList(cmd, "get", "Interface", ifname, \
> -                             "statistics:" name, NULL); \
> +        virCommandAddArgList(cmd, "--if-exists", "get", "Interface", \
> +                             ifname, "statistics:" name, NULL); \
>           virCommandSetOutputBuffer(cmd, &output); \
> -        if (virCommandRun(cmd, NULL) < 0) { \
> +        if (virCommandRun(cmd, NULL) < 0 || !*output || *output == '\n') { \

I think we need '|| !output' before any of these @output checks. If 
virCommandRun() fails before it had a chance to allocate @output this 
would case NULL dereference.

>               stats->member = -1; \
>           } else { \
>               if (virStrToLong_ll(output, &tmp, 10, &stats->member) < 0 || \
> 

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list