From d4aa5b09ee07feffef63e2459cd9d5d7e6d05699 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 15 Apr 2015 11:45:47 +0200 Subject: [PATCH 3/5] Cleanup "/sys/class/net" usage Throughout the code, we have several places need to construct a path somewhere in /sys/class/net/... They are not consistent and nearly each code piece invents its own way how to do it. So unify this by: 1) use virNetDevSysfsFile() wherever possible 2) At least use common macro SYSFS_NET_DIR declared in virnetdev.h at the rest of places which can't go with 1) Signed-off-by: Michal Privoznik (cherry picked from commit 96a21e975f4b9d2f66a6aee1a16188837c98f82c) --- src/Makefile.am | 4 ++-- src/parallels/parallels_network.c | 10 ++++------ src/util/virnetdev.c | 5 ++--- src/util/virnetdev.h | 1 + src/util/virnetdevbridge.c | 13 ++++++------- src/util/virnetdevmacvlan.c | 30 +++++++++++++++--------------- src/util/virnetdevveth.c | 2 +- 7 files changed, 31 insertions(+), 34 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index d38432e..dd3ca29 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1385,8 +1385,8 @@ noinst_LTLIBRARIES += libvirt_driver_parallels.la libvirt_la_BUILT_LIBADD += libvirt_driver_parallels.la libvirt_driver_parallels_la_CFLAGS = \ -I$(srcdir)/conf $(AM_CFLAGS) \ - $(PARALLELS_SDK_CFLAGS) -libvirt_driver_parallels_la_LIBADD = $(PARALLELS_SDK_LIBS) + $(PARALLELS_SDK_CFLAGS) $(LIBNL_CFLAGS) +libvirt_driver_parallels_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS) libvirt_driver_parallels_la_SOURCES = $(PARALLELS_DRIVER_SOURCES) endif WITH_PARALLELS diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index 3e7087d..b4ae737 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -28,6 +28,7 @@ #include "viralloc.h" #include "virerror.h" #include "virfile.h" +#include "virnetdev.h" #include "md5.h" #include "parallels_utils.h" #include "virstring.h" @@ -39,8 +40,6 @@ virReportErrorHelper(VIR_FROM_TEST, VIR_ERR_OPERATION_FAILED, __FILE__, \ __FUNCTION__, __LINE__, _("Can't parse prlctl output")) -#define SYSFS_NET_DIR "/sys/class/net" - static int parallelsGetBridgedNetInfo(virNetworkDefPtr def, virJSONValuePtr jobj) { const char *ifname; @@ -56,8 +55,7 @@ static int parallelsGetBridgedNetInfo(virNetworkDefPtr def, virJSONValuePtr jobj goto cleanup; } - if (virAsprintf(&bridgeLink, "%s/%s/brport/bridge", - SYSFS_NET_DIR, ifname) < 0) + if (virAsprintf(&bridgeLink, SYSFS_NET_DIR "%s/brport/bridge", ifname) < 0) goto cleanup; if (virFileResolveLink(bridgeLink, &bridgePath) < 0) { @@ -68,8 +66,8 @@ static int parallelsGetBridgedNetInfo(virNetworkDefPtr def, virJSONValuePtr jobj if (VIR_STRDUP(def->bridge, last_component(bridgePath)) < 0) goto cleanup; - if (virAsprintf(&bridgeAddressPath, "%s/%s/brport/bridge/address", - SYSFS_NET_DIR, ifname) < 0) + if (virAsprintf(&bridgeAddressPath, SYSFS_NET_DIR "%s/brport/bridge/address", + ifname) < 0) goto cleanup; if ((len = virFileReadAll(bridgeAddressPath, 18, &bridgeAddress)) < 0) { diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 4d90fd0..3ca7493 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1512,14 +1512,13 @@ int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, #ifdef __linux__ -# define NET_SYSFS "/sys/class/net/" int virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname, const char *file) { - if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) < 0) + if (virAsprintf(pf_sysfs_device_link, SYSFS_NET_DIR "%s/%s", ifname, file) < 0) return -1; return 0; } @@ -1529,7 +1528,7 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, const char *file) { - if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/device/%s", ifname, + if (virAsprintf(pf_sysfs_device_link, SYSFS_NET_DIR "%s/device/%s", ifname, file) < 0) return -1; return 0; diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 62039aa..5d90e2a 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -215,6 +215,7 @@ int virNetDevSetRcvAllMulti(const char *ifname, bool receive) int virNetDevGetRcvAllMulti(const char *ifname, bool *receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +# define SYSFS_NET_DIR "/sys/class/net/" int virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname, const char *file) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index d9dcc39..282746b 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -114,7 +114,6 @@ static int virNetDevBridgeCmd(const char *brname, #endif #if defined(HAVE_STRUCT_IFREQ) && defined(__linux__) -# define SYSFS_NET_DIR "/sys/class/net" /* * Bridge parameters can be set via sysfs on newish kernels, * or by ioctl on older kernels. Perhaps we could just use @@ -130,7 +129,7 @@ static int virNetDevBridgeSet(const char *brname, char *path = NULL; int ret = -1; - if (virAsprintf(&path, "%s/%s/bridge/%s", SYSFS_NET_DIR, brname, paramname) < 0) + if (virAsprintf(&path, SYSFS_NET_DIR "%s/bridge/%s", brname, paramname) < 0) return -1; if (virFileExists(path)) { @@ -177,7 +176,7 @@ static int virNetDevBridgeGet(const char *brname, char *path = NULL; int ret = -1; - if (virAsprintf(&path, "%s/%s/bridge/%s", SYSFS_NET_DIR, brname, paramname) < 0) + if (virAsprintf(&path, SYSFS_NET_DIR "%s/bridge/%s", brname, paramname) < 0) return -1; if (virFileExists(path)) { @@ -235,8 +234,8 @@ virNetDevBridgePortSet(const char *brname, snprintf(valuestr, sizeof(valuestr), "%lu", value); - if (virAsprintf(&path, "%s/%s/brif/%s/%s", - SYSFS_NET_DIR, brname, ifname, paramname) < 0) + if (virAsprintf(&path, SYSFS_NET_DIR "%s/brif/%s/%s", + brname, ifname, paramname) < 0) return -1; if (!virFileExists(path)) @@ -265,8 +264,8 @@ virNetDevBridgePortGet(const char *brname, char *valuestr = NULL; int ret = -1; - if (virAsprintf(&path, "%s/%s/brif/%s/%s", - SYSFS_NET_DIR, brname, ifname, paramname) < 0) + if (virAsprintf(&path, SYSFS_NET_DIR "%s/brif/%s/%s", + brname, ifname, paramname) < 0) return -1; if (virFileReadAll(path, INT_BUFSIZE_BOUND(unsigned long), &valuestr) < 0) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 72e1f65..d95eb3a 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -297,19 +297,15 @@ static int virNetDevMacVLanTapOpen(const char *ifname, int retries) { - FILE *file; - char path[64]; + int ret = -1; + FILE *file = NULL; + char *path; int ifindex; char tapname[50]; int tapfd; - if (snprintf(path, sizeof(path), - "/sys/class/net/%s/ifindex", ifname) >= sizeof(path)) { - virReportSystemError(errno, - "%s", - _("buffer for ifindex path is too small")); + if (virNetDevSysfsFile(&path, ifname, "ifindex") < 0) return -1; - } file = fopen(path, "r"); @@ -317,15 +313,14 @@ int virNetDevMacVLanTapOpen(const char *ifname, virReportSystemError(errno, _("cannot open macvtap file %s to determine " "interface index"), path); - return -1; + goto cleanup; } if (fscanf(file, "%d", &ifindex) != 1) { virReportSystemError(errno, "%s", _("cannot determine macvtap's tap device " "interface index")); - VIR_FORCE_FCLOSE(file); - return -1; + goto cleanup; } VIR_FORCE_FCLOSE(file); @@ -335,7 +330,7 @@ int virNetDevMacVLanTapOpen(const char *ifname, virReportSystemError(errno, "%s", _("internal buffer for tap device is too small")); - return -1; + goto cleanup; } while (1) { @@ -349,12 +344,17 @@ int virNetDevMacVLanTapOpen(const char *ifname, break; } - if (tapfd < 0) + if (tapfd < 0) { virReportSystemError(errno, _("cannot open macvtap tap device %s"), tapname); - - return tapfd; + goto cleanup; + } + ret = tapfd; + cleanup: + VIR_FREE(path); + VIR_FORCE_FCLOSE(file); + return ret; } diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c index e9d6f9c..6905168 100644 --- a/src/util/virnetdevveth.c +++ b/src/util/virnetdevveth.c @@ -47,7 +47,7 @@ static int virNetDevVethExists(int devNum) { int ret; char *path = NULL; - if (virAsprintf(&path, "/sys/class/net/vnet%d/", devNum) < 0) + if (virAsprintf(&path, SYSFS_NET_DIR "vnet%d/", devNum) < 0) return -1; ret = virFileExists(path) ? 1 : 0; VIR_DEBUG("Checked dev vnet%d usage: %d", devNum, ret); -- 2.4.1