[PATCH 2/2] tests: Turn testQemuPrepareHostBackendChardevOne() into proper mock

Michal Privoznik mprivozn at redhat.com
Thu Apr 14 14:44:25 UTC 2022


Commit v8.0.0-409-gad81aa8ad0 added another function into
qemuhotplugmock.c. However, it did so in a bit clumsy way: the
function calls testQemuPrepareHostBackendChardevOne() which is
not exported and is a part of libtest_utils_qemu.a static
library. Fortunately, qemuhotplugtest links with it and thus we
did not see any trouble at runtime as the symbol was resolved
into something in the binary. The problem arose when the test is
ran under valgrind. There the symbol is not resolved (although I
don't fully understand why).

Nevertheless, the testQemuPrepareHostBackendChardevOne() function
can be turned into a proper mock of
qemuProcessPrepareHostBackendChardev() (since they former was
heavily inspired by the latter).

Moreover, if the QEMU stub driver config is changed so that
stdioLogD is false, then more code can be cleaned up:

1) qemuProcessPrepareHostBackendChardevHotplug() mock from
   qemuhotplugmock.c can be dropped (effectively reverting the
   original commit),

2) testCompareXMLToArgvCreateArgs() can call full blown
   qemuProcessPrepareHostBackendChardev() instead of open coding
   it.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/qemu/qemu_process.c     |  13 +----
 src/qemu/qemu_processpriv.h |  18 ++++++
 tests/qemuhotplugmock.c     |  11 ----
 tests/qemuhotplugtest.c     |   1 +
 tests/qemumonitorjsontest.c |   7 ++-
 tests/qemuxml2argvmock.c    |  94 +++++++++++++++++++++++++++++++
 tests/qemuxml2argvtest.c    |  10 ++--
 tests/testutilsqemu.c       | 107 +-----------------------------------
 tests/testutilsqemu.h       |   4 --
 9 files changed, 126 insertions(+), 139 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e1ac4c7032..c7949e8a34 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6773,16 +6773,7 @@ qemuProcessPrepareHostBackendChardevFileHelper(const char *path,
 }
 
 
-struct qemuProcessPrepareHostBackendChardevData {
-    qemuDomainObjPrivate *priv;
-    virLogManager *logManager;
-    virQEMUDriverConfig *cfg;
-    virDomainDef *def;
-    const char *fdprefix;
-};
-
-
-static int
+int
 qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
                                         virDomainChrSourceDef *chardev,
                                         void *opaque)
@@ -6900,7 +6891,7 @@ qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
 /* prepare the chardev backends for various devices:
  * serial/parallel/channel chardevs, vhost-user disks, vhost-user network
  * interfaces, smartcards, shared memory, and redirdevs */
