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

On Thu, Apr 14, 2022 at 16:44:25 +0200, Michal Privoznik wrote: > 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: But we actually want to primarily test the case when virtlogd is actually used thus 'stdioLogD' shall be true in the tests. > > 1) qemuProcessPrepareHostBackendChardevHotplug() mock from > qemuhotplugmock.c can be dropped (effectively reverting the > original commit), The commit doing this was specifically part of a series where we didn't honour the use of virtlogd when hotplugging sockets thus bypassing the protections which virtlogd is supposed to provide. > > 2) testCompareXMLToArgvCreateArgs() can call full blown > qemuProcessPrepareHostBackendChardev() instead of open coding > it. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- [...]

> 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; > + I don't think this change is wanted and also it's not properly justified. We want to be testing any potential side effects of having logd enabled as it's the default configuration even if it at this point doesn't have impact on what we test.