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(a)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(a)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