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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
> + */
> +
> +#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@xxxxxxxxxx>
> + */
> +
> +#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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list