The point of qemu_processpriv.h file is to allow a small subset of functions to be called from test suite but not elsewhere. This is implemented by requiring everybody that includes the file to define a macro. If not done so, an error is printed at compile time. However, this error message contains a typo because it mentions qemu_process_priv.h while the file is called qemu_processpriv.h. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/qemu/qemu_processpriv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_processpriv.h b/src/qemu/qemu_processpriv.h index 2bc60ce103..9cb176e97c 100644 --- a/src/qemu/qemu_processpriv.h +++ b/src/qemu/qemu_processpriv.h @@ -20,7 +20,7 @@ */ #ifndef LIBVIRT_QEMU_PROCESSPRIV_H_ALLOW -# error "qemu_process_priv.h may only be included by qemu_process.c or test suites" +# error "qemu_processpriv.h may only be included by qemu_process.c or test suites" #endif /* LIBVIRT_QEMU_PROCESSPRIV_H_ALLOW */ #pragma once -- 2.35.1

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

On 4/14/22 18:03, Peter Krempa wrote: Alright, so you want driver->config->stdioLogD = true to stay; fair enough. However, clearly testQemuPrepareHostBackendChardevOne() (and also it's qemu_process.c origin: qemuProcessPrepareHostBackendChardevOne()) have affect on generated cmd line. So PrepareHost infix is a bit misleading. But what I'm trying to fix is: _build/tests $ valgrind --trace-children=yes ./qemuhotplugtest ==13118== Memcheck, a memory error detector ==13118== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==13118== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info ==13118== Command: ./qemuhotplugtest ==13118== valgrind: symbol lookup error: /.../libvirt.git/_build/tests/libqemuhotplugmock.so: undefined symbol: testQemuPrepareHostBackendChardevOne So I guess I'll need to try something else. Michal