-static int
+int
 qemuProcessPrepareHostBackendChardev(virDomainObj *vm)
 {
     qemuDomainObjPrivate *priv = vm->privateData;
diff --git a/src/qemu/qemu_processpriv.h b/src/qemu/qemu_processpriv.h
index 9cb176e97c..34ab829876 100644
--- a/src/qemu/qemu_processpriv.h
+++ b/src/qemu/qemu_processpriv.h
@@ -27,6 +27,7 @@
 
 #include "domain_conf.h"
 #include "qemu_monitor.h"
+#include "qemu_domain.h"
 
 /*
  * This header file should never be used outside unit tests.
@@ -38,3 +39,20 @@ void qemuProcessHandleDeviceDeleted(qemuMonitor *mon,
                                     void *opaque);
 
 int qemuProcessQMPInitMonitor(qemuMonitor *mon);
+
+struct qemuProcessPrepareHostBackendChardevData {
+    qemuDomainObjPrivate *priv;
+    virLogManager *logManager;
+    virQEMUDriverConfig *cfg;
+    virDomainDef *def;
+    const char *fdprefix;
+};
+
+
+int
+qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
+                                        virDomainChrSourceDef *chardev,
+                                        void *opaque) G_GNUC_NO_INLINE;
+
+int
+qemuProcessPrepareHostBackendChardev(virDomainObj *vm);
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index d618ff9d06..a8dbc45b60 100644
--- a/tests/qemuhotplugmock.c
+++ b/tests/qemuhotplugmock.c
@@ -21,7 +21,6 @@
 #include "qemu/qemu_hotplug.h"
 #include "qemu/qemu_interface.h"
 #include "qemu/qemu_process.h"
-#include "testutilsqemu.h"
 #include "conf/domain_conf.h"
 #include "virdevmapper.h"
 #include "virutil.h"
@@ -102,16 +101,6 @@ qemuInterfaceVDPAConnect(virDomainNetDef *net G_GNUC_UNUSED)
 }
 
 
-int
-qemuProcessPrepareHostBackendChardevHotplug(virDomainObj *vm,
-                                            virDomainDeviceDef *dev)
-{
-    return qemuDomainDeviceBackendChardevForeachOne(dev,
-                                                    testQemuPrepareHostBackendChardevOne,
-                                                    vm);
-}
-
-
 /* we don't really want to send fake FDs across the monitor */
 int
 qemuMonitorIOWriteWithFD(qemuMonitor *mon,
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 21302e0fce..3815011c1b 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -913,4 +913,5 @@ VIR_TEST_MAIN_PRELOAD(mymain,
                       VIR_TEST_MOCK("virpci"),
                       VIR_TEST_MOCK("domaincaps"),
                       VIR_TEST_MOCK("virprocess"),
+                      VIR_TEST_MOCK("qemuxml2argv"),
                       VIR_TEST_MOCK("qemuhotplug"));
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 278d7ba765..0d5624760d 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -36,6 +36,8 @@
 #include "qemu/qemu_migration_params.h"
 #define LIBVIRT_QEMU_MIGRATION_PARAMSPRIV_H_ALLOW
 #include "qemu/qemu_migration_paramspriv.h"
+#define LIBVIRT_QEMU_PROCESSPRIV_H_ALLOW
+#include "qemu/qemu_processpriv.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -742,7 +744,7 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOption *xmlopt,
                                                              "'logappend':false}}}");
 
         chrdev.source = chr;
-        ignore_value(testQemuPrepareHostBackendChardevOne(&dev, chr, NULL));
+        ignore_value(qemuProcessPrepareHostBackendChardevOne(&dev, chr, NULL));
         qemuFDPassTransferMonitorFake(charpriv->sourcefd);
         qemuFDPassTransferMonitorFake(charpriv->logfd);
         CHECK("file", false,
@@ -3284,4 +3286,5 @@ mymain(void)
     return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
-VIR_TEST_MAIN(mymain)
+VIR_TEST_MAIN_PRELOAD(mymain,
+                      VIR_TEST_MOCK("qemuxml2argv"));
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 08f176667d..c0e73f44ac 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -19,6 +19,7 @@
 #include <config.h>
 
 #define LIBVIRT_VIRIDENTITYPRIV_H_ALLOW
+#define LIBVIRT_QEMU_PROCESSPRIV_H_ALLOW
 
 #include "internal.h"
 #include "viralloc.h"
@@ -41,6 +42,7 @@
 #include "virutil.h"
 #include "qemu/qemu_interface.h"
 #include "qemu/qemu_command.h"
+#include "qemu/qemu_processpriv.h"
 #include <time.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -292,6 +294,98 @@ qemuInterfaceVDPAConnect(virDomainNetDef *net G_GNUC_UNUSED)
     return 1732;
 }
 
+
+int
+qemuProcessPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
+                                        virDomainChrSourceDef *chardev,
+                                        void *opaque)
+{
+    struct qemuProcessPrepareHostBackendChardevData *data = opaque;
+    qemuDomainObjPrivate *priv = data ? data->priv : NULL;
+    qemuDomainChrSourcePrivate *charpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chardev);
+    int fakesourcefd = -1;
+    const char *devalias = NULL;
+    bool usefdset = true;
+
+    if (dev) {
+        virDomainDeviceInfo *info = virDomainDeviceGetInfo(dev);
+        devalias = info->alias;
+
+        /* vhost-user disk doesn't use FD passing */
+        if (dev->type == VIR_DOMAIN_DEVICE_DISK)
+            return 0;
+
+        if (dev->type == VIR_DOMAIN_DEVICE_NET) {
+            /* due to a historical bug in qemu we don't use FD passtrhough for
+             * vhost-sockets for network devices */
+            return 0;
+        }
+
+        /* TPMs FD passing setup is special and handled separately */
+        if (dev->type == VIR_DOMAIN_DEVICE_TPM)
+            return 0;
+    } else {
+        devalias = "monitor";
+    }
+
+    switch ((virDomainChrType) chardev->type) {
+    case VIR_DOMAIN_CHR_TYPE_NULL:
+    case VIR_DOMAIN_CHR_TYPE_VC:
+    case VIR_DOMAIN_CHR_TYPE_PTY:
+    case VIR_DOMAIN_CHR_TYPE_DEV:
+    case VIR_DOMAIN_CHR_TYPE_PIPE:
+    case VIR_DOMAIN_CHR_TYPE_STDIO:
+    case VIR_DOMAIN_CHR_TYPE_UDP:
+    case VIR_DOMAIN_CHR_TYPE_TCP:
+    case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+    case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+        break;
+
+    case VIR_DOMAIN_CHR_TYPE_FILE:
+        fakesourcefd = 1750;
+        break;
+
+    case VIR_DOMAIN_CHR_TYPE_UNIX:
+        if (chardev->data.nix.listen)
+            fakesourcefd = 1729;
+
+        usefdset = false;
+        break;
+
+    case VIR_DOMAIN_CHR_TYPE_NMDM:
+    case VIR_DOMAIN_CHR_TYPE_LAST:
+        break;
+    }
+
+    if (fakesourcefd != -1) {
+        if (fcntl(fakesourcefd, F_GETFD) != -1)
+            abort();
+
+        if (usefdset)
+            charpriv->sourcefd = qemuFDPassNew(devalias, priv);
+        else
+            charpriv->sourcefd = qemuFDPassNewDirect(devalias, priv);
+
+        if (qemuFDPassAddFD(charpriv->sourcefd, &fakesourcefd, "-source") < 0)
+            return -1;
+    }
+
+    if (chardev->logfile) {
+        int fd = 1751;
+
+        if (fcntl(fd, F_GETFD) != -1)
+            abort();
+
+        charpriv->logfd = qemuFDPassNew(devalias, priv);
+
+        if (qemuFDPassAddFD(charpriv->logfd, &fd, "-log") < 0)
+            return -1;
+    }
+
+    return 0;
+}
+
+
 char *
 virIdentityEnsureSystemToken(void)
 {
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ed41b7a7a2..e10bb61785 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -34,6 +34,9 @@
 # define LIBVIRT_QEMU_CAPSPRIV_H_ALLOW
 # include "qemu/qemu_capspriv.h"
 
+# define LIBVIRT_QEMU_PROCESSPRIV_H_ALLOW
+# include "qemu/qemu_processpriv.h"
+
 # include "testutilsqemu.h"
 
 # define VIR_FROM_THIS VIR_FROM_QEMU
@@ -393,12 +396,7 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv,
                                            VIR_QEMU_PROCESS_START_COLD) < 0)
         return NULL;
 
