On Wed, May 13, 2026 at 12:14:56PM +0200, Peter Krempa via Devel wrote:
> From: Peter Krempa <[email protected]>
>
> Some test cases (such as cases excercising FD passing in
> qemuxmlconftest) use dup2 to put FDs on certain numbers to have stable
> test output. If the environment the test program is run in
> "leaks"/passes some FDs the test may file in a very obscure way.
>
> Since the tests are designed to be standalone we can simply close
> everything except std(in|out|err) before running the tests to avoid
> unstable tests.
>
> Use virCommandFDMassClose to close everything except stdio.
>
> Signed-off-by: Peter Krempa <[email protected]>
> ---
> tests/testutils.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/tests/testutils.c b/tests/testutils.c
> index 35571fb2ad..9ce549370b 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -33,6 +33,7 @@
> #include "virbuffer.h"
> #include "virlog.h"
> #include "virstring.h"
> +#include "vircommand.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
> @@ -840,6 +841,7 @@ int virTestMain(int argc,
> int (*func)(void),
> ...)
> {
> + g_autoptr(virBitmap) keepfds = virBitmapNew(0);
> const char *lib;
> va_list ap;
> int ret;
> @@ -859,6 +861,14 @@ int virTestMain(int argc,
> preloads[npreloads] = NULL;
> }
>
> + /* Environment may leak FDs into the test which we don't want to use and
> + * it could break certain test cases which use 'dup2' to put FDs to
> correct
> + * numbers to have stable output. */
> + virBitmapSetBitExpand(keepfds, STDIN_FILENO);
> + virBitmapSetBitExpand(keepfds, STDOUT_FILENO);
> + virBitmapSetBitExpand(keepfds, STDERR_FILENO);
> + virCommandFDMassClose(keepfds);
I'm not convinced this is safe to do if it is not followed by an
immediate exec.
Libraries we link to are liable to open file descriptors, either in
constructors, or in Libc code that runs before main(). By doing a mass
close we are at risk of library code using a bad FD.
While the tests may pass CI with this today, it feels like a foot-gun
waiting to go off.
> +
> va_start(ap, func);
> while ((lib = va_arg(ap, const char *))) {
> g_autofree char *abs_lib_path = g_strdup_printf("%s/%s",
> abs_builddir, lib);
> --
> 2.54.0
>
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
