[libvirt] [PATCH] Introduce virnetdevtest
This is yet another test for check of basic functionality of our NIC state handling code. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virnetdev.c | 4 +- src/util/virnetdev.h | 4 ++ tests/Makefile.am | 15 +++++ tests/virnetdevmock.c | 48 ++++++++++++++ tests/virnetdevtest.c | 94 +++++++++++++++++++++++++++ tests/virnetdevtestdata/eth0-broken/operstate | 1 + tests/virnetdevtestdata/eth0-broken/speed | 1 + tests/virnetdevtestdata/eth0/operstate | 1 + tests/virnetdevtestdata/eth0/speed | 1 + tests/virnetdevtestdata/lo/operstate | 1 + tests/virnetdevtestdata/lo/speed | 1 + 12 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 tests/virnetdevmock.c create mode 100644 tests/virnetdevtest.c create mode 100644 tests/virnetdevtestdata/eth0-broken/operstate create mode 100644 tests/virnetdevtestdata/eth0-broken/speed create mode 100644 tests/virnetdevtestdata/eth0/operstate create mode 100644 tests/virnetdevtestdata/eth0/speed create mode 100644 tests/virnetdevtestdata/lo/operstate create mode 100644 tests/virnetdevtestdata/lo/speed diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9f82926..0b42238 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1741,6 +1741,7 @@ virNetDevSetPromiscuous; virNetDevSetRcvAllMulti; virNetDevSetRcvMulti; virNetDevSetupControl; +virNetDevSysfsFile; virNetDevValidateConfig; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 54d866e..a2d55a8 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1519,9 +1519,9 @@ int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, #ifdef __linux__ # define NET_SYSFS "/sys/class/net/" -static int +int virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname, - const char *file) + const char *file) { if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) < 0) diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 856127b..999a89a 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -219,4 +219,8 @@ int virNetDevSetRcvAllMulti(const char *ifname, bool receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevGetRcvAllMulti(const char *ifname, bool *receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevSysfsFile(char **pf_sysfs_device_link, + const char *ifname, + const char *file) + ATTRIBUTE_NONNULL(1); #endif /* __VIR_NETDEV_H__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 046cd08..9ebedc3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -176,6 +176,7 @@ test_programs = virshtest sockettest \ domainconftest \ virhostdevtest \ vircaps2xmltest \ + virnetdevtest \ $(NULL) if WITH_REMOTE @@ -402,6 +403,7 @@ test_libraries = libshunload.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ + virnetdevmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -1029,6 +1031,19 @@ virpcimock_la_LIBADD = $(GNULIB_LIBS) \ virpcimock_la_LDFLAGS = -module -avoid-version \ -rpath /evil/libtool/hack/to/force/shared/lib/creation +virnetdevtest_SOURCES = \ + virnetdevtest.c testutils.h testutils.c +virnetdevtest_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS) +virnetdevtest_LDADD = $(LDADDS) + +virnetdevmock_la_SOURCES = \ + virnetdevmock.c +virnetdevmock_la_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS) +virnetdevmock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la +virnetdevmock_la_LDFLAGS = -module -avoid-version \ + -rpath /evil/libtool/hack/to/force/shared/lib/creation + if WITH_LINUX virusbtest_SOURCES = \ virusbtest.c testutils.h testutils.c diff --git a/tests/virnetdevmock.c b/tests/virnetdevmock.c new file mode 100644 index 0000000..681e5fe --- /dev/null +++ b/tests/virnetdevmock.c @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#ifdef __linux__ +# include "internal.h" +# include <stdlib.h> +# include <stdio.h> +# include "virstring.h" +# include "virnetdev.h" + +# define NET_DEV_TEST_DATA_PREFIX abs_srcdir "/virnetdevtestdata" + +int +virNetDevSysfsFile(char **pf_sysfs_device_link, + const char *ifname, + const char *file) +{ + + if (virAsprintfQuiet(pf_sysfs_device_link, "%s/%s/%s", + NET_DEV_TEST_DATA_PREFIX, ifname, file) < 0) { + fprintf(stderr, "Out of memory\n"); + abort(); + } + + return 0; +} +#else +/* Nothing to override on non-__linux__ platforms */ +#endif diff --git a/tests/virnetdevtest.c b/tests/virnetdevtest.c new file mode 100644 index 0000000..8795bf1 --- /dev/null +++ b/tests/virnetdevtest.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#include "testutils.h" + +#ifdef __linux__ + +# include "virnetdev.h" + +# define VIR_FROM_THIS VIR_FROM_NONE + +struct testVirNetDevGetLinkInfoData { + const char *ifname; /* ifname to get info on */ + virInterfaceState state; /* expected state */ + unsigned int speed; /* expected speed */ +}; + +static int +testVirNetDevGetLinkInfo(const void *opaque) +{ + int ret = -1; + const struct testVirNetDevGetLinkInfoData *data = opaque; + virInterfaceLink lnk; + + if (virNetDevGetLinkInfo(data->ifname, &lnk) < 0) + goto cleanup; + + if (lnk.state != data->state) { + fprintf(stderr, + "Fetched link state (%s) doesn't match the expected one (%s)", + virInterfaceStateTypeToString(lnk.state), + virInterfaceStateTypeToString(data->state)); + goto cleanup; + } + + if (lnk.speed != data->speed) { + fprintf(stderr, + "Fetched link speed (%u) doesn't match the expected one (%u)", + lnk.speed, data->speed); + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + +# define DO_TEST_LINK(ifname, state, speed) \ + do { \ + struct testVirNetDevGetLinkInfoData data = {ifname, state, speed}; \ + if (virtTestRun("Link info: " # ifname, \ + testVirNetDevGetLinkInfo, &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_LINK("eth0", VIR_INTERFACE_STATE_UP, 1000); + DO_TEST_LINK("lo", VIR_INTERFACE_STATE_UNKNOWN, 0); + DO_TEST_LINK("eth0-broken", VIR_INTERFACE_STATE_DOWN, 0); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdevmock.so") +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/virnetdevtestdata/eth0-broken/operstate b/tests/virnetdevtestdata/eth0-broken/operstate new file mode 100644 index 0000000..eb0e904 --- /dev/null +++ b/tests/virnetdevtestdata/eth0-broken/operstate @@ -0,0 +1 @@ +down diff --git a/tests/virnetdevtestdata/eth0-broken/speed b/tests/virnetdevtestdata/eth0-broken/speed new file mode 100644 index 0000000..4f6ff86 --- /dev/null +++ b/tests/virnetdevtestdata/eth0-broken/speed @@ -0,0 +1 @@ +4294967295 diff --git a/tests/virnetdevtestdata/eth0/operstate b/tests/virnetdevtestdata/eth0/operstate new file mode 100644 index 0000000..e31ee94 --- /dev/null +++ b/tests/virnetdevtestdata/eth0/operstate @@ -0,0 +1 @@ +up diff --git a/tests/virnetdevtestdata/eth0/speed b/tests/virnetdevtestdata/eth0/speed new file mode 100644 index 0000000..83b33d2 --- /dev/null +++ b/tests/virnetdevtestdata/eth0/speed @@ -0,0 +1 @@ +1000 diff --git a/tests/virnetdevtestdata/lo/operstate b/tests/virnetdevtestdata/lo/operstate new file mode 100644 index 0000000..3546645 --- /dev/null +++ b/tests/virnetdevtestdata/lo/operstate @@ -0,0 +1 @@ +unknown diff --git a/tests/virnetdevtestdata/lo/speed b/tests/virnetdevtestdata/lo/speed new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virnetdevtestdata/lo/speed @@ -0,0 +1 @@ +0 -- 2.0.5
On 04/03/2015 08:36 AM, Michal Privoznik wrote:
This is yet another test for check of basic functionality of our NIC state handling code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virnetdev.c | 4 +- src/util/virnetdev.h | 4 ++ tests/Makefile.am | 15 +++++ tests/virnetdevmock.c | 48 ++++++++++++++ tests/virnetdevtest.c | 94 +++++++++++++++++++++++++++ tests/virnetdevtestdata/eth0-broken/operstate | 1 + tests/virnetdevtestdata/eth0-broken/speed | 1 + tests/virnetdevtestdata/eth0/operstate | 1 + tests/virnetdevtestdata/eth0/speed | 1 + tests/virnetdevtestdata/lo/operstate | 1 + tests/virnetdevtestdata/lo/speed | 1 + 12 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 tests/virnetdevmock.c create mode 100644 tests/virnetdevtest.c create mode 100644 tests/virnetdevtestdata/eth0-broken/operstate create mode 100644 tests/virnetdevtestdata/eth0-broken/speed create mode 100644 tests/virnetdevtestdata/eth0/operstate create mode 100644 tests/virnetdevtestdata/eth0/speed create mode 100644 tests/virnetdevtestdata/lo/operstate create mode 100644 tests/virnetdevtestdata/lo/speed
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9f82926..0b42238 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1741,6 +1741,7 @@ virNetDevSetPromiscuous; virNetDevSetRcvAllMulti; virNetDevSetRcvMulti; virNetDevSetupControl; +virNetDevSysfsFile; virNetDevValidateConfig;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 54d866e..a2d55a8 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1519,9 +1519,9 @@ int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, #ifdef __linux__ # define NET_SYSFS "/sys/class/net/"
-static int +int virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname, - const char *file) + const char *file) {
if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) < 0)
Not related specifically to this change, but there seems to be four more places which make up their own 'version' of this type of logic - two use 'SYSFS_NET_DIR' instead of 'NET_SYSFS' and two use the raw '/sys/class/net' path... Might be "nice" to have them use this function now too.
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 856127b..999a89a 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -219,4 +219,8 @@ int virNetDevSetRcvAllMulti(const char *ifname, bool receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevGetRcvAllMulti(const char *ifname, bool *receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevSysfsFile(char **pf_sysfs_device_link, + const char *ifname, + const char *file) + ATTRIBUTE_NONNULL(1);
Seems (2) and (3) should be checked as well.. Also, checked current callers and found all have whatever gets used as arg (2) and (3) checked for non null, except for one... The virNetDevGetVirtualFunctionInfo() prototype doesn't make sure that it's arg (2) is non-null, but goes ahead and derefs it right away...
#endif /* __VIR_NETDEV_H__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 046cd08..9ebedc3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -176,6 +176,7 @@ test_programs = virshtest sockettest \ domainconftest \ virhostdevtest \ vircaps2xmltest \ + virnetdevtest \ $(NULL)
if WITH_REMOTE @@ -402,6 +403,7 @@ test_libraries = libshunload.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ + virnetdevmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -1029,6 +1031,19 @@ virpcimock_la_LIBADD = $(GNULIB_LIBS) \ virpcimock_la_LDFLAGS = -module -avoid-version \ -rpath /evil/libtool/hack/to/force/shared/lib/creation
+virnetdevtest_SOURCES = \ + virnetdevtest.c testutils.h testutils.c +virnetdevtest_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS) +virnetdevtest_LDADD = $(LDADDS) + +virnetdevmock_la_SOURCES = \ + virnetdevmock.c +virnetdevmock_la_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS) +virnetdevmock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la +virnetdevmock_la_LDFLAGS = -module -avoid-version \ + -rpath /evil/libtool/hack/to/force/shared/lib/creation + if WITH_LINUX virusbtest_SOURCES = \ virusbtest.c testutils.h testutils.c diff --git a/tests/virnetdevmock.c b/tests/virnetdevmock.c new file mode 100644 index 0000000..681e5fe --- /dev/null +++ b/tests/virnetdevmock.c @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2013 Red Hat, Inc.
Been sitting on this awhile?
+ * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#ifdef __linux__ +# include "internal.h" +# include <stdlib.h> +# include <stdio.h> +# include "virstring.h" +# include "virnetdev.h" + +# define NET_DEV_TEST_DATA_PREFIX abs_srcdir "/virnetdevtestdata" + +int +virNetDevSysfsFile(char **pf_sysfs_device_link, + const char *ifname, + const char *file) +{ + + if (virAsprintfQuiet(pf_sysfs_device_link, "%s/%s/%s", + NET_DEV_TEST_DATA_PREFIX, ifname, file) < 0) { + fprintf(stderr, "Out of memory\n"); + abort(); + } + + return 0; +} +#else +/* Nothing to override on non-__linux__ platforms */ +#endif diff --git a/tests/virnetdevtest.c b/tests/virnetdevtest.c new file mode 100644 index 0000000..8795bf1 --- /dev/null +++ b/tests/virnetdevtest.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2014 Red Hat, Inc.
Ditto ACK for what's here with some minor adjustments - extra points for the additional changes to use the now non static function ;-) John
+ * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#include "testutils.h" + +#ifdef __linux__ + +# include "virnetdev.h" + +# define VIR_FROM_THIS VIR_FROM_NONE + +struct testVirNetDevGetLinkInfoData { + const char *ifname; /* ifname to get info on */ + virInterfaceState state; /* expected state */ + unsigned int speed; /* expected speed */ +}; + +static int +testVirNetDevGetLinkInfo(const void *opaque) +{ + int ret = -1; + const struct testVirNetDevGetLinkInfoData *data = opaque; + virInterfaceLink lnk; + + if (virNetDevGetLinkInfo(data->ifname, &lnk) < 0) + goto cleanup; + + if (lnk.state != data->state) { + fprintf(stderr, + "Fetched link state (%s) doesn't match the expected one (%s)", + virInterfaceStateTypeToString(lnk.state), + virInterfaceStateTypeToString(data->state)); + goto cleanup; + } + + if (lnk.speed != data->speed) { + fprintf(stderr, + "Fetched link speed (%u) doesn't match the expected one (%u)", + lnk.speed, data->speed); + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + +# define DO_TEST_LINK(ifname, state, speed) \ + do { \ + struct testVirNetDevGetLinkInfoData data = {ifname, state, speed}; \ + if (virtTestRun("Link info: " # ifname, \ + testVirNetDevGetLinkInfo, &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_LINK("eth0", VIR_INTERFACE_STATE_UP, 1000); + DO_TEST_LINK("lo", VIR_INTERFACE_STATE_UNKNOWN, 0); + DO_TEST_LINK("eth0-broken", VIR_INTERFACE_STATE_DOWN, 0); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdevmock.so") +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/virnetdevtestdata/eth0-broken/operstate b/tests/virnetdevtestdata/eth0-broken/operstate new file mode 100644 index 0000000..eb0e904 --- /dev/null +++ b/tests/virnetdevtestdata/eth0-broken/operstate @@ -0,0 +1 @@ +down diff --git a/tests/virnetdevtestdata/eth0-broken/speed b/tests/virnetdevtestdata/eth0-broken/speed new file mode 100644 index 0000000..4f6ff86 --- /dev/null +++ b/tests/virnetdevtestdata/eth0-broken/speed @@ -0,0 +1 @@ +4294967295 diff --git a/tests/virnetdevtestdata/eth0/operstate b/tests/virnetdevtestdata/eth0/operstate new file mode 100644 index 0000000..e31ee94 --- /dev/null +++ b/tests/virnetdevtestdata/eth0/operstate @@ -0,0 +1 @@ +up diff --git a/tests/virnetdevtestdata/eth0/speed b/tests/virnetdevtestdata/eth0/speed new file mode 100644 index 0000000..83b33d2 --- /dev/null +++ b/tests/virnetdevtestdata/eth0/speed @@ -0,0 +1 @@ +1000 diff --git a/tests/virnetdevtestdata/lo/operstate b/tests/virnetdevtestdata/lo/operstate new file mode 100644 index 0000000..3546645 --- /dev/null +++ b/tests/virnetdevtestdata/lo/operstate @@ -0,0 +1 @@ +unknown diff --git a/tests/virnetdevtestdata/lo/speed b/tests/virnetdevtestdata/lo/speed new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virnetdevtestdata/lo/speed @@ -0,0 +1 @@ +0
On 04/13/2015 11:11 AM, John Ferlan wrote:
On 04/03/2015 08:36 AM, Michal Privoznik wrote:
This is yet another test for check of basic functionality of our NIC state handling code.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virnetdev.c | 4 +- src/util/virnetdev.h | 4 ++ tests/Makefile.am | 15 +++++ tests/virnetdevmock.c | 48 ++++++++++++++ tests/virnetdevtest.c | 94 +++++++++++++++++++++++++++ tests/virnetdevtestdata/eth0-broken/operstate | 1 + tests/virnetdevtestdata/eth0-broken/speed | 1 + tests/virnetdevtestdata/eth0/operstate | 1 + tests/virnetdevtestdata/eth0/speed | 1 + tests/virnetdevtestdata/lo/operstate | 1 + tests/virnetdevtestdata/lo/speed | 1 + 12 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 tests/virnetdevmock.c create mode 100644 tests/virnetdevtest.c create mode 100644 tests/virnetdevtestdata/eth0-broken/operstate create mode 100644 tests/virnetdevtestdata/eth0-broken/speed create mode 100644 tests/virnetdevtestdata/eth0/operstate create mode 100644 tests/virnetdevtestdata/eth0/speed create mode 100644 tests/virnetdevtestdata/lo/operstate create mode 100644 tests/virnetdevtestdata/lo/speed
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9f82926..0b42238 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1741,6 +1741,7 @@ virNetDevSetPromiscuous; virNetDevSetRcvAllMulti; virNetDevSetRcvMulti; virNetDevSetupControl; +virNetDevSysfsFile; virNetDevValidateConfig;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 54d866e..a2d55a8 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1519,9 +1519,9 @@ int virNetDevValidateConfig(const char *ifname ATTRIBUTE_UNUSED, #ifdef __linux__ # define NET_SYSFS "/sys/class/net/"
-static int +int virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname, - const char *file) + const char *file) {
if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) < 0)
Not related specifically to this change, but there seems to be four more places which make up their own 'version' of this type of logic - two use 'SYSFS_NET_DIR' instead of 'NET_SYSFS' and two use the raw '/sys/class/net' path... Might be "nice" to have them use this function now too.
Well, the other two uses have parameters that would need to be passed in printf-style, so that complicates any standard function. Not that I would mind such a thing, but an in-between solution could be to provide the #define of the base directory in a .h file, or a function that returns that string.
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 856127b..999a89a 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -219,4 +219,8 @@ int virNetDevSetRcvAllMulti(const char *ifname, bool receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevGetRcvAllMulti(const char *ifname, bool *receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virNetDevSysfsFile(char **pf_sysfs_device_link, + const char *ifname, + const char *file) + ATTRIBUTE_NONNULL(1); Seems (2) and (3) should be checked as well..
"checked" seems like an incorrect description to me - I thought that the function of ATTRIBUTE_NONNULL ended up being exactly the opposite of that - if you mark an arg as ATTRIBUTE_NONNULL then the code within the function (and more importantly, static analyzers) can assume that the arg will always be non-null, so no checking is required, i.e. it is a more of an optimization aid than a debugging aid. And as a matter of fact, if you turn on optimization in gcc, any code in a function that checks for NULL in an ATTRIBUTE_NONNULL arg *will be removed* (or at least it *would have* without the 2nd patch I point out below). In my experience, ATTRIBUTE_NONNULL has done more to obscure failure to check for non-null than to point it out: https://www.redhat.com/archives/libvir-list/2012-April/msg01370.html although I *think* this patch from 2012 eliminated that failing: https://www.redhat.com/archives/libvir-list/2012-April/msg01376.html so maybe I'm blathering on about nothing ;-)
Also, checked current callers and found all have whatever gets used as arg (2) and (3) checked for non null, except for one...
The virNetDevGetVirtualFunctionInfo() prototype doesn't make sure that it's arg (2) is non-null, but goes ahead and derefs it right away...
#endif /* __VIR_NETDEV_H__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 046cd08..9ebedc3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -176,6 +176,7 @@ test_programs = virshtest sockettest \ domainconftest \ virhostdevtest \ vircaps2xmltest \ + virnetdevtest \ $(NULL)
if WITH_REMOTE @@ -402,6 +403,7 @@ test_libraries = libshunload.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ + virnetdevmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -1029,6 +1031,19 @@ virpcimock_la_LIBADD = $(GNULIB_LIBS) \ virpcimock_la_LDFLAGS = -module -avoid-version \ -rpath /evil/libtool/hack/to/force/shared/lib/creation
+virnetdevtest_SOURCES = \ + virnetdevtest.c testutils.h testutils.c +virnetdevtest_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS) +virnetdevtest_LDADD = $(LDADDS) + +virnetdevmock_la_SOURCES = \ + virnetdevmock.c +virnetdevmock_la_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS) +virnetdevmock_la_LIBADD = $(GNULIB_LIBS) \ + ../src/libvirt.la +virnetdevmock_la_LDFLAGS = -module -avoid-version \ + -rpath /evil/libtool/hack/to/force/shared/lib/creation + if WITH_LINUX virusbtest_SOURCES = \ virusbtest.c testutils.h testutils.c diff --git a/tests/virnetdevmock.c b/tests/virnetdevmock.c new file mode 100644 index 0000000..681e5fe --- /dev/null +++ b/tests/virnetdevmock.c @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2013 Red Hat, Inc. Been sitting on this awhile?
+ * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#ifdef __linux__ +# include "internal.h" +# include <stdlib.h> +# include <stdio.h> +# include "virstring.h" +# include "virnetdev.h" + +# define NET_DEV_TEST_DATA_PREFIX abs_srcdir "/virnetdevtestdata"
Maybe this could be abs_srcdir "/virnetdevtestdata/sys/class/net/" so that when/if the tests are expanded to use anything outside of /sys/class/net, all of the test data could be added under the same base directory.
+ +int +virNetDevSysfsFile(char **pf_sysfs_device_link, + const char *ifname, + const char *file) +{ + + if (virAsprintfQuiet(pf_sysfs_device_link, "%s/%s/%s", + NET_DEV_TEST_DATA_PREFIX, ifname, file) < 0) { + fprintf(stderr, "Out of memory\n"); + abort(); + } + + return 0; +} +#else +/* Nothing to override on non-__linux__ platforms */ +#endif diff --git a/tests/virnetdevtest.c b/tests/virnetdevtest.c new file mode 100644 index 0000000..8795bf1 --- /dev/null +++ b/tests/virnetdevtest.c @@ -0,0 +1,94 @@ +/* + * Copyright (C) 2014 Red Hat, Inc. Ditto
ACK for what's here with some minor adjustments - extra points for the additional changes to use the now non static function ;-)
John
+ * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#include "testutils.h" + +#ifdef __linux__ + +# include "virnetdev.h" + +# define VIR_FROM_THIS VIR_FROM_NONE + +struct testVirNetDevGetLinkInfoData { + const char *ifname; /* ifname to get info on */ + virInterfaceState state; /* expected state */ + unsigned int speed; /* expected speed */ +}; + +static int +testVirNetDevGetLinkInfo(const void *opaque) +{ + int ret = -1; + const struct testVirNetDevGetLinkInfoData *data = opaque; + virInterfaceLink lnk; + + if (virNetDevGetLinkInfo(data->ifname, &lnk) < 0) + goto cleanup; + + if (lnk.state != data->state) { + fprintf(stderr, + "Fetched link state (%s) doesn't match the expected one (%s)", + virInterfaceStateTypeToString(lnk.state), + virInterfaceStateTypeToString(data->state)); + goto cleanup; + } + + if (lnk.speed != data->speed) { + fprintf(stderr, + "Fetched link speed (%u) doesn't match the expected one (%u)", + lnk.speed, data->speed); + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + +# define DO_TEST_LINK(ifname, state, speed) \ + do { \ + struct testVirNetDevGetLinkInfoData data = {ifname, state, speed}; \ + if (virtTestRun("Link info: " # ifname, \ + testVirNetDevGetLinkInfo, &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_LINK("eth0", VIR_INTERFACE_STATE_UP, 1000); + DO_TEST_LINK("lo", VIR_INTERFACE_STATE_UNKNOWN, 0); + DO_TEST_LINK("eth0-broken", VIR_INTERFACE_STATE_DOWN, 0); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdevmock.so") +#else +int +main(void) +{ + return EXIT_AM_SKIP; +} +#endif diff --git a/tests/virnetdevtestdata/eth0-broken/operstate b/tests/virnetdevtestdata/eth0-broken/operstate new file mode 100644 index 0000000..eb0e904 --- /dev/null +++ b/tests/virnetdevtestdata/eth0-broken/operstate @@ -0,0 +1 @@ +down diff --git a/tests/virnetdevtestdata/eth0-broken/speed b/tests/virnetdevtestdata/eth0-broken/speed new file mode 100644 index 0000000..4f6ff86 --- /dev/null +++ b/tests/virnetdevtestdata/eth0-broken/speed @@ -0,0 +1 @@ +4294967295 diff --git a/tests/virnetdevtestdata/eth0/operstate b/tests/virnetdevtestdata/eth0/operstate new file mode 100644 index 0000000..e31ee94 --- /dev/null +++ b/tests/virnetdevtestdata/eth0/operstate @@ -0,0 +1 @@ +up diff --git a/tests/virnetdevtestdata/eth0/speed b/tests/virnetdevtestdata/eth0/speed new file mode 100644 index 0000000..83b33d2 --- /dev/null +++ b/tests/virnetdevtestdata/eth0/speed @@ -0,0 +1 @@ +1000 diff --git a/tests/virnetdevtestdata/lo/operstate b/tests/virnetdevtestdata/lo/operstate new file mode 100644 index 0000000..3546645 --- /dev/null +++ b/tests/virnetdevtestdata/lo/operstate @@ -0,0 +1 @@ +unknown diff --git a/tests/virnetdevtestdata/lo/speed b/tests/virnetdevtestdata/lo/speed new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/virnetdevtestdata/lo/speed @@ -0,0 +1 @@ +0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
John Ferlan -
Laine Stump -
Michal Privoznik