-    if (qemuDomainDeviceBackendChardevForeach(vm->def,
-                                              testQemuPrepareHostBackendChardevOne,
-                                              vm) < 0)
-        return NULL;
-
-    if (testQemuPrepareHostBackendChardevOne(NULL, priv->monConfig, vm) < 0)
+    if (qemuProcessPrepareHostBackendChardev(vm) < 0)
         return NULL;
 
     for (i = 0; i < vm->def->ndisks; i++) {
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 105b41cbeb..e65d0cf64e 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -578,6 +578,8 @@ int qemuTestDriverInit(virQEMUDriver *driver)
     if (!driver->config)
         goto error;
 
+    driver->config->stdioLogD = false;
+
     /* Do this early so that qemuTestDriverFree() doesn't see (unlink) the real
      * dirs. */
     VIR_FREE(driver->config->stateDir);
@@ -982,108 +984,3 @@ testQemuInfoClear(struct testQemuInfo *info)
     virObjectUnref(info->qemuCaps);
     g_clear_pointer(&info->args.fakeCaps, virObjectUnref);
 }
-
-
-/**
- * testQemuPrepareHostBackendChardevOne:
- * @dev: device definition object
- * @chardev: chardev source object
- * @opaque: Caller is expected to pass pointer to virDomainObj or NULL
- *
- * This helper sets up a chardev source backend for FD passing with fake
- * file descriptros. It's expected to be used as  callback for
- * 'qemuDomainDeviceBackendChardevForeach', thus the VM object is passed via
- * @opaque. Callers may pass NULL if the test scope is limited.
- */
-int
-testQemuPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
-                                     virDomainChrSourceDef *chardev,
-                                     void *opaque)
-{
-    virDomainObj *vm = opaque;
-    qemuDomainObjPrivate *priv = NULL;
-    qemuDomainChrSourcePrivate *charpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chardev);
-    int fakesourcefd = -1;
-    const char *devalias = NULL;
-    bool usefdset = true;
-
-    if (vm)
-        priv = vm->privateData;
-
-    if (dev) {
-        virDomainDeviceInfo *info = virDomainDeviceGetInfo(dev);
-        devalias = info->alias;
-
-        /* vhost-user disk doesn't use FD passing */
-        if (dev->type == VIR_DOMAIN_DEVICE_DISK)
-            return 0;
-
-        if (dev->type == VIR_DOMAIN_DEVICE_NET) {
-            /* due to a historical bug in qemu we don't use FD passtrhough for
-             * vhost-sockets for network devices */
-            return 0;
-        }
-
-        /* TPMs FD passing setup is special and handled separately */
-        if (dev->type == VIR_DOMAIN_DEVICE_TPM)
-            return 0;
-    } else {
-        devalias = "monitor";
-    }
-
-    switch ((virDomainChrType) chardev->type) {
-    case VIR_DOMAIN_CHR_TYPE_NULL:
-    case VIR_DOMAIN_CHR_TYPE_VC:
-    case VIR_DOMAIN_CHR_TYPE_PTY:
-    case VIR_DOMAIN_CHR_TYPE_DEV:
-    case VIR_DOMAIN_CHR_TYPE_PIPE:
-    case VIR_DOMAIN_CHR_TYPE_STDIO:
-    case VIR_DOMAIN_CHR_TYPE_UDP:
-    case VIR_DOMAIN_CHR_TYPE_TCP:
-    case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
-    case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
-        break;
-
-    case VIR_DOMAIN_CHR_TYPE_FILE:
-        fakesourcefd = 1750;
-        break;
-
-    case VIR_DOMAIN_CHR_TYPE_UNIX:
-        if (chardev->data.nix.listen)
-            fakesourcefd = 1729;
-
-        usefdset = false;
-        break;
-
-    case VIR_DOMAIN_CHR_TYPE_NMDM:
-    case VIR_DOMAIN_CHR_TYPE_LAST:
-        break;
-    }
-
-    if (fakesourcefd != -1) {
-        if (fcntl(fakesourcefd, F_GETFD) != -1)
-            abort();
-
-        if (usefdset)
-            charpriv->sourcefd = qemuFDPassNew(devalias, priv);
-        else
-            charpriv->sourcefd = qemuFDPassNewDirect(devalias, priv);
-
-        if (qemuFDPassAddFD(charpriv->sourcefd, &fakesourcefd, "-source") < 0)
-            return -1;
-    }
-
-    if (chardev->logfile) {
-        int fd = 1751;
-
-        if (fcntl(fd, F_GETFD) != -1)
-            abort();
-
-        charpriv->logfd = qemuFDPassNew(devalias, priv);
-
-        if (qemuFDPassAddFD(charpriv->logfd, &fd, "-log") < 0)
-            return -1;
-    }
-
-    return 0;
-}
diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h
index 187f9b7cd3..a28f640670 100644
--- a/tests/testutilsqemu.h
+++ b/tests/testutilsqemu.h
@@ -142,8 +142,4 @@ void testQemuInfoSetArgs(struct testQemuInfo *info,
                          ...);
 int testQemuInfoInitArgs(struct testQemuInfo *info);
 void testQemuInfoClear(struct testQemuInfo *info);
-
-int testQemuPrepareHostBackendChardevOne(virDomainDeviceDef *dev,
-                                         virDomainChrSourceDef *chardev,
-                                         void *opaque);
 #endif
-- 
2.35.1



More information about the libvir-list mailing list