On 6/11/20 3:40 PM, Erik Skultety wrote:
On Wed, Jun 10, 2020 at 08:01:47PM +0200, Michal Privoznik wrote:
On 6/9/20 11:43 PM, Jonathon Jongsma wrote:
Test that we run 'mdevctl' with the proper arguments when creating new
mediated devices with virNodeDeviceCreateXML().
Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
---
build-aux/syntax-check.mk | 2 +-
tests/Makefile.am | 14 +
...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 1 +
...019_36ea_4111_8f0a_8c9a70e21366-start.json | 1 +
...d39_495e_4243_ad9f_beb3f14c23d9-start.argv | 1 +
...d39_495e_4243_ad9f_beb3f14c23d9-start.json | 1 +
...916_1ca8_49ac_b176_871d16c13076-start.argv | 1 +
...916_1ca8_49ac_b176_871d16c13076-start.json | 1 +
tests/nodedevmdevctltest.c | 257 ++++++++++++++++++
...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 8 +
...v_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml | 10 +
...v_fedc4916_1ca8_49ac_b176_871d16c13076.xml | 9 +
12 files changed, 305 insertions(+), 1 deletion(-)
create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv
create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json
create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv
create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json
create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv
create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json
create mode 100644 tests/nodedevmdevctltest.c
create mode 100644 tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml
create mode 100644 tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml
create mode 100644 tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index bf8832a2a5..d47a92b530 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -2015,7 +2015,7 @@ exclude_file_name_regexp--sc_prohibit_close = \
(\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$)
exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \
- (^tests/(virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$)
+ (^tests/(nodedevmdevctl|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$)
exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
(^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f5766a7790..13cbdbb31e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -388,6 +388,10 @@ test_programs += storagevolxml2xmltest
test_programs += nodedevxml2xmltest
+if WITH_NODE_DEVICES
+test_programs += nodedevmdevctltest
+endif WITH_NODE_DEVICES
+
test_programs += interfacexml2xmltest
test_programs += cputest
@@ -970,6 +974,16 @@ nodedevxml2xmltest_SOURCES = \
testutils.c testutils.h
nodedevxml2xmltest_LDADD = $(LDADDS)
+if WITH_NODE_DEVICES
+nodedevmdevctltest_SOURCES = \
+ nodedevmdevctltest.c \
+ testutils.c testutils.h
+
+nodedevmdevctltest_LDADD = \
+ ../src/libvirt_driver_nodedev_impl.la \
+ $(LDADDS)
+endif WITH_NODE_DEVICES
+
interfacexml2xmltest_SOURCES = \
interfacexml2xmltest.c \
testutils.c testutils.h
diff --git a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv
new file mode 100644
index 0000000000..dae6dedf7f
--- /dev/null
+++ b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv
@@ -0,0 +1 @@
+/usr/sbin/mdevctl start -p 0000:00:02.0 --jsonfile /dev/stdin
This is a problem. If there is no mdevctl found during configure (my case),
then MDEVCTL is going to be plain "mdevctl". Which is okay for the nodedev
driver, because it uses virCommandRun() which uses virFindFileInPath() to
get the real path at runtime. But for tests it won't fly. Alternativelly, if
the mdevctl lived under say /bin or any other location than /usr/sbin the
expected output would be different.
In virnetdevbandwidthtest.c (which uses TC) I've worked around this by using
TC directly to construct expected output. However, I guess we can safely
assume that either mdevctl is present at build time so that its location is
Why can we assume it safely? config.log doesn't complain and since we didn't
put this in a BuildRequires stanza, even if you do dnf builddep, you won't get
the program. Note that the situation with TC is different because (if I
overlook gentoo), it is provided by iproute-tc on Fedora and iproute2 on Debian
which are installed by default, so TC will be always present. I don't
think we should silently assume anything here, I believe configure should
complain because otherwise the tests will be failing and the developer would
not know why and it would also be confusing since our gitlab pipelines would be
green.
Well, is iproute installed even for minimal installation? Because fedora
builders certainly don't have it:
https://kojipkgs.fedoraproject.org//packages/libvirt/6.1.0/4.fc32/data/logs/x86_64/build.log
checking for radvd... /usr/sbin/radvd
checking for tc... tc
(if tc was found then the full path would be printed, like it is for radvd)
and we certainly don't want to fail configure if mdevctl wasn't found.
What we can do then, is to generate the expected cmd line on the fly -
just like virnetdevbandwidthtest does, or skip argv[0] and compare only
the rest of the args.
Michal
