Re: [libvirt PATCH v2 07/10] nodedev: Add testing for 'mdevctl start'

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(a)redhat.com&gt; > --- > 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