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

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. Fixes: b66ca0220a ("qemu: domain: Call virDomainCapsDeviceDefValidate") Signed-off-by: Daniel Henrique Barboza <danielhb413@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; } -- 2.21.0

On 8/12/19 5:06 PM, 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:
Thanks for catching this, I had no idea. But I think the commit that caused the issue is in fact 5b9819eedc7 because that added code that leads to the error: virReportSystemErrorFull 1 $ bt #0 virReportSystemErrorFull at util/virerror.c:1350 #1 0x00007ffff7a786eb in virDirOpenInternal at util/virfile.c:2845 #2 0x00007ffff7a78757 in virDirOpenIfExists at util/virfile.c:2877 #3 0x00005555555f89bf in qemuFirmwareBuildFileList at qemu/qemu_firmware.c:918 #4 0x00005555555f8f1e in qemuFirmwareFetchConfigs at qemu/qemu_firmware.c:1012 #5 0x00005555555f9e8b in qemuFirmwareFetchParsedConfigs at qemu/qemu_firmware.c:1333 #6 0x00005555555fa355 in qemuFirmwareGetSupported at qemu/qemu_firmware.c:1430 #7 0x00005555555c6bcf in virQEMUCapsFillDomainOSCaps at qemu/qemu_capabilities.c:5179 #8 0x00005555555c7c26 in virQEMUCapsFillDomainCaps at qemu/qemu_capabilities.c:5573 #9 0x00005555555ffd5b in virQEMUDriverGetDomainCapabilities at qemu/qemu_conf.c:1432 #10 0x00005555555d8c88 in qemuDomainDeviceDefValidate at qemu/qemu_domain.c:6741 #11 0x00007ffff7b20be2 in virDomainDeviceDefValidate at conf/domain_conf.c:6547 #12 0x00007ffff7b20c51 in virDomainDefValidateDeviceIterator at conf/domain_conf.c:6564 #13 0x00007ffff7b1b6a6 in virDomainDeviceInfoIterateInternal at conf/domain_conf.c:4141 #14 0x00007ffff7b21da6 in virDomainDefValidate at conf/domain_conf.c:6951 #15 0x00007ffff7b54b46 in virDomainDefParseNode at conf/domain_conf.c:21513 #16 0x00007ffff7b54948 in virDomainDefParse at conf/domain_conf.c:21447 #17 0x00007ffff7b549ee in virDomainDefParseFile at conf/domain_conf.c:21473 #18 0x00005555555bcb69 in testCompareDomXML2XMLFiles at testutils.c:1303 #19 0x000055555558ad63 in testXML2XMLInactive at qemuxml2xmltest.c:45 #20 0x00005555555ba29e in virTestRun at testutils.c:174 #21 0x00005555555a4ae2 in mymain at qemuxml2xmltest.c:657 #22 0x00005555555bc362 in virTestMain at testutils.c:1096 #23 0x00005555555b7a4c in main at qemuxml2xmltest.c:1332 I'm changing the commit message accordingly, ACKing and pushing. Michal

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? Thanks, Cole
Fixes: b66ca0220a ("qemu: domain: Call virDomainCapsDeviceDefValidate") Signed-off-by: Daniel Henrique Barboza <danielhb413@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

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@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@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
participants (3)
-
Cole Robinson
-
Daniel Henrique Barboza
-
Michal Privoznik