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

[Michal Prívozník]

On 4/14/22 18:03, Peter Krempa wrote:
> 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 at redhat.com>
>> ---
> 
> [...]


>> 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.
> 

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