Re: [libvirt] [PATCH 2/2] Cleanup "/sys/class/net" usage

On 04/15/2015 05:51 AM, Michal Privoznik wrote: > 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 <mprivozn(a)redhat.com&gt; > --- > src/Makefile.am | 4 ++-- > src/parallels/parallels_network.c | 3 +-- > src/util/virnetdev.c | 5 ++--- > src/util/virnetdev.h | 2 ++ > src/util/virnetdevbridge.c | 1 - > src/util/virnetdevmacvlan.c | 28 ++++++++++++++-------------- > 6 files changed, 21 insertions(+), 22 deletions(-) > One more place w/ /sys/class/net: src/util/virnetdevveth.c (cscope egrep search) > diff --git a/src/Makefile.am b/src/Makefile.am > index 91a4c17..3c9eac6 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -1383,8 +1383,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 47f4886..47353a7 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; Technically - looks fine; however, does requiring the parallels driver to link against libnl just to get a symbol seem reasonable? It's another dependency right? I would think it's 'safer' to change the call: - if (virAsprintf(&bridgeLink, "%s/%s/brport/bridge", - SYSFS_NET_DIR, ifname) < 0) + if (virNetDevSysfsFile(&bridgeLink, ifname, "brport/bridge")) < 0)