On Wed, 2020-06-10 at 20:01 +0200, Michal Privoznik wrote:
>
> > new file mode 100644
> > index 0000000000..dae6dedf7f
> > --- /dev/null
> > +++
> > b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e2136
> > 6-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 detected properly, or we hardcode its path. IOW, this
> needs
> to be squashed into 05/10:
>
>
> diff --git i/m4/virt-external-programs.m4 w/m4/virt-external-
> programs.m4
> index bd3cb1f757..f642dcbf0e 100644
> --- i/m4/virt-external-programs.m4
> +++ w/m4/virt-external-programs.m4
> @@ -65,7 +65,7 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
> AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl],
> [$LIBVIRT_SBIN_PATH])
> AC_PATH_PROG([SCRUB], [scrub], [scrub], [$LIBVIRT_SBIN_PATH])
> AC_PATH_PROG([ADDR2LINE], [addr2line], [addr2line],
> [$LIBVIRT_SBIN_PATH])
> - AC_PATH_PROG([MDEVCTL], [mdevctl], [mdevctl],
> [$LIBVIRT_SBIN_PATH])
> + AC_PATH_PROG([MDEVCTL], [mdevctl], [/usr/sbin/mdevctl],
> [$LIBVIRT_SBIN_PATH])
>
> AC_DEFINE_UNQUOTED([DMIDECODE], ["$DMIDECODE"],
> [Location or name of the dmidecode program])
That's a good point. But is this adequate? What if there's a distro
that installs mdevctl into /usr/bin instead of /usr/sbin. Configure
will find the /usr/bin/mdevctl binary and the test output will not
match... How do we get around that issue for other argv tests? (e.g.
qemu?) Or do we just assume that tests will fail on machines that
install things in non-standard locations?
> > +/* Bare minimum driver init to be able to test nodedev
> > functionality */
> > +static int
> > +nodedevTestDriverInit(void)
> > +{
> > + int ret = -1;
> > + char statedir[] = abs_builddir "/nodedevstatedir-XXXXXX";
> > + if (VIR_ALLOC(driver) < 0)
> > + return -1;
> > +
> > + if (!g_mkdtemp(statedir)) {
> > + fprintf(stderr, "Cannot create fake stateDir");
> > + goto error;
> > + }
>
> This creates a temporary dir which is never removed. But it doesn't
> look
> like the directory is needed at all, is it?
Oh yes, this was leftover from the previous version of the patch where
I created a temp file for the json config in the state dir. It can be
dropped.
Jonathon
