From 263a88806ca9bcf25de5a7ac6bc3c40371950f01 Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Wed, 1 Jun 2016 11:52:53 +0200
Subject: [PATCH] virNetDevBridgeGet: Don't require users to
 virNetDevSetupControl

So far, this function has just three callers. Two of them call
virNetDevSetupControl to create a socket that we can then
optionally use for ioctl() to fetch data. However, querying sysfs
is preferred. Therefore it doesn't make much sense to require
users to set up the socket if they don't even know it will be
used in favour of sysfs. We can set up the socket iff we need to.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virnetdevbridge.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index ef1f4cc42f..ebe9dba9c7 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -169,12 +169,12 @@ static int virNetDevBridgeSet(const char *brname,
 
 static int virNetDevBridgeGet(const char *brname,
                               const char *paramname,  /* sysfs param name */
-                              unsigned long *value,   /* current value */
-                              int fd,                 /* control socket */
-                              struct ifreq *ifr)      /* pre-filled bridge name */
+                              unsigned long *value)   /* current value */
 {
     char *path = NULL;
     int ret = -1;
+    int fd = -1;
+    struct ifreq ifr;
 
     if (virAsprintf(&path, SYSFS_NET_DIR "%s/bridge/%s", brname, paramname) < 0)
         return -1;
@@ -196,7 +196,11 @@ static int virNetDevBridgeGet(const char *brname,
     } else {
         struct __bridge_info info;
         unsigned long args[] = { BRCTL_GET_BRIDGE_INFO, (unsigned long)&info, 0, 0 };
-        ifr->ifr_data = (char*)&args;
+
+        if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
+            goto cleanup;
+
+        ifr.ifr_data = (char*)&args;
         if (ioctl(fd, SIOCDEVPRIVATE, ifr) < 0) {
             virReportSystemError(errno,
                                  _("Unable to get bridge %s %s"), brname, paramname);
@@ -216,6 +220,7 @@ static int virNetDevBridgeGet(const char *brname,
 
     ret = 0;
  cleanup:
+    VIR_FORCE_CLOSE(fd);
     VIR_FREE(path);
     return ret;
 }
@@ -825,20 +830,12 @@ int virNetDevBridgeSetSTPDelay(const char *brname,
 int virNetDevBridgeGetSTPDelay(const char *brname,
                                int *delayms)
 {
-    int fd = -1;
     int ret = -1;
-    struct ifreq ifr;
     unsigned long val;
 
-    if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
-        goto cleanup;
-
-    ret = virNetDevBridgeGet(brname, "forward_delay", &val,
-                             fd, &ifr);
+    ret = virNetDevBridgeGet(brname, "forward_delay", &val);
     *delayms = JIFFIES_TO_MS(val);
 
- cleanup:
-    VIR_FORCE_CLOSE(fd);
     return ret;
 }
 
@@ -885,20 +882,12 @@ int virNetDevBridgeSetSTP(const char *brname,
 int virNetDevBridgeGetSTP(const char *brname,
                           bool *enabled)
 {
-    int fd = -1;
     int ret = -1;
-    struct ifreq ifr;
     unsigned long val;
 
-    if ((fd = virNetDevSetupControl(brname, &ifr)) < 0)
-        goto cleanup;
-
-    ret = virNetDevBridgeGet(brname, "stp_state", &val,
-                             fd, &ifr);
+    ret = virNetDevBridgeGet(brname, "stp_state", &val);
     *enabled = val ? true : false;
 
- cleanup:
-    VIR_FORCE_CLOSE(fd);
     return ret;
 }
 #elif defined(HAVE_BSD_BRIDGE_MGMT)
@@ -1000,7 +989,7 @@ virNetDevBridgeGetVlanFiltering(const char *brname,
     int ret = -1;
     unsigned long value;
 
-    if (virNetDevBridgeGet(brname, "vlan_filtering", &value, -1, NULL) < 0)
+    if (virNetDevBridgeGet(brname, "vlan_filtering", &value) < 0)
         goto cleanup;
 
     *enable = !!value;
-- 
GitLab