qemuxml2argvmock: Fix behaviour of 'virCommandPass' mock

The 'virCommandPass' mock in 'qemuxml2argvmock.so' skipped passing most
FDs to the real implementation of 'virCommandPass', except for few fake
FDs that were hardcoded.

This meant that if a test case had an valid FD that it used for testing
that FD would be leaked. At the same time fake fds 1730, 1731, 1732
would be actually passed to the virCommand itself although the FD was
invalid.

Since neither of the above makes sense fix the implementation the
following way:

 - refuse to pass any STDIO fds
     They are real and they would break test program output. Some tests
     do try to use them errorneously; they will be addressed later.

 - pass real FDs to virCommand
    Real FDs can be properly handled by virCommand. Especially they will
    be closed once the virCommand object is disposed of.

 - don't pass fake FDs
    They create extra noise e.g. in valgrind. Skip those as it makes no
    sense to handle those.

This patch addresses most failures that valgrind reports with
--track-fds=all.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index b4ebf1a172488e83aeaa49ec19cb5a6c1074792d..d24319638b96f200240a0fbf73f44a79b29beaf5 100644 (file)
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -180,24 +180,32 @@ virHostGetDRMRenderNode(void)
 
 static void (*real_virCommandPassFD)(virCommand *cmd, int fd, unsigned int flags);
 
-static const int testCommandPassSafeFDs[] = { 1730, 1731, 1732 };
 
 void
 virCommandPassFD(virCommand *cmd,
                  int fd,
                  unsigned int flags)
 {
-    size_t i;
-
-    for (i = 0; i < G_N_ELEMENTS(testCommandPassSafeFDs); i++) {
-        if (testCommandPassSafeFDs[i] == fd) {
-            if (!real_virCommandPassFD)
-                VIR_MOCK_REAL_INIT(virCommandPassFD);
+    /* Test cases run in the context of the test program, so attempting to use
+     * the STDIO fds with virCommand could break/close them and thus break
+     * output of the test itself. */
+    if (fd == STDIN_FILENO ||
+        fd == STDOUT_FILENO ||
+        fd == STDERR_FILENO) {
+        return;
+    }
 
-            real_virCommandPassFD(cmd, fd, flags);
-            return;
-        }
+    /* Some test scenarios pass invalid FDs to virCommand. We want to skip
+     * operations on those since they cause errors in e.g. valgrind.
+     */
+    if (fcntl(fd, F_GETFD) == -1) {
+        return;
     }
+
+    if (!real_virCommandPassFD)
+        VIR_MOCK_REAL_INIT(virCommandPassFD);
+
+    real_virCommandPassFD(cmd, fd, flags);
 }
 
 int