[libvirt] [PATCH 1/1] qemuxml2xmltest.c: mock /etc/qemu/firmware path

[Daniel Henrique Barboza]

Hey Cole,

On 8/19/19 6:49 PM, Cole Robinson wrote:
> On 8/12/19 11:06 AM, Daniel Henrique Barboza wrote:
>> Since commit 'b66ca0220a' the 'tpm-emulator' tests in qemuxml2xmltest
>> started to fail unless 'make check' is either executed as root
>> or with an user with access to /etc/qemu/firmware file. This is the
>> error that happens in a regular 'make check' run with an unprivileged
>> user:
>>
>> ------
>>
>>   546) QEMU XML-2-XML-inactive tpm-emulator                              ...
>> 1315 In '/home/danielhb/kvm-project/libvirt/tests/qemuxml2xmloutdata/tpm-emulator.xml':
>> 1316 Offset 909
>> 1317 Expect [1.2]
>> 1318 Actual [default]
>> 1319                                                                       ... Expected result      code=0 but received code=4libvirt:  error : cannot open directory '/home/danielhb/kvm-pr     oject/libvirt/usr/etc/qemu/firmware': Permission denied
>> 1320 FAILED
>> 1321 547) QEMU XML-2-XML-active tpm-emulator                                ...
>> 1322 In '/home/danielhb/kvm-project/libvirt/tests/qemuxml2xmloutdata/tpm-emulator.xml':
>> 1323 Offset 909
>> 1324 Expect [1.2]
>> 1325 Actual [default]
>> 1326                                                                       ... Expected result      code=0 but received code=4libvirt:  error : cannot open directory '/home/danielhb/kvm-pr     oject/libvirt/usr/etc/qemu/firmware': Permission denied
>> 1327 FAILED
>>
>> ------
>>
>> The reason is that commit b66ca0220a changed the domain validation to
>> fetch the capabilities in qemuDomainDeviceDefValidate time, directly
>> impacting existing tpm-emulator tests in qemuxml2xmltest.c that
>> that since then will attempt to read the /etc/qemu/firmware file
>> that wasn't being mocked.
>>
>> This patch fixes it by adding the mock paths for /etc/qemu/firmware
>> in qemuxml2xmltest.c as well, using the virFileWrapper API like it
>> is done in commit 5b9819eedc71.
>>
> Sorry about that. But I can't reproduce and I can't really tell what the
> issue is, I tried reproducing the permission issue but nothing failed
> for me in qemuxml2xml. The error report above seems like the test suite
> $srcdir/usr/etc/qemu/firmware which looks strange anyways, do you have
> --prefix set to $srcdir? Are you still seeing this with libvirt.git?


  The setup I'm using to compile Libvirt is:

$ ./autogen.sh --prefix=$PWD/usr


in qemu.conf:

security_driver=none

user/group = root


It is not reproducible with current master anymore -  Michal already pushed
this fix upstream. My explanation in the commit message wasn't precise
though, and Michal ended up amending it. This is the reviewed version
after Michal pushed it:

commit 92832577d1ef29d26d42bcbf978be0e845a31365
Author: Daniel Henrique Barboza <danielhb413 at gmail.com>
Date:   Mon Aug 12 12:06:22 2019 -0300

     qemuxml2xmltest: Redirect access to FW descriptor dirs

     If /etc/qemu/firmware directory exists, but is not readable then
     qemuxml2xmltest fails. This is because once domain XML is parsed
     it is validated. For that domain capabilities are needed.
     However, when constructing domain capabilities, FW descriptors
     are loaded and this is the point where the test fails, because it
     fails to open one of the directories.




Thanks,


DHB


>
> Thanks,
> Cole
>
>> Fixes: b66ca0220a ("qemu: domain: Call virDomainCapsDeviceDefValidate")
>> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
>> ---
>>   tests/Makefile.am       |  3 ++-
>>   tests/qemuxml2xmltest.c | 13 +++++++++++++
>>   2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index 1c92e3ca6f..31a22ebefd 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -573,7 +573,8 @@ qemuxml2argvmock_la_LIBADD = $(MOCKLIBS_LIBS)
>>   
>>   qemuxml2xmltest_SOURCES = \
>>   	qemuxml2xmltest.c testutilsqemu.c testutilsqemu.h \
>> -	testutils.c testutils.h
>> +	testutils.c testutils.h \
>> +	virfilewrapper.c virfilewrapper.h
>>   qemuxml2xmltest_LDADD = $(qemu_LDADDS)
>>   
>>   qemumonitorjsontest_SOURCES = \
>> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
>> index 525eb9a740..51be6e0263 100644
>> --- a/tests/qemuxml2xmltest.c
>> +++ b/tests/qemuxml2xmltest.c
>> @@ -14,6 +14,9 @@
>>   # include "qemu/qemu_domain.h"
>>   # include "testutilsqemu.h"
>>   # include "virstring.h"
>> +# include "virfilewrapper.h"
>> +# include "testutils.h"
>> +# include "configmake.h"
>>   
>>   # define VIR_FROM_THIS VIR_FROM_NONE
>>   
>> @@ -167,6 +170,15 @@ mymain(void)
>>   
>>       setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1);
>>   
>> +    /* Required for tpm-emulator tests
>> +     */
>> +    virFileWrapperAddPrefix(SYSCONFDIR "/qemu/firmware",
>> +                            abs_srcdir "/qemufirmwaredata/etc/qemu/firmware");
>> +    virFileWrapperAddPrefix(PREFIX "/share/qemu/firmware",
>> +                            abs_srcdir "/qemufirmwaredata/usr/share/qemu/firmware");
>> +    virFileWrapperAddPrefix("/home/user/.config/qemu/firmware",
>> +                            abs_srcdir "/qemufirmwaredata/home/user/.config/qemu/firmware");
>> +
>>       if (qemuTestDriverInit(&driver) < 0)
>>           return EXIT_FAILURE;
>>   
>> @@ -1325,6 +1337,7 @@ mymain(void)
>>       virHashFree(capslatest);
>>       qemuTestDriverFree(&driver);
>>       VIR_FREE(fakerootdir);
>> +    virFileWrapperClearPrefixes();
>>   
>>       return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>>   }
>>
>
> - Cole