[libvirt PATCH 0/2] tests: Fix qemuxml2argvtest failure on macOS 12
As reported in https://listman.redhat.com/archives/libvir-list/2022-May/230762.html I'm still unclear on why this worked with all versions of macOS until 12, but the fix feels reasonable enough and I can't really justify digging much further. Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/615807379 macOS 12 job: https://gitlab.com/abologna/libvirt/-/jobs/2895958597 Andrea Bolognani (2): util: Preserve macOS dyld environment by default tests: Reset macOS dyld environment src/util/vircommand.c| 2 ++ tests/qemuxml2argvtest.c | 2 ++ 2 files changed, 4 insertions(+) -- 2.37.2
[libvirt PATCH 1/2] util: Preserve macOS dyld environment by default
The DYLD_* environment variables on macOS have the same purpose as the LD_* variables have on Linux. Since we're preserving the latter by default, it makes sense to do the same for the former as well. Signed-off-by: Andrea Bolognani --- src/util/vircommand.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index d78c666f28..4e003266bf 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1417,6 +1417,8 @@ virCommandAddEnvPassCommon(virCommand *cmd) virCommandAddEnvPass(cmd, "LD_PRELOAD"); virCommandAddEnvPass(cmd, "LD_LIBRARY_PATH"); +virCommandAddEnvPass(cmd, "DYLD_INSERT_LIBRARIES"); +virCommandAddEnvPass(cmd, "DYLD_FORCE_FLAT_NAMESPACE"); virCommandAddEnvPass(cmd, "PATH"); virCommandAddEnvPass(cmd, "HOME"); virCommandAddEnvPass(cmd, "USER"); -- 2.37.2
[libvirt PATCH 2/2] tests: Reset macOS dyld environment
This is needed to ensure the environment variables that we need
for the test program itself, specifically to load mock libraries,
does not interfere with any command that gets invoked by it,
either directly or indirectly. We already perform the same
cleanup step for LD_* variables.
This makes the test failures
error : virCommandWait:2752 : internal error: Child process
(/usr/libexec/qemu/vhost-user/test-vhost-user-gpu --print-capabilities)
unexpected fatal signal 6: dyld[8896]: symbol not found in flat
namespace '_virQEMUCapsGet'
error : qemuVhostUserFillDomainGPU:394 : operation failed: Unable to
find a satisfying vhost-user-gpu
that were showing up on macOS 12 go away.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvtest.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8933e373f7..4ecccdc272 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1053,6 +1053,8 @@ mymain(void)
g_unsetenv("TMPDIR");
g_unsetenv("LD_PRELOAD");
g_unsetenv("LD_LIBRARY_PATH");
+g_unsetenv("DYLD_INSERT_LIBRARIES");
+g_unsetenv("DYLD_FORCE_FLAT_NAMESPACE");
g_unsetenv("QEMU_AUDIO_DRV");
g_unsetenv("SDL_AUDIODRIVER");
--
2.37.2
Re: [libvirt PATCH 0/3] tests: Fix qemucapabilitiestest on macOS
On Wed, Aug 10, 2022 at 10:54:16AM -0500, Andrea Bolognani wrote: > On Wed, Aug 10, 2022 at 05:16:57PM +0200, Christophe de Dinechin wrote: > > This works. We now have a clean test suite on macOS 12: > > > > Ok: 252 > > Expected Fail: 0 > > Fail: 0 > > Unexpected Pass:0 > > Skipped:19 > > Timeout:0 > > I'll try switching the Cirrus CI configuration to macOS 12 again, but > I'm afraid it's still going to hit the qemuxml2argvtest issue that > doesn't show up on your machine. We'll see :) Yeah, that still fails. https://gitlab.com/abologna/libvirt/-/jobs/2843833417 This might be too much to ask but, you could possibly create a macOS VM on your machine and see whether the issue reproduces there? I'm starting to think that virtualization is somehow the issue, even though that doesn't really make any sense to me. And a fresh install would rule out that whatever local configuration changes you're made to your main OS are what's causing the test to pass rather than fail. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/3] scripts: Add $DESTDIR support to meson-install-web.py
On Tue, Aug 09, 2022 at 05:26:28PM +0200, Pavel Hrdina wrote:
> On Tue, Jul 19, 2022 at 04:17:44PM +0200, Andrea Bolognani wrote:
> > +destdir = os.getenv('DESTDIR')
> > +if destdir:
> > +destdir = Path(destdir)
> > +if not destdir.is_absolute():
> > +print('$DESTDIR must be an absolute path')
> > +sys.exit(1)
>
> I don't see any reason for this check. Yes, DESTDIR is mostly used with
> absolute path but the other two scripts where we use DESTDIR don't have
> this check and meson itself doesn't complaint if the path is
> not absolute as well.
>
> That brings me to the other point that there is no need to use pathlib
> at all. We can just do the same as scripts/meson-install-dirs.py or
> scripts/meson-install-symlink.py:
>
> destdir = os.environ.get('DESTDIR', os.sep)
>
> for desc in sys.argv[1:]:
> inst = desc.split(':')
> dst = os.path.join(destdir, inst[1].strip(os.sep))
> os.makedirs(dst, exist_ok=True)
> shutil.copy(src, dst)
You're absolutely right. Somehow I had not realized that we already
had custom DESTDIR handling in other scripts that I could rip off ;)
v2 here:
https://listman.redhat.com/archives/libvir-list/2022-August/233655.html
--
Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH v2 0/2] ci: Fix paths shown in the website
Compare https://abologna.gitlab.io/-/libvirt/-/jobs/2840280594/artifacts/website/manpages/virtqemud.html#files with https://libvirt.org/manpages/virtqemud.html#files Changes from [v1]: * don't use pathlib; * don't check whether DESTDIR is absolute. [v1] https://listman.redhat.com/archives/libvir-list/2022-July/232929.html Andrea Bolognani (2): scripts: Add $DESTDIR support to meson-install-web.py ci: Fix paths shown in the website .gitlab-ci.yml | 6 +++--- scripts/meson-install-web.py | 8 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) -- 2.37.1
[libvirt PATCH v2 1/2] scripts: Add $DESTDIR support to meson-install-web.py
meson already supports $DESTDIR natively, but in this case
we're using a custom script and so we have to do some extra
work ourselves.
Signed-off-by: Andrea Bolognani
---
scripts/meson-install-web.py | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/scripts/meson-install-web.py b/scripts/meson-install-web.py
index a03f8523cd..086d465ae4 100755
--- a/scripts/meson-install-web.py
+++ b/scripts/meson-install-web.py
@@ -4,7 +4,11 @@ import os
import shutil
import sys
+destdir = os.environ.get('DESTDIR', os.sep)
+
for desc in sys.argv[1:]:
inst = desc.split(':')
-os.makedirs(inst[1], exist_ok=True)
-shutil.copy(inst[0], inst[1])
+src = inst[0]
+dst = os.path.join(destdir, inst[1].strip(os.sep))
+os.makedirs(dst, exist_ok=True)
+shutil.copy(src, dst)
--
2.37.1
[libvirt PATCH v2 2/2] ci: Fix paths shown in the website
Right now we're setting the prefix to a custom path, which results in paths like /builds/libvirt/libvirt/vroot/etc/libvirt/virtqemud.conf ending up in the generated HTML. In order to avoid that, set the prefix and other installation paths to reasonable default values by passing -Dsystem=true and then take advantage of $DESTDIR support to still be able to write the HTML files without requiring root privileges. Reported-by: Martin Kletzander Signed-off-by: Andrea Bolognani Reviewed-by: Pavel Hrdina --- .gitlab-ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 39c5f8fb6d..f6ed14bf65 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -67,9 +67,9 @@ website: before_script: - *script_variables script: -- meson setup build --werror --prefix=$(pwd)/vroot || (cat build/meson-logs/meson-log.txt && exit 1) -- ninja -C build install-web -- mv vroot/share/doc/libvirt/html/ website +- meson setup build --werror -Dsystem=true || (cat build/meson-logs/meson-log.txt && exit 1) +- DESTDIR=$(pwd)/install ninja -C build install-web +- mv install/usr/share/doc/libvirt/html/ website artifacts: expose_as: 'Website' name: 'website' -- 2.37.1
Re: [libvirt PATCH 0/3] tests: Fix qemucapabilitiestest on macOS
On Wed, Aug 10, 2022 at 05:16:57PM +0200, Christophe de Dinechin wrote: > On 2022-08-10 at 16:29 +02, Andrea Bolognani wrote... > > We need to mock the function that probes for HVF support. > > > > Andrea Bolognani (3): > > tests: Use domaincapsmock in qemucapabilitiestest > > qemu: Make virQEMUCapsProbeHVF() non-static > > tests: Mock virQEMUCapsProbeHVF() > > > > src/qemu/qemu_capabilities.c | 4 ++-- > > src/qemu/qemu_capabilities.h | 2 ++ > > tests/domaincapsmock.c | 6 ++ > > tests/qemucapabilitiestest.c | 3 ++- > > 4 files changed, 12 insertions(+), 3 deletions(-) > > This works. We now have a clean test suite on macOS 12: > > Ok: 252 > Expected Fail: 0 > Fail: 0 > Unexpected Pass:0 > Skipped:19 > Timeout:0 > > For the series: > > Reviewed-by: Christophe de Dinechin > Tested-by: Christophe de Dinechin Thanks to both you and Jano! Pushed now. I'll try switching the Cirrus CI configuration to macOS 12 again, but I'm afraid it's still going to hit the qemuxml2argvtest issue that doesn't show up on your machine. We'll see :) -- Andrea Bolognani / Red Hat / Virtualization
Re: Test failures on macOS 12
On Mon, Aug 08, 2022 at 08:54:14PM +0200, Christophe de Dinechin wrote:
> On Fri, May 06, 2022 at 03:00:14AM -0700, Andrea Bolognani wrote:
> > The other issue is in qemuxml2argvtest:
> >
> > error : virCommandWait:2752 : internal error: Child process
> >(/usr/libexec/qemu/vhost-user/test-vhost-user-gpu --print-capabilities)
> >unexpected fatal signal 6: dyld[8896]: symbol not found in flat
> >namespace '_virQEMUCapsGet'
> > error : qemuVhostUserFillDomainGPU:394 : operation failed: Unable to
> >find a satisfying vhost-user-gpu
>
> I don’t see this one.
Dang :(
> What I see instead is failures on qemucapabilitiestest.
> The tests seems to depend on a particular install path for the
> qemu-system- binaries:
>
> binary = g_strdup_printf("/usr/bin/qemu-system-%s",
> data->archName);
>
> On macOS, there is something called “system integrity protection”
> which makes it a bit more difficult to add stuff to /usr/bin.
>
> However, I don’t really think that’s the problem.
We definitely shouldn't be looking at the contents of the build
machine's actual /usr/bin directory in our test suite! That'd be a
bug in our mocking machinery.
> I ran some tests with:
>
> VIR_TEST_DEBUG=1
> VIR_TEST_RANGE=1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31,33,35,37,39,41,43,45,47,49,51,53,55,57,59,61,63,65,67,69
> /Users/ddd/Work/libvirt/build/tests/qemucapabilitiestest
>
> The error I get seem to point to ‘hvf’ been unexected in the output:
>
> 15) 6.2.0 (x86_64)...
> In '/Users/ddd/Work/libvirt/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml':
> Offset 7557
> Expect [c]
> Actual [hvf'/>
>
> I do not really understand what the test is supposed to do. It seems
> to be comparing with reference files, but where does it get the
> qemu capabilities to compare with from?
The test reads a .replies file from tests/qemucapabilitiesdata/,
parses it as if a QEMU binary had replied that way to our poking it,
figure out what the corresponding emulator capabilities are, format
them to XML and compare the result to the matching .xml file.
> How should I proceed?
I've prepared patches that should address the issue you're seeing:
https://listman.redhat.com/archives/libvir-list/2022-August/233645.html
It'd be great if you could confirm that they work as intended.
--
Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 0/3] tests: Fix qemucapabilitiestest on macOS
We need to mock the function that probes for HVF support. Andrea Bolognani (3): tests: Use domaincapsmock in qemucapabilitiestest qemu: Make virQEMUCapsProbeHVF() non-static tests: Mock virQEMUCapsProbeHVF() src/qemu/qemu_capabilities.c | 4 ++-- src/qemu/qemu_capabilities.h | 2 ++ tests/domaincapsmock.c | 6 ++ tests/qemucapabilitiestest.c | 3 ++- 4 files changed, 12 insertions(+), 3 deletions(-) -- 2.37.1
[libvirt PATCH 3/3] tests: Mock virQEMUCapsProbeHVF()
Successfully returning without doing anything is what the
function already does on non-Apple platforms.
When building on macOS, however, the check for HVF availability
will be performed. When running on bare metal, that will result
in the QEMU_CAPS_HVF flag being added to the virQEMUCaps
instance, and a bunch of error messages along the lines of
In 'tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml':
Offset 7557
Expect [c]
Actual [hvf'/>
Signed-off-by: Andrea Bolognani
---
tests/domaincapsmock.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/tests/domaincapsmock.c b/tests/domaincapsmock.c
index d382d06e27..4d53e48c48 100644
--- a/tests/domaincapsmock.c
+++ b/tests/domaincapsmock.c
@@ -51,6 +51,12 @@ virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps)
return real_virQEMUCapsGetKVMSupportsSecureGuest(qemuCaps);
}
+
+int
+virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps G_GNUC_UNUSED)
+{
+return 0;
+}
#endif
int
--
2.37.1
[libvirt PATCH 2/3] qemu: Make virQEMUCapsProbeHVF() non-static
We need to do this so that we can mock it in the test suite.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_capabilities.c | 4 ++--
src/qemu/qemu_capabilities.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 79ef8b88ef..80a8002f0c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3259,7 +3259,7 @@ virQEMUCapsProbeQMPKVMState(virQEMUCaps *qemuCaps,
}
#ifdef __APPLE__
-static int
+int
virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps)
{
int hv_support = 0;
@@ -3281,7 +3281,7 @@ virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps)
return 0;
}
#else
-static int
+int
virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps G_GNUC_UNUSED)
{
return 0;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 20b1034ca5..b172cfb7f1 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -848,5 +848,7 @@ virQEMUCapsGetSEVCapabilities(virQEMUCaps *qemuCaps);
bool
virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps) G_NO_INLINE;
+int virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps) G_NO_INLINE;
+
virArch virQEMUCapsArchFromString(const char *arch);
const char *virQEMUCapsArchToString(virArch arch);
--
2.37.1
[libvirt PATCH 1/3] tests: Use domaincapsmock in qemucapabilitiestest
This doesn't change anything at the moment, but is necessary
for the upcoming fix.
Signed-off-by: Andrea Bolognani
---
tests/qemucapabilitiestest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index 79dd358ef4..0066ba15fa 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -230,4 +230,5 @@ mymain(void)
return (data.ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
-VIR_TEST_MAIN(mymain)
+VIR_TEST_MAIN_PRELOAD(mymain,
+ VIR_TEST_MOCK("domaincaps"))
--
2.37.1
[PATCH] qemu: Fix indentation
Signed-off-by: Andrea Bolognani
---
Pushed as trivial.
src/qemu/qemu_capabilities.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f8e70470f6..79ef8b88ef 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3284,7 +3284,7 @@ virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps)
static int
virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps G_GNUC_UNUSED)
{
- return 0;
+return 0;
}
#endif
--
2.37.1
Re: Test failures on macOS 12
On Tue, Aug 09, 2022 at 11:34:20AM +0200, Christophe de Dinechin wrote: > On 9 Aug 2022, at 11:28, Andrea Bolognani wrote: > > Yeah, this seems to help and the change makes sense to me. > > > > I wonder why we didn't run into this much earlier though? As I > > mentioned, the test runs successfully as-is on macOS 11. Plus, many > > other tests rely on library injection and yet work okay even without > > this change. > > > > Anyway, I'm happy to add my > > > > Reviewed-by: Andrea Bolognani > > > > to this patch and push it. The authorship information looks a bit > > funky though, with the two S-o-bs... > > I did not know which one you’d prefer (in case there is a policy). > If I get to choose, assign that to Red Hat (and change the author > accordingly). > > (and I’ll change my libvirt gitconfig accordingly in the future) Done. I'll push once CI has passed. It would be great if you could use git-publish for future code submissions: that way patches can be applied locally more conveniently by the reviewer. I was able to make it work regardless, it just took a bit more effort :) -- Andrea Bolognani / Red Hat / Virtualization
Re: Test failures on macOS 12
On Mon, Aug 08, 2022 at 08:19:28PM +0200, Christophe de Dinechin wrote: > Hi Andrea, > > I saw your call to Sergio and Marc-André on IRC, and I thought I would > spend a few minutes inviestigating. Thanks, I appreciate that! > >> I'm trying to enable CI coverage for macOS 12, but I'm running into a > >> couple of issues that I'm not sure how to handle. > >> > >> Note that the test suite currently passes on macOS 11[1], so these > >> failures have to be a consequence to changes made to macOS that we > >> haven't yet learned how to cope with. > >> > >> The first one is in vircryptotest: > >> > >> Encrypt aes265cbc ... Expected ciphertext doesn't match > >> > >> I've added some debug statements and it looks like the generated data > >> is different every time, which seems like a pretty good indication > >> that virrandommock is not being picked up correctly. This is not the > >> only test program that uses that specific mock though, so I'm not > >> sure what makes it fail when all the others are succeeding. > > I believe that the following patch fixes this one: > > From: Christophe de Dinechin > Date: Mon, 8 Aug 2022 20:14:08 +0200 > Subject: [PATCH] tests: Pass the flat_namespace option to the linker > > This fixes vircryptotest on macOS 12 (Monterey). > > The test relies on library injection (using DYLD_INSERT_LIBRARIES) > to replace the normal random functions with functions giving predictable > results, defined in virrandommock.c. However, using DYLD_INSERT_LIBRARIES > only works when building with flat namespaces. > > Adding the -Wl,-flat_namespace option to the linker fixes the problem. > The option was already defined in the top-level meson.build, but had been > forgotten in the test linker arguments. > > Signed-off-by: Christophe de Dinechin > Signed-off-by: Christophe de Dinechin > --- > tests/meson.build | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/meson.build b/tests/meson.build > index bc9d8ccc4c..d6b1bb2bf0 100644 > --- a/tests/meson.build > +++ b/tests/meson.build > @@ -28,6 +28,7 @@ tests_dep = declare_dependency( >], >link_args: ( > libvirt_export_dynamic > ++ libvirt_flat_namespace > + coverage_flags >), > ) > -- > 2.37.1 > > > Could you please check? Yeah, this seems to help and the change makes sense to me. I wonder why we didn't run into this much earlier though? As I mentioned, the test runs successfully as-is on macOS 11. Plus, many other tests rely on library injection and yet work okay even without this change. Anyway, I'm happy to add my Reviewed-by: Andrea Bolognani to this patch and push it. The authorship information looks a bit funky though, with the two S-o-bs... Should I drop the @redhat.com one and only keep the one with your personal email address, matching the commit authorship information? -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] conf: Simplify IOMMU device validation
Instead of duplicating the list of attributes that are not
allowed for some of the IOMMU models, use two separate switch
statements: one for the attributes and one for the address.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_validate.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 88205c64e0..1c78a3d31c 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2633,6 +2633,7 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu)
{
switch (iommu->model) {
case VIR_DOMAIN_IOMMU_MODEL_SMMUV3:
+case VIR_DOMAIN_IOMMU_MODEL_VIRTIO:
if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT ||
iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT ||
iommu->eim != VIR_TRISTATE_SWITCH_ABSENT ||
@@ -2643,8 +2644,15 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu)
virDomainIOMMUModelTypeToString(iommu->model));
return -1;
}
-G_GNUC_FALLTHROUGH;
+break;
+case VIR_DOMAIN_IOMMU_MODEL_INTEL:
+case VIR_DOMAIN_IOMMU_MODEL_LAST:
+break;
+}
+
+switch (iommu->model) {
+case VIR_DOMAIN_IOMMU_MODEL_SMMUV3:
case VIR_DOMAIN_IOMMU_MODEL_INTEL:
if (iommu->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
virReportError(VIR_ERR_XML_ERROR,
@@ -2655,18 +2663,6 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu)
break;
case VIR_DOMAIN_IOMMU_MODEL_VIRTIO:
-if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT ||
-iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT ||
-iommu->eim != VIR_TRISTATE_SWITCH_ABSENT ||
-iommu->iotlb != VIR_TRISTATE_SWITCH_ABSENT ||
-iommu->aw_bits != 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("iommu model '%s' doesn't support additional
attributes"),
- virDomainIOMMUModelTypeToString(iommu->model));
-return -1;
-}
-break;
-
case VIR_DOMAIN_IOMMU_MODEL_LAST:
break;
}
--
2.37.1
[libvirt PATCH v2 2/2] kbase: Document how to disable Secure Boot entirely
In most cases, disabling the secure-boot or the enrolled-keys firmware feature will achieve the same result: allowing an unsigned operating system to run. Right now we're only documenting the latter configuration. Add the former as well, and explain the difference between the two. Signed-off-by: Andrea Bolognani --- docs/kbase/secureboot.rst | 31 --- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/docs/kbase/secureboot.rst b/docs/kbase/secureboot.rst index 5fa59ad5e2..4340454a7b 100644 --- a/docs/kbase/secureboot.rst +++ b/docs/kbase/secureboot.rst @@ -19,7 +19,17 @@ ask for Secure Boot to be enabled with -and for it to be disabled with +and for it to be disabled with either + +:: + + + + + + + +or :: @@ -30,8 +40,10 @@ and for it to be disabled with -These configuration will cause unsigned guest operating systems to -be rejected and allowed respectively. +The first configuration will cause unsigned guest operating systems +to be rejected, while the remaining two will allow running them. See +below for a more detailed explanation of how each knob affects the +firmware selection process. Older libvirt versions @@ -103,3 +115,16 @@ The opposite configuration, where the feature is explicitly disabled, will result in no keys being present in the NVRAM file. Unable to verify signatures, the firmware will allow even unsigned operating systems to run. + +If running unsigned code is desired, it's also possible to ask for +the ``secure-boot`` feature to be disabled, which will cause libvirt +to pick a build of EDKII that doesn't have Secure Boot support at +all. + +The main difference between using a build of EDKII that has Secure +Boot support but without keys enrolled and one that doesn't have +Secure Boot support at all is that, with the former, you could enroll +your own keys and securely run an operating system that you've built +and signed yourself. If you are only planning to run existing, +off-the-shelf operating system images, then the two configurations +are functionally equivalent. -- 2.37.1
[libvirt PATCH v2 0/2] kbase: Improve secureboot page
Changes from [v1]: * document configuration where Secure Boot support is completely absent, not just inactive; * explain in more detail how the various firmware features interact with one another. [v1] https://listman.redhat.com/archives/libvir-list/2022-August/233449.html Andrea Bolognani (2): kbase: Always explicitly enable secure-boot firmware feature kbase: Document how to disable Secure Boot entirely docs/kbase/secureboot.rst | 34 +++--- 1 file changed, 31 insertions(+), 3 deletions(-) -- 2.37.1
[libvirt PATCH v2 1/2] kbase: Always explicitly enable secure-boot firmware feature
It should be enough to enable or disable the enrolled-keys feature to control whether Secure Boot is enforced, but there's a slight complication: many distro packages for edk2 include, in addition to general purpose firmware images, builds that are targeting the Confidential Computing use case. For those, the firmware descriptor will not advertise the enrolled-keys feature, which will technically make them suitable for satisfying a configuration such as In practice, users will expect the general purpose build to be used in this case. Explicitly asking for the secure-boot feature to be enabled achieves that result at the cost of some slight additional verbosity. Signed-off-by: Andrea Bolognani --- docs/kbase/secureboot.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/kbase/secureboot.rst b/docs/kbase/secureboot.rst index 8f151c1f2a..5fa59ad5e2 100644 --- a/docs/kbase/secureboot.rst +++ b/docs/kbase/secureboot.rst @@ -14,6 +14,7 @@ ask for Secure Boot to be enabled with + @@ -24,6 +25,7 @@ and for it to be disabled with + @@ -44,6 +46,7 @@ snippet: + -- 2.37.1
Re: [libvirt PATCH] kbase: Always explicitly enable secure-boot firmware feature
On Thu, Aug 04, 2022 at 10:29:12AM +0100, Daniel P. Berrangé wrote: > On Thu, Aug 04, 2022 at 03:32:32AM -0500, Andrea Bolognani wrote: > > On Wed, Aug 03, 2022 at 05:29:15PM +0100, Daniel P. Berrangé wrote: > > > On Wed, Aug 03, 2022 at 06:15:24PM +0200, Andrea Bolognani wrote: > > > > > > > > > > > > + > > > > > > > > > > > > > > > > > > If we want secureboot disabled, this looks wrong. It just enables > > > secureboot, but without any keys. We need enabled=no to ask for > > > a firmware without SecureBoot at all. > > > > Mh. From a practical standpoint, the scenarios > > > > * firmware has secure boot support but there are no enrolled keys > > * firmware doesn't have secure boot support > > > > are pretty much equivalent: either way, unsigned code will be allowed > > to run. > > Yes & no - one allows you to enroll custom keys, the other doesn't > allow it. For most people that distinction doesn't matter but it is > a significant difference. > > I don't mind documenting both, but we should explain why we are > illustrating two different mechanisms, as when the question is > "how to I disable secureboot" an answer saying "secure_boot enabled=yes" > simply looks wrong. Right :) There's an underlying issue with naming, because what most people would think of when talking about the state of secure boot is, counterintuitively, not actually controlled by the "secure-boot" firmware feature. That's the reason why I wrote this kbase in the first place: if the XML mechanism matched people's intuition, it wouldn't be necessary. I have tried to explain why there are two firmware features controlling what is, in most people's mind, a binary knob in the "additional information" section. I'll attempt to expand upon that without making it too complicated in v2. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH] kbase: Always explicitly enable secure-boot firmware feature
On Wed, Aug 03, 2022 at 05:29:15PM +0100, Daniel P. Berrangé wrote: > On Wed, Aug 03, 2022 at 06:15:24PM +0200, Andrea Bolognani wrote: > > > > > > + > > > > > > > > If we want secureboot disabled, this looks wrong. It just enables > secureboot, but without any keys. We need enabled=no to ask for > a firmware without SecureBoot at all. Mh. From a practical standpoint, the scenarios * firmware has secure boot support but there are no enrolled keys * firmware doesn't have secure boot support are pretty much equivalent: either way, unsigned code will be allowed to run. And, although that doesn't seem the case anywhere at the moment, I can imagine that at some point a distro will decide to only ship a single firmware build. That would be comparable to how you can't really find a motherboard that doesn't come with secure boot support at this point, and your only choice is whether you want the feature to be active. So I don't disagree with you, but I'm also not completely convinced that we should *not* document a way to disable secure boot while still picking a firmware build that contains the feature. Would squashing the diff below be a good enough compromise in your opinion? diff --git a/docs/kbase/secureboot.rst b/docs/kbase/secureboot.rst index 5fa59ad5e2..68732701df 100644 --- a/docs/kbase/secureboot.rst +++ b/docs/kbase/secureboot.rst @@ -19,7 +19,17 @@ ask for Secure Boot to be enabled with -and for it to be disabled with +and for it to be disabled with either + +:: + + + + + + + +or :: @@ -30,8 +40,8 @@ and for it to be disabled with -These configuration will cause unsigned guest operating systems to -be rejected and allowed respectively. +The first configuration will cause unsigned guest operating systems +to be rejected, while the remaining two will allow running them. Older libvirt versions -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] kbase: Always explicitly enable secure-boot firmware feature
It should be enough to enable or disable the enrolled-keys feature to control whether Secure Boot is enforced, but there's a slight complication: many distro packages for edk2 include, in addition to general purpose firmware images, builds that are targeting the Confidential Computing use case. For those, the firmware descriptor will not advertise the enrolled-keys feature, which will technically make them suitable for satisfying a configuration such as In practice, users will expect the general purpose build to be used in this case. Explicitly asking for the secure-boot feature to be enabled achieves that result at the cost of some slight additional verbosity. Signed-off-by: Andrea Bolognani --- docs/kbase/secureboot.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/kbase/secureboot.rst b/docs/kbase/secureboot.rst index 8f151c1f2a..5fa59ad5e2 100644 --- a/docs/kbase/secureboot.rst +++ b/docs/kbase/secureboot.rst @@ -14,6 +14,7 @@ ask for Secure Boot to be enabled with + @@ -24,6 +25,7 @@ and for it to be disabled with + @@ -44,6 +46,7 @@ snippet: + -- 2.37.1
Re: [PATCH 3/3] testutilsqemu: Fake TPM versions
On Tue, Aug 02, 2022 at 09:03:01AM +0200, Michal Prívozník wrote:
> On 8/1/22 18:10, Andrea Bolognani wrote:
> > On Fri, Jul 29, 2022 at 09:42:13AM +0200, Michal Privoznik wrote:
> >> +++ b/tests/testutilsqemu.c
> >> @@ -150,12 +150,13 @@ bool
> >> virTPMSwtpmSetupCapsGet(virTPMSwtpmSetupFeature cap)
> >> {
> >> switch (cap) {
> >> +case VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2:
> >> +case VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0:
> >> +return true;
> >> case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD:
> >> case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES:
> >> case VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT:
> >> case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS:
> >> -case VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2:
> >> -case VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0:
> >> case VIR_TPM_SWTPM_SETUP_FEATURE_LAST:
> >> break;
> >> }
> >
> > So our test suite will work against a mocked TPM that supports a very
> > small set of hardcoded capabilities. Would it make sense to extend
> > this so that it's possible to control things as the test case level,
> > so that we can have coverage for things like e.g. trying to use TPM
> > 1.2 when the swtpm binary only supports TPM 2.0?
>
> Good point. We could have an environment variable that, if set, would
> make this function return just a subset of TPM versions. And then, when
> we want to test scenario you suggest we could do the following:
>
> setenv("VIR_TEST_MOCK_FAKE_TPM_VERSION", "1.2");
> DO_TEST_FAILURE("guest-tpm-2.0");
> unsetenv("VIR_TEST_MOCK_FAKE_TPM_VERSION");
>
> Alternatively, we can modify the DO_TEST_* macro so that it accepts TPM
> version and sets the ENV var accordingly, but I'd rather not do that,
> because it's not very extensible.
That covers the basic scenario of mocking a build of swtpm that has
only one TPM version enabled, but for example until very recently
many builds supported both versions. And no other recent / optional
feature can be toggled this way.
Without thinking too much about it or prototyping it, having
DO_TEST_FULL() accept a new ARG_SWTPMSETUP_FEATURE argument that
contains a list of virTPMSwtpmSetupFeature sounds like a reasonable
enough approach. The more commonly used macros would of course not
expose this argument.
> > That'd all be follow-up work, of course. Your change is good to have
> > as is :)
>
> In fact, I can just not merge this patch and send v2 of this patch that
> implements the idea from above. I'll push the other two though, because
> they fix the issue.
This one's already nice and the discussion on how to make it more
generic could end up taking a bit. I'd say just push it as is now,
then work on improving upon it later.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 3/3] testutilsqemu: Fake TPM versions
On Fri, Jul 29, 2022 at 09:42:13AM +0200, Michal Privoznik wrote:
> +++ b/tests/testutilsqemu.c
> @@ -150,12 +150,13 @@ bool
> virTPMSwtpmSetupCapsGet(virTPMSwtpmSetupFeature cap)
> {
> switch (cap) {
> +case VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2:
> +case VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0:
> +return true;
> case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD:
> case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_CREATE_CONFIG_FILES:
> case VIR_TPM_SWTPM_SETUP_FEATURE_TPM12_NOT_NEED_ROOT:
> case VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_RECONFIGURE_PCR_BANKS:
> -case VIR_TPM_SWTPM_SETUP_FEATURE_TPM_1_2:
> -case VIR_TPM_SWTPM_SETUP_FEATURE_TPM_2_0:
> case VIR_TPM_SWTPM_SETUP_FEATURE_LAST:
> break;
> }
So our test suite will work against a mocked TPM that supports a very
small set of hardcoded capabilities. Would it make sense to extend
this so that it's possible to control things as the test case level,
so that we can have coverage for things like e.g. trying to use TPM
1.2 when the swtpm binary only supports TPM 2.0?
That'd all be follow-up work, of course. Your change is good to have
as is :)
Reviewed-by: Andrea Bolognani
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 2/3] testutilsqemu: Mock virTPMSwtpmSetupCapsGet()
On Fri, Jul 29, 2022 at 09:42:12AM +0200, Michal Privoznik wrote: > In a recent commit of v8.5.0-85-g430ab88ab1 I've made domaincaps > XML report supported TPM versions. This was done by calling > virTPMSwtpmSetupCapsGet(). But this function isn't mocked and > thus domaincapstest calls the real implementation, which tries to > execute swtpm_setup binary. This fails, because > virFindFileInPath() is mocked in such way that it returns NULL > for anything else than qemu-*. > > Anyway, while the real binary is not executed after all, we > should mock the function which tries to execute it so that > predictable result is returned. > > Signed-off-by: Michal Privoznik > --- > tests/testutilsqemu.c | 18 ++ > 1 file changed, 18 insertions(+) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 1/3] virtpm: Use corresponding type for argument for virTPM*CapsGet()
On Fri, Jul 29, 2022 at 09:42:11AM +0200, Michal Privoznik wrote: > In virtpm.h there are two functions exposed for querying swtpm > and swtpm_setup capabilieis: virTPMSwtpmCapsGet() and *capabilities Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 0/2] qemu: support stateless UEFI firmware
On Fri, Jul 22, 2022 at 05:23:15PM +0100, Daniel P. Berrangé wrote: > This is to enable SEV builds of UEFI which provide only a single CODE.fd > file, with not VARS.fd. This is a significant enough user-visible change that a NEWS entry for it would be warranted. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/2] qemu: support use of stateless EFI firmware
On Fri, Jul 22, 2022 at 05:23:17PM +0100, Daniel P. Berrangé wrote:
> VIR_DEBUG("decided on firmware '%s' template '%s' NVRAM '%s'",
>def->os.loader->path,
> - def->os.loader->nvramTemplate,
> - def->os.loader->nvram->path);
> + NULLSTR(def->os.loader->nvramTemplate),
> + NULLSTR(def->os.loader->nvram ?
> def->os.loader->nvram->path : NULL));
It would be nice to reflect in the live XML whether or not a
stateless firmware has been picked. Something along the lines of the
hastily thrown together, very lightly tested diff below should do the
trick.
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index eb7abb0b32..68d562285e 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1200,6 +1200,9 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
def->os.loader->nvram->format = VIR_STORAGE_FILE_RAW;
qemuDomainNVRAMPathFormat(cfg, def,
>os.loader->nvram->path);
}
+def->os.loader->stateless = VIR_TRISTATE_BOOL_NO;
+} else if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_STATELESS) {
+def->os.loader->stateless = VIR_TRISTATE_BOOL_YES;
}
VIR_DEBUG("decided on firmware '%s' template '%s' NVRAM '%s'",
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 1/2] conf: support stateless UEFI firmware
Apologies for this feedback coming very late - not just post-merge
but also extremely close to release.
On Fri, Jul 22, 2022 at 05:23:16PM +0100, Daniel P. Berrangé wrote:
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 3ea094e64c..4199abfd1a 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -242,7 +242,11 @@ harddisk, cdrom, network) determining where to
> obtain/find the boot image.
> firmwares may implement the Secure boot feature. Attribute ``secure`` can
> be
> used to tell the hypervisor that the firmware is capable of Secure Boot
> feature.
> It cannot be used to enable or disable the feature itself in the firmware.
> - :since:`Since 2.1.0`
> + :since:`Since 2.1.0`. If the loader is marked as read-only, then with
> UEFI it
> + is assumed that there will be a writable NVRAM available. In some cases,
> + however, it may be desirable for the loader to run without any NVRAM,
> discarding
> + any config changes on shutdown. The ``stateless`` flag can be used to
> control
> + this behaviour, when set to ``no`` NVRAM will never be created.
Isn't the actual behavior the opposite of what you're describing
here? That is, stateless=yes is what causes the NVRAM file to not be
created.
> +++ b/src/conf/domain_validate.c
> @@ -1672,6 +1672,32 @@ virDomainDefOSValidate(const virDomainDef *def,
> }
> }
>
> +if (loader->stateless == VIR_TRISTATE_BOOL_YES) {
> +if (loader->nvramTemplate) {
> +virReportError(VIR_ERR_XML_DETAIL, "%s",
> + _("NVRAM template is not permitted when loader is
> stateless"));
> +return -1;
> +}
> +
> +if (loader->nvram) {
> +virReportError(VIR_ERR_XML_DETAIL, "%s",
> + _("NVRAM is not permitted when loader is
> stateless"));
> +return -1;
> +}
> +} else if (loader->stateless == VIR_TRISTATE_BOOL_NO) {
> +if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) {
> +if (def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH) {
> +virReportError(VIR_ERR_XML_DETAIL, "%s",
> + _("Only pflash loader type permits NVRAM"));
> +return -1;
> +}
> +} else if (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) {
> +virReportError(VIR_ERR_XML_DETAIL, "%s",
> + _("Only EFI firmware permits NVRAM"));
> +return -1;
> +}
These last two error messages could be improved IMO.
Consider the firmware-auto-bios-not-stateless test case, where the
input configuration looks like
hvm
In this case, printing out
Only EFI firmware permits NVRAM
is a bit confusing, since the user has not directly mentioned NVRAM
anywhere. Something along the lines of
virReportError(VIR_ERR_XML_DETAIL,
_("Firmware type '%s' only supports stateless operations"),
virDomainOsDefFirmwareTypeToString(def->os.firmware));
would be more understandable and actionable, I think.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 3/3] ci: Drop Debian 10
On Fri, Jul 22, 2022 at 12:56:00PM +0200, Peter Krempa wrote: > Debian 10 reaches EOL in August of 2022. > > Signed-off-by: Peter Krempa > --- > .../debian-10-cross-aarch64.Dockerfile| 125 -- > .../debian-10-cross-armv6l.Dockerfile | 124 - > .../debian-10-cross-armv7l.Dockerfile | 125 -- > ci/containers/debian-10-cross-i686.Dockerfile | 124 - > ci/containers/debian-10-cross-mips.Dockerfile | 124 - > .../debian-10-cross-mips64el.Dockerfile | 124 - > .../debian-10-cross-mipsel.Dockerfile | 124 - > .../debian-10-cross-ppc64le.Dockerfile| 124 - > .../debian-10-cross-s390x.Dockerfile | 124 - > ci/containers/debian-10.Dockerfile| 105 --- > ci/gitlab/builds.yml | 119 - > ci/gitlab/containers.yml | 80 --- > ci/manifest.yml | 40 -- > 13 files changed, 1462 deletions(-) > delete mode 100644 ci/containers/debian-10-cross-aarch64.Dockerfile > delete mode 100644 ci/containers/debian-10-cross-armv6l.Dockerfile > delete mode 100644 ci/containers/debian-10-cross-armv7l.Dockerfile > delete mode 100644 ci/containers/debian-10-cross-i686.Dockerfile > delete mode 100644 ci/containers/debian-10-cross-mips.Dockerfile > delete mode 100644 ci/containers/debian-10-cross-mips64el.Dockerfile > delete mode 100644 ci/containers/debian-10-cross-mipsel.Dockerfile > delete mode 100644 ci/containers/debian-10-cross-ppc64le.Dockerfile > delete mode 100644 ci/containers/debian-10-cross-s390x.Dockerfile > delete mode 100644 ci/containers/debian-10.Dockerfile Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 2/3] ci: Move active Debian-10 jobs to Debian-11
On Fri, Jul 22, 2022 at 12:55:59PM +0200, Peter Krempa wrote: > Debian 10 will reach EOL in august of 2022 and thus libvirt will no > longer target it. Move CI jobs over to Debian-11. > > Signed-off-by: Peter Krempa > --- > ci/gitlab/builds.yml | 4 ++-- > ci/gitlab/containers.yml | 3 ++- > ci/manifest.yml | 5 ++--- > 3 files changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 1/3] ci: Move builds from alpine-314 to alpine-315
On Fri, Jul 22, 2022 at 12:55:58PM +0200, Peter Krempa wrote: > +++ b/ci/manifest.yml > @@ -19,12 +19,9 @@ targets: >RPM: skip >CC: clang > > - alpine-314: x86_64 > - >alpine-315: > jobs: >- arch: x86_64 > -builds: false This makes for a cleaner diff, but you could also optimize for a more minimal manifest and change the entry to alpine-315: x86_64 instead. Either way, Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 3/3] ci: Drop Debian 10
On Mon, Jul 25, 2022 at 09:39:03AM +0200, Erik Skultety wrote: > On Fri, Jul 22, 2022 at 12:56:00PM +0200, Peter Krempa wrote: > > Debian 10 reaches EOL in August of 2022. > > While I don't have a serious issue with this patch which also conforms to our > platform policy I don't think we need to always be so keen on dropping > platforms ASAP manually. IOW I think all platform changes should be ideally be > performed with lcitool, which however, won't be able to drop Debian 10 so soon > sice QEMU will still likely rely on Debian 10 in their CI for a while unless > matters around platform support have changed in QEMU??. QEMU has the same platform support policy as libvirt these days, but their release schedule is different and that can result in keeping a target platform around for a few additional months. This influences how soon we can drop the platform in question from lcitool, but shouldn't affect libvirt: from libvirt's point of view, dropping a target platform is just a matter of updating the manifest, and there is no compelling reason to wait until lcitool has dropped support for it. If anything, it's more convenient to do it *before* that has happened, as in that case you can just run 'lcitool manifest' and have the tool dutifully delete the now-unused files for you instead of having to hunt them down manually :) -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 2/3] qemu: Assign default alias to IOMMU devices
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_alias.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 7efd91051e..7b91fe3141 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -597,6 +597,14 @@ qemuAssignDeviceVsockAlias(virDomainVsockDef *vsock)
}
+static void
+qemuAssignDeviceIOMMUAlias(virDomainIOMMUDef *iommu)
+{
+if (!iommu->info.alias)
+iommu->info.alias = g_strdup("iommu0");
+}
+
+
int
qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps *qemuCaps)
{
@@ -681,6 +689,8 @@ qemuAssignDeviceAliases(virDomainDef *def, virQEMUCaps
*qemuCaps)
if (def->vsock) {
qemuAssignDeviceVsockAlias(def->vsock);
}
+if (def->iommu)
+qemuAssignDeviceIOMMUAlias(def->iommu);
return 0;
}
--
2.35.3
[libvirt PATCH 3/3] qemu: Add IOMMU device alias to command line
Note that we can only do this for intel-iommu and virtio-iommu,
which are configured using -device; smmuv3 is configured using
a machine type property, so there's no room on the command line
for an alias in that case.
https://bugzilla.redhat.com/show_bug.cgi?id=2108483
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_command.c | 2 ++
tests/qemuxml2argvdata/intel-iommu-aw-bits.x86_64-latest.args | 2 +-
.../intel-iommu-caching-mode.x86_64-latest.args | 2 +-
.../intel-iommu-device-iotlb.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/intel-iommu-eim.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/intel-iommu.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/virtio-iommu-aarch64.aarch64-latest.args | 2 +-
tests/qemuxml2argvdata/virtio-iommu-x86_64.x86_64-latest.args | 2 +-
8 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 262fffe5fe..30c9bbbf2e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6441,6 +6441,7 @@ qemuBuildIOMMUCommandLine(virCommand *cmd,
case VIR_DOMAIN_IOMMU_MODEL_INTEL:
if (virJSONValueObjectAdd(,
"s:driver", "intel-iommu",
+ "s:id", iommu->info.alias,
"S:intremap", qemuOnOffAuto(iommu->intremap),
"T:caching-mode", iommu->caching_mode,
"S:eim", qemuOnOffAuto(iommu->eim),
@@ -6457,6 +6458,7 @@ qemuBuildIOMMUCommandLine(virCommand *cmd,
case VIR_DOMAIN_IOMMU_MODEL_VIRTIO:
if (virJSONValueObjectAdd(,
"s:driver", "virtio-iommu",
+ "s:id", iommu->info.alias,
NULL) < 0) {
return -1;
}
diff --git a/tests/qemuxml2argvdata/intel-iommu-aw-bits.x86_64-latest.args
b/tests/qemuxml2argvdata/intel-iommu-aw-bits.x86_64-latest.args
index f61ff3f8ff..df11e3ddab 100644
--- a/tests/qemuxml2argvdata/intel-iommu-aw-bits.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/intel-iommu-aw-bits.x86_64-latest.args
@@ -27,7 +27,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-no-shutdown \
-no-acpi \
-boot strict=on \
--device '{"driver":"intel-iommu","intremap":"on","aw-bits":48}' \
+-device '{"driver":"intel-iommu","id":"iommu0","intremap":"on","aw-bits":48}' \
-audiodev '{"id":"audio1","driver":"none"}' \
-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/intel-iommu-caching-mode.x86_64-latest.args
b/tests/qemuxml2argvdata/intel-iommu-caching-mode.x86_64-latest.args
index 083c6bfe4b..47976fa6a7 100644
--- a/tests/qemuxml2argvdata/intel-iommu-caching-mode.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/intel-iommu-caching-mode.x86_64-latest.args
@@ -27,7 +27,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-no-shutdown \
-no-acpi \
-boot strict=on \
--device '{"driver":"intel-iommu","intremap":"on","caching-mode":true}' \
+-device
'{"driver":"intel-iommu","id":"iommu0","intremap":"on","caching-mode":true}' \
-audiodev '{"id":"audio1","driver":"none"}' \
-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/intel-iommu-device-iotlb.x86_64-latest.args
b/tests/qemuxml2argvdata/intel-iommu-device-iotlb.x86_64-latest.args
index fc06e15191..af36c45292 100644
--- a/tests/qemuxml2argvdata/intel-iommu-device-iotlb.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/intel-iommu-device-iotlb.x86_64-latest.args
@@ -27,7 +27,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-no-shutdown \
-no-acpi \
-boot strict=on \
--device '{"driver":"intel-iommu","intremap":"on","device-iotlb":true}' \
+-device
'{"driver":"intel-iommu","id":"iommu0","intremap":"on","device-iotlb":true}' \
-audiodev '{"id":"audio1","driver":"none"}' \
-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/intel-iommu-eim.x86_64-latest.args
b/tests/qemuxml2argvdata/intel-iommu-eim.x86_64-latest.args
index c8dd31c5fe..8e2b54d1f9 100644
--- a/tests/qemuxml2argvdata/in
[libvirt PATCH 1/3] schema: Allow IOMMU devices to have aliases
Signed-off-by: Andrea Bolognani --- src/conf/schemas/domaincommon.rng | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 2f07c25430..d15dd33f47 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -5658,6 +5658,9 @@ + + + -- 2.35.3
[libvirt PATCH 0/3] qemu: Allow IOMMU devices to have aliases
Andrea Bolognani (3): schema: Allow IOMMU devices to have aliases qemu: Assign default alias to IOMMU devices qemu: Add IOMMU device alias to command line src/conf/schemas/domaincommon.rng | 3 +++ src/qemu/qemu_alias.c | 10 ++ src/qemu/qemu_command.c| 2 ++ .../intel-iommu-aw-bits.x86_64-latest.args | 2 +- .../intel-iommu-caching-mode.x86_64-latest.args| 2 +- .../intel-iommu-device-iotlb.x86_64-latest.args| 2 +- .../intel-iommu-eim.x86_64-latest.args | 2 +- tests/qemuxml2argvdata/intel-iommu.x86_64-latest.args | 2 +- .../virtio-iommu-aarch64.aarch64-latest.args | 2 +- .../virtio-iommu-x86_64.x86_64-latest.args | 2 +- 10 files changed, 22 insertions(+), 7 deletions(-) -- 2.35.3
[libvirt PATCH 2/3] scripts: Add $DESTDIR support to meson-install-web.py
meson already supports $DESTDIR natively, but in this case
we're using a custom script and so we have to do some extra
work ourselves.
Signed-off-by: Andrea Bolognani
---
scripts/meson-install-web.py | 13 +
1 file changed, 13 insertions(+)
diff --git a/scripts/meson-install-web.py b/scripts/meson-install-web.py
index fdf407ba33..e7456fa750 100755
--- a/scripts/meson-install-web.py
+++ b/scripts/meson-install-web.py
@@ -6,10 +6,23 @@ import sys
from pathlib import Path
+destdir = os.getenv('DESTDIR')
+if destdir:
+destdir = Path(destdir)
+if not destdir.is_absolute():
+print('$DESTDIR must be an absolute path')
+sys.exit(1)
+
for desc in sys.argv[1:]:
inst = desc.split(':')
src = Path(inst[0])
dst = Path(inst[1])
+if destdir:
+# Turn dst into a relative path by dropping its first component
+# and append it to destdir to obtain the absolute destination
+# path that respects the value $DESTDIR found in the environment
+dst = Path(destdir, *dst.parts[1:])
+
dst.mkdir(parents=True, exist_ok=True)
shutil.copy(src, dst)
--
2.35.3
[libvirt PATCH 1/3] scripts: Port meson-install-web.py to pathlib
This will be useful later.
Signed-off-by: Andrea Bolognani
---
scripts/meson-install-web.py | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/scripts/meson-install-web.py b/scripts/meson-install-web.py
index a03f8523cd..fdf407ba33 100755
--- a/scripts/meson-install-web.py
+++ b/scripts/meson-install-web.py
@@ -4,7 +4,12 @@ import os
import shutil
import sys
+from pathlib import Path
+
for desc in sys.argv[1:]:
inst = desc.split(':')
-os.makedirs(inst[1], exist_ok=True)
-shutil.copy(inst[0], inst[1])
+src = Path(inst[0])
+dst = Path(inst[1])
+
+dst.mkdir(parents=True, exist_ok=True)
+shutil.copy(src, dst)
--
2.35.3
[libvirt PATCH 3/3] ci: Fix paths shown in the website
Right now we're setting the prefix to a custom path, which results in paths like /builds/libvirt/libvirt/vroot/etc/libvirt/virtqemud.conf ending up in the generated HTML. In order to avoid that, set the prefix and other installation paths to reasonable default values by passing -Dsystem=true and then take advantage of $DESTDIR support to still be able to write the HTML files without requiring root privileges. Reported-by: Martin Kletzander Signed-off-by: Andrea Bolognani --- .gitlab-ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6a8b89729f..d5c431dee0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -67,9 +67,9 @@ website: before_script: - *script_variables script: -- meson setup build --werror --prefix=$(pwd)/vroot || (cat build/meson-logs/meson-log.txt && exit 1) -- ninja -C build install-web -- mv vroot/share/doc/libvirt/html/ website +- meson setup build --werror -Dsystem=true || (cat build/meson-logs/meson-log.txt && exit 1) +- DESTDIR=$(pwd)/install ninja -C build install-web +- mv install/usr/share/doc/libvirt/html/ website artifacts: expose_as: 'Website' name: 'website' -- 2.35.3
[libvirt PATCH 0/3] ci: Fix paths shown in the website
Compare https://abologna.gitlab.io/-/libvirt/-/jobs/2741293181/artifacts/website/manpages/virtqemud.html#files with https://libvirt.org/manpages/virtqemud.html#files Andrea Bolognani (3): scripts: Port meson-install-web.py to pathlib scripts: Add $DESTDIR support to meson-install-web.py ci: Fix paths shown in the website .gitlab-ci.yml | 6 +++--- scripts/meson-install-web.py | 22 -- 2 files changed, 23 insertions(+), 5 deletions(-) -- 2.35.3
Re: [libvirt PATCH 00/28] Improve firmware autoselection
On Mon, Jun 27, 2022 at 11:07:35AM +0100, Daniel P. Berrangé wrote:
> On Mon, Jun 27, 2022 at 12:00:59PM +0200, Gerd Hoffmann wrote:
> > On Thu, Jun 23, 2022 at 06:14:12PM +0200, Andrea Bolognani wrote:
> > > The main motivation behind this series was making it as simple as
> > > possible ("one click") to enable Secure Boot for a VM.
> >
> > Heads up, and sort-of follow-up to the recent secure boot and smm (x86)
> > and tz (arm) discussion.
Thanks for the heads up, Gerd!
> > We'll most likely get a new secure boot variant soon. This will not
> > require smm, but it will also not support persistent variables. The
> > underlying idea is to simply re-initialize the variable store from
> > known-good ROM on each boot to compensate for the varstore not being
> > protected against the guest OS tampering with it.
> >
> > Which of course implies some drawbacks: The guest can't add keys (via
> > mokutil) for example, and turning off secure boot in firmware setup
> > wouldn't work either. There are enough use cases (like just booting
> > cloud images in secure boot mode) where this doesn't matter, so I
> > consider this useful nevertheless, but maybe a separate feature flag
> > like 'stateless-secure-boot' makes sense for that.
>
> Since the use case will be virt related, there's always the possibility
> of using host side tools to inject a custom key into the default varstore
> before the guest OS runs. That doesn't cover all possible mokutil
> scenarios, but at least addresses the big one of providing a firmware
> that trusts the user's keys, instead of the OS vendor keys.
>
> I don't think we need a 'stateles-secure-boot' flag, as thats
> implicit from mapping.mode=statusless and features.secure-boot
We don't currently offer a way to filter firmware builds based on
their mode. So on a machine where this new firmware is available, a
VM configuration like
might result in either a firmware with writable variables or a
stateless one being selected. If the user's expectation is that they
will be able to use mokutil inside the VM, the latter will not make
them happy.
If we had a separate feature, one could use
to ensure mokutils can be used.
Maybe we can make the mode filterable instead? Like
or something along those lines.
> > Not sure yet how to package that up, best is probably as stateless image
> > because that'll reduce the chances of getting it wrong, i.e. something
> > like this:
> >
> > {
> > "description": "OVMF with secure boot, no persistent vars",
> > "interface-types": [
> > "uefi"
> > ],
> > "mapping": {
> > "device": "flash",
> > "mode": "stateless",
> > "executable": {
> > "filename": "/usr/share/edk2/ovmf/OVMF.secboot.fd",
Just to be clear: the firmware build supporting this new, stateless
style of Secure Boot would be a completely separate one from the
existing OVMF.secboot.fd, right?
> > The idea idea should work for aarch64 too and remove the trustzone support
> > requirement.
Yeah, that'd be a pretty great outcome :)
--
Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 27/28] docs: Add kbase page for Secure Boot
Provide simple recipes for the most common high-level tasks. Signed-off-by: Andrea Bolognani --- docs/kbase/index.rst | 3 ++ docs/kbase/meson.build| 1 + docs/kbase/secureboot.rst | 102 ++ 3 files changed, 106 insertions(+) create mode 100644 docs/kbase/secureboot.rst diff --git a/docs/kbase/index.rst b/docs/kbase/index.rst index 8b710db85a..896ececdf2 100644 --- a/docs/kbase/index.rst +++ b/docs/kbase/index.rst @@ -61,6 +61,9 @@ Usage `Snapshots `__ Details about snapshotting a VM +`Secure Boot `__ +Enable and disable the Secure Boot feature + Debugging - diff --git a/docs/kbase/meson.build b/docs/kbase/meson.build index eb9c9544d6..c7eae3738f 100644 --- a/docs/kbase/meson.build +++ b/docs/kbase/meson.build @@ -15,6 +15,7 @@ docs_kbase_files = [ 'qemu-passthrough-security', 'rpm-deployment', 's390_protected_virt', + 'secureboot', 'secureusage', 'snapshots', 'systemtap', diff --git a/docs/kbase/secureboot.rst b/docs/kbase/secureboot.rst new file mode 100644 index 00..90c37d707c --- /dev/null +++ b/docs/kbase/secureboot.rst @@ -0,0 +1,102 @@ +=== +Secure Boot +=== + +.. contents:: + +Quick configuration +=== + +If you have libvirt 8.5.0 or newer, when creating a new VM you can +ask for Secure Boot to be enabled with + +:: + + + + + + + +and for it to be disabled with + +:: + + + + + + + +These configuration will cause unsigned guest operating systems to +be rejected and allowed respectively. + + +Older libvirt versions +== + +If your libvirt version is older than 8.5.0 but newer than 7.2.0, +then enabling Secure Boot requires a slightly more verbose XML +snippet: + +:: + + + + + + + + +Versions older than 7.2.0 require manually providing all information +about the firmware and are not covered here. Plese refer to `the +relevant documentation +<../formatdomain.html#operating-system-booting>`__ for details. + + +Changing an existing VM +=== + +Once the VM has been created, updating the XML configuration as +described above is **not** enough to change the Secure Boot status: +the NVRAM file associated with the VM has to be regenerated from its +template as well. + +In order to do that, update the XML and then start the VM with + +:: + + $ virsh start $vm --reset-nvram + +This option is only available starting with libvirt 8.1.0, so if your +version of libvirt is older than that you will have to delete the +NVRAM file manually before starting the VM. + +Most guest operating systems will be able to cope with the NVRAM file +being reinitialized, but in some cases the VM will be unable to boot +after the change. + + +Additional information +== + +There are two parts to enabling Secure Boot: the firmware supporting +the feature, and it being active. + +Most host operating systems ship a build of EDKII (the open source +EFI implementation used for QEMU VMs) that supports the Secure Boot +feature, but simply using such a build will not result in unsigned +guest operating systems being rejected: for that to happen, keys that +can be used to validate the operating system signature need to be +provided as well. + +Asking for the ``enrolled-keys`` firmware feature to be enabled will +cause libvirt to initialize the NVRAM file associated with the VM +from a template that contains a suitable set of keys. These keys +being present will cause the firmware to enforce the Secure Boot +signing requirements. + +The opposite configuration, where the feature is explicitly disabled, +will result in no keys being present in the NVRAM file. Unable to +verify signatures, the firmware will allow even unsigned operating +systems to run. -- 2.35.3
[libvirt PATCH 20/28] conf: Always parse all firmware information
Currently we're simply ignoring some elements and attributes,
such as the loader path, when firmware autoselection is enabled
because we know we're not going to use them.
This makes sense, but has the unfortunate consequence of
confusing users who experience part of their configuration
simply going away for no apparent reason.
A more user-friendly approach is to produce meaningful error
messages in those scenarios. As a first step towards that goal,
stop conditionally parsing information.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 38 +++---
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0c6504348c..7947b1f5e6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18018,8 +18018,7 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
xmlNodePtr nvramSourceNode,
xmlXPathContextPtr ctxt,
virDomainXMLOption *xmlopt,
-unsigned int flags,
-bool fwAutoSelect)
+unsigned int flags)
{
g_autoptr(virStorageSource) src = virStorageSourceNew();
int typePresent;
@@ -18027,8 +18026,7 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
if (!nvramNode)
return 0;
-if (!fwAutoSelect)
-loader->nvramTemplate = virXMLPropString(nvramNode, "template");
+loader->nvramTemplate = virXMLPropString(nvramNode, "template");
src->format = VIR_STORAGE_FILE_RAW;
@@ -18070,33 +18068,29 @@ virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
xmlNodePtr nvramSourceNode,
xmlXPathContextPtr ctxt,
virDomainXMLOption *xmlopt,
- unsigned int flags,
- bool fwAutoSelect)
+ unsigned int flags)
{
if (virDomainLoaderDefParseXMLNvram(loader,
nvramNode, nvramSourceNode,
-ctxt, xmlopt, flags,
-fwAutoSelect) < 0)
+ctxt, xmlopt, flags) < 0)
return -1;
if (!loaderNode)
return 0;
-if (!fwAutoSelect) {
-if (virXMLPropTristateBool(loaderNode, "readonly", VIR_XML_PROP_NONE,
- >readonly) < 0)
-return -1;
+if (virXMLPropTristateBool(loaderNode, "readonly", VIR_XML_PROP_NONE,
+ >readonly) < 0)
+return -1;
-if (virXMLPropEnum(loaderNode, "type", virDomainLoaderTypeFromString,
- VIR_XML_PROP_NONZERO, >type) < 0)
-return -1;
+if (virXMLPropEnum(loaderNode, "type", virDomainLoaderTypeFromString,
+ VIR_XML_PROP_NONZERO, >type) < 0)
+return -1;
-if (!(loader->path = virXMLNodeContentString(loaderNode)))
-return -1;
+if (!(loader->path = virXMLNodeContentString(loaderNode)))
+return -1;
-if (STREQ(loader->path, ""))
-VIR_FREE(loader->path);
-}
+if (STREQ(loader->path, ""))
+VIR_FREE(loader->path);
if (virXMLPropTristateBool(loaderNode, "secure", VIR_XML_PROP_NONE,
>secure) < 0)
@@ -18498,7 +18492,6 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
xmlNodePtr loaderNode = virXPathNode("./os/loader[1]", ctxt);
xmlNodePtr nvramNode = virXPathNode("./os/nvram[1]", ctxt);
xmlNodePtr nvramSourceNode = virXPathNode("./os/nvram/source[1]", ctxt);
-const bool fwAutoSelect = def->os.firmware !=
VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
if (!loaderNode && !nvramNode)
return 0;
@@ -18507,8 +18500,7 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
if (virDomainLoaderDefParseXML(def->os.loader,
loaderNode, nvramNode, nvramSourceNode,
- ctxt, xmlopt, flags,
- fwAutoSelect) < 0)
+ ctxt, xmlopt, flags) < 0)
return -1;
return 0;
--
2.35.3
[libvirt PATCH 19/28] conf: Reject enrolled-keys=yes with secure-boot=no
This combination doesn't make sense and so the firmware
autoselection logic will not be able to find a suitable firmware,
but it's more user-friendly to report a detailed error upfront.
Note that this check would ideally happen in the validate phase,
but if we moved it there we would no longer be able to
automatically enable secure-boot when enrolled-keys=yes. Since
the combination never resulted in a working configuration, the
chances of this causing real-world VMs to disappear are
extremely low.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c| 7 +++
...enrolled-keys-no-secboot.x86_64-latest.err | 1 +
...ware-auto-efi-enrolled-keys-no-secboot.xml | 21 +++
tests/qemuxml2argvtest.c | 1 +
4 files changed, 30 insertions(+)
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys-no-secboot.x86_64-latest.err
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys-no-secboot.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d88d352fb6..0c6504348c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4879,6 +4879,13 @@ virDomainDefPostParseOs(virDomainDef *def)
if (def->os.firmwareFeatures &&
def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_ENROLLED_KEYS] ==
VIR_TRISTATE_BOOL_YES) {
+if
(def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_SECURE_BOOT] ==
VIR_TRISTATE_BOOL_NO) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("firmware feature 'enrolled-keys' cannot be
enabled when "
+ "firmware feature 'secure-boot' is disabled"));
+return -1;
+}
+
/* For all non-broken firmware builds, enrolled-keys implies
* secure-boot, and having the Secure Boot keys in the NVRAM file
* when the firmware doesn't support the Secure Boot feature doesn't
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys-no-secboot.x86_64-latest.err
b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys-no-secboot.x86_64-latest.err
new file mode 100644
index 00..989d3dbf5a
--- /dev/null
+++
b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys-no-secboot.x86_64-latest.err
@@ -0,0 +1 @@
+firmware feature 'enrolled-keys' cannot be enabled when firmware feature
'secure-boot' is disabled
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys-no-secboot.xml
b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys-no-secboot.xml
new file mode 100644
index 00..722793684c
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys-no-secboot.xml
@@ -0,0 +1,21 @@
+
+ fedora
+ 63840878-0deb-4095-97e6-fc444d9bc9fa
+ 8192
+ 1
+
+hvm
+
+
+
+
+
+
+
+
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d21b2d9154..b01ad8d4e9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1224,6 +1224,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("firmware-auto-efi-no-secboot");
DO_TEST_CAPS_LATEST("firmware-auto-efi-enrolled-keys");
DO_TEST_CAPS_LATEST("firmware-auto-efi-no-enrolled-keys");
+
DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-enrolled-keys-no-secboot");
DO_TEST_CAPS_ARCH_LATEST("firmware-auto-efi-aarch64", "aarch64");
DO_TEST_NOCAPS("clock-utc");
--
2.35.3
[libvirt PATCH 23/28] conf: Always parse firmware features
Regardless of whether firmware autoselection is in use, we
still want to parse the list of requested features. Doing this
will allow us to produce better error messages.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7947b1f5e6..fb8bf4cfec 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18439,20 +18439,6 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def,
int n = 0;
size_t i;
-if (!firmware)
-return 0;
-
-fw = virDomainOsDefFirmwareTypeFromString(firmware);
-
-if (fw <= 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("unknown firmware value %s"),
- firmware);
-return -1;
-}
-
-def->os.firmware = fw;
-
if ((n = virXPathNodeSet("./os/firmware/feature", ctxt, )) < 0)
return -1;
@@ -18479,6 +18465,20 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def,
def->os.firmwareFeatures = g_steal_pointer();
+if (!firmware)
+return 0;
+
+fw = virDomainOsDefFirmwareTypeFromString(firmware);
+
+if (fw <= 0) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("unknown firmware value %s"),
+ firmware);
+return -1;
+}
+
+def->os.firmware = fw;
+
return 0;
}
--
2.35.3
[libvirt PATCH 13/28] conf: Handle NVRAM in virDomainLoaderDefParseXML()
All the data in the element ends up in the same struct
as that coming from the element, so it makes sense to
have a single entry point for parsing an XML document into a
virDomainLoaderDef instance.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fcb468b465..d6a33a0f81 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18042,8 +18042,11 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
static int
-virDomainLoaderDefParseXML(xmlNodePtr node,
- virDomainLoaderDef *loader,
+virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virDomainXMLOption *xmlopt,
+ unsigned int flags,
bool fwAutoSelect)
{
if (!fwAutoSelect) {
@@ -18066,6 +18069,11 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
>secure) < 0)
return -1;
+if (virDomainLoaderDefParseXMLNvram(loader,
+ctxt, xmlopt, flags,
+fwAutoSelect) < 0)
+return -1;
+
return 0;
}
@@ -18467,16 +18475,12 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
def->os.loader = g_new0(virDomainLoaderDef, 1);
-if (virDomainLoaderDefParseXML(loader_node,
- def->os.loader,
+if (virDomainLoaderDefParseXML(def->os.loader,
+ loader_node,
+ ctxt, xmlopt, flags,
fwAutoSelect) < 0)
return -1;
-if (virDomainLoaderDefParseXMLNvram(def->os.loader,
-ctxt, xmlopt, flags,
-fwAutoSelect) < 0)
-return -1;
-
return 0;
}
--
2.35.3
[libvirt PATCH 22/28] conf: Validate firmware configuration more thoroughly
Generally speaking, when firmware autoselection is in use we
don't want any information to be provided manually. There are
two exceptions:
* we still want the path to the NVRAM file to be customizable;
* using was how you would ask for a
firmware that implements the Secure Boot feature in the
original approach to firmware autoselection, so we want to
keep that working.
Anything else should result in a descriptive error.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/327
Signed-off-by: Andrea Bolognani
---
src/conf/domain_validate.c| 48 +++
...firmware-auto-bios-nvram.x86_64-latest.err | 1 +
.../firmware-auto-bios-nvram.xml | 18 +++
...auto-efi-loader-insecure.x86_64-latest.err | 1 +
.../firmware-auto-efi-loader-insecure.xml | 18 +++
...are-auto-efi-loader-path.x86_64-latest.err | 1 +
.../firmware-auto-efi-loader-path.xml | 18 +++
tests/qemuxml2argvtest.c | 3 ++
8 files changed, 108 insertions(+)
create mode 100644
tests/qemuxml2argvdata/firmware-auto-bios-nvram.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-auto-bios-nvram.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-path.xml
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 1f6c32a816..87fdb677d1 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1606,6 +1606,54 @@ virDomainDefOSValidate(const virDomainDef *def,
_("firmware auto selection not implemented for this
driver"));
return -1;
}
+
+if (!loader)
+return 0;
+
+if (loader->readonly) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("loader attribute 'readonly' cannot be specified "
+ "when firmware autoselection is enabled"));
+return -1;
+}
+if (loader->type) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("loader attribute 'type' cannot be specified "
+ "when firmware autoselection is enabled"));
+return -1;
+}
+if (loader->path) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("loader path cannot be specified "
+ "when firmware autoselection is enabled"));
+return -1;
+}
+if (loader->nvramTemplate) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("nvram attribute 'template' cannot be specified "
+ "when firmware autoselection is enabled"));
+return -1;
+}
+
+/* We need to accept 'yes' here because the initial implementation
+ * of firmware autoselection used it as a way to request a firmware
+ * with Secure Boot support, so the error message is technically
+ * incorrect; however, we want to discourage people from using this
+ * attribute at all, so it's fine to be a bit more aggressive than
+ * it would be strictly required :) */
+if (loader->secure == VIR_TRISTATE_BOOL_NO) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("loader attribute 'secure' cannot be specified "
+ "when firmware autoselection is enabled"));
+return -1;
+}
+
+if (loader->nvram && def->os.firmware !=
VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) {
+virReportError(VIR_ERR_XML_DETAIL,
+ _("firmware type '%s' does not support nvram"),
+
virDomainOsDefFirmwareTypeToString(def->os.firmware));
+return -1;
+}
} else {
if (!loader)
return 0;
diff --git a/tests/qemuxml2argvdata/firmware-auto-bios-nvram.x86_64-latest.err
b/tests/qemuxml2argvdata/firmware-auto-bios-nvram.x86_64-latest.err
new file mode 100644
index 00..772beb49e2
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-bios-nvram.x86_64-latest.err
@@ -0,0 +1 @@
+firmware type 'bios' does not support nvram
diff --git a/tests/qemuxml2argvdata/firmware-auto-bios-nvram.xml
b/tests/qemuxml2argvdata/firmware-auto-bios-nvram.xml
new file mode 100644
index 00..6dad1e1f7f
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-bios-nvram.xml
@@ -0,0 +1,18 @@
+
+ fedora
+ 63840878
[libvirt PATCH 25/28] qemu_firmware: Enable loader.secure when requires-smm
Currently, a firmware configuration such as
will correctly pick a firmware that implements the Secure Boot
feature and initialize the NVRAM file so that it contains the
keys necessary to enforce the signing requirements. However, the
lack of a
element makes it possible for pflash writes to happen outside
of SMM mode. This means that the EFI secure variables where the
keys are stored could potentially be overwritten by malicious
code running in the guest, thus making it possible to circumvent
Secure Boot.
To prevent that from happening, automatically turn on the
loader.secure feature whenever a firmware that implements Secure
Boot is chosen by the firmware autoselection logic. This is
identical to the way we already automatically enable SMM in such
a scenario.
Note that, while this is technically a guest-visible change, it
will not affect migration of existings VMs and will not prevent
legitimate guest code from running.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c| 2 ++
.../firmware-auto-efi-enrolled-keys.x86_64-latest.args | 1 +
.../qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args | 1 +
.../firmware-auto-efi-secboot.x86_64-latest.args| 1 +
tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args | 1 +
5 files changed, 6 insertions(+)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index c8f462bfcf..5b2fa51a9c 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1240,6 +1240,8 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
case VIR_TRISTATE_SWITCH_LAST:
break;
}
+VIR_DEBUG("Enabling secure loader");
+def->os.loader->secure = VIR_TRISTATE_BOOL_YES;
break;
case QEMU_FIRMWARE_FEATURE_NONE:
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.x86_64-latest.args
index 885c83445d..7479b05af4 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.x86_64-latest.args
@@ -17,6 +17,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
-machine
pc-q35-4.0,usb=off,smm=on,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram
\
-accel kvm \
-cpu qemu64 \
+-global driver=cfi.pflash01,property=secure,value=on \
-m 8 \
-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":8388608}' \
-overcommit mem-lock=off \
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
index e37521b0a3..1061e93554 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
@@ -17,6 +17,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
-machine
pc-q35-4.0,usb=off,smm=on,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram
\
-accel kvm \
-cpu qemu64 \
+-global driver=cfi.pflash01,property=secure,value=on \
-m 8 \
-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":8388608}' \
-overcommit mem-lock=off \
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-secboot.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-secboot.x86_64-latest.args
index 885c83445d..7479b05af4 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-secboot.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-secboot.x86_64-latest.args
@@ -17,6 +17,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
-machine
pc-q35-4.0,usb=off,smm=on,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram
\
-accel kvm \
-cpu qemu64 \
+-global driver=cfi.pflash01,property=secure,value=on \
-m 8 \
-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":8388608}' \
-overcommit mem-lock=off \
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args
index 885c83445d..7479b05af4 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args
@@ -17,6 +17,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
-machine
pc-q35-4.0,usb=off,smm=on,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram
\
-accel kvm \
-cpu qemu64 \
+-global driver=cfi.pflash01,property=secure,value=on \
-m 8 \
-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":8388608}' \
-overcommit mem-lock=off \
--
2.35.3
[libvirt PATCH 17/28] conf: Enable secure-boot when enrolled-keys is enabled
The latter doesn't make sense without the former, so make that
visible in the XML.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 10 ++
.../firmware-auto-efi-enrolled-keys.x86_64-latest.xml | 1 +
2 files changed, 11 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9b82f391c4..3977b5040f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4876,6 +4876,16 @@ virDomainDefPostParseMemory(virDomainDef *def,
static void
virDomainDefPostParseOs(virDomainDef *def)
{
+if (def->os.firmwareFeatures &&
+
def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_ENROLLED_KEYS] ==
VIR_TRISTATE_BOOL_YES) {
+
+/* For all non-broken firmware builds, enrolled-keys implies
+ * secure-boot, and having the Secure Boot keys in the NVRAM file
+ * when the firmware doesn't support the Secure Boot feature doesn't
+ * make sense anyway. Reflect this fact explicitly in the XML */
+
def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_SECURE_BOOT] =
VIR_TRISTATE_BOOL_YES;
+}
+
if (!def->os.loader)
return;
diff --git
a/tests/qemuxml2xmloutdata/firmware-auto-efi-enrolled-keys.x86_64-latest.xml
b/tests/qemuxml2xmloutdata/firmware-auto-efi-enrolled-keys.x86_64-latest.xml
index aa08caa4f7..8dcc741c1e 100644
--- a/tests/qemuxml2xmloutdata/firmware-auto-efi-enrolled-keys.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/firmware-auto-efi-enrolled-keys.x86_64-latest.xml
@@ -8,6 +8,7 @@
hvm
+
--
2.35.3
[libvirt PATCH 16/28] conf: Always parse NVRAM path if present
Currently, the lack of a element results in the
element being completely ignored, but this is unnecessarily
limiting: even when firmware autoselection is in use, it should
be possible for the user to specify a custom path for the NVRAM
file.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 17 ++---
.../firmware-auto-efi-nvram.x86_64-latest.args | 2 +-
.../firmware-auto-efi-nvram.x86_64-latest.xml | 1 +
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 858242b3ae..9b82f391c4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18053,6 +18053,15 @@ virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
unsigned int flags,
bool fwAutoSelect)
{
+if (virDomainLoaderDefParseXMLNvram(loader,
+nvramNode, nvramSourceNode,
+ctxt, xmlopt, flags,
+fwAutoSelect) < 0)
+return -1;
+
+if (!loaderNode)
+return 0;
+
if (!fwAutoSelect) {
if (virXMLPropTristateBool(loaderNode, "readonly", VIR_XML_PROP_NONE,
>readonly) < 0)
@@ -18073,12 +18082,6 @@ virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
>secure) < 0)
return -1;
-if (virDomainLoaderDefParseXMLNvram(loader,
-nvramNode, nvramSourceNode,
-ctxt, xmlopt, flags,
-fwAutoSelect) < 0)
-return -1;
-
return 0;
}
@@ -18477,7 +18480,7 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
xmlNodePtr nvramSourceNode = virXPathNode("./os/nvram/source[1]", ctxt);
const bool fwAutoSelect = def->os.firmware !=
VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
-if (!loaderNode)
+if (!loaderNode && !nvramNode)
return 0;
def->os.loader = g_new0(virDomainLoaderDef, 1);
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
index 885c83445d..e37521b0a3 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
@@ -12,7 +12,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-fedora/master-key.aes"}'
\
-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.secboot.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
\
--blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/fedora_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"driver":"file","filename":"/path/to/fedora_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
\
-machine
pc-q35-4.0,usb=off,smm=on,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram
\
-accel kvm \
diff --git a/tests/qemuxml2xmloutdata/firmware-auto-efi-nvram.x86_64-latest.xml
b/tests/qemuxml2xmloutdata/firmware-auto-efi-nvram.x86_64-latest.xml
index 7e2e40036e..abd6ec079d 100644
--- a/tests/qemuxml2xmloutdata/firmware-auto-efi-nvram.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/firmware-auto-efi-nvram.x86_64-latest.xml
@@ -6,6 +6,7 @@
1
hvm
+/path/to/fedora_VARS.fd
--
2.35.3
[libvirt PATCH 05/28] tests: Use minimal hardware for firmware tests
When testing firmware selection, we don't really care about any
of the hardware assigned to the VM, and in fact it's better to
keep it as minimal as possible to make sure that the focus
remains on the firmware bits.
Signed-off-by: Andrea Bolognani
---
.../firmware-auto-bios.x86_64-latest.args | 12 +
tests/qemuxml2argvdata/firmware-auto-bios.xml | 53 +--
...mware-auto-efi-aarch64.aarch64-latest.args | 6 +--
.../firmware-auto-efi-aarch64.xml | 18 +--
...-auto-efi-loader-secure.x86_64-latest.args | 12 +
.../firmware-auto-efi-loader-secure.xml | 53 +--
...to-efi-no-enrolled-keys.x86_64-latest.args | 3 --
.../firmware-auto-efi-no-enrolled-keys.xml| 32 +--
.../firmware-auto-efi.x86_64-latest.args | 12 +
tests/qemuxml2argvdata/firmware-auto-efi.xml | 53 +--
...manual-bios-rw-implicit.x86_64-latest.args | 8 +--
.../firmware-manual-bios-rw-implicit.xml | 21 +---
...firmware-manual-bios-rw.x86_64-latest.args | 8 +--
.../firmware-manual-bios-rw.xml | 21 +---
.../firmware-manual-bios.args | 11 +---
.../qemuxml2argvdata/firmware-manual-bios.xml | 26 +
.../firmware-manual-efi-acpi-aarch64.args | 1 -
.../firmware-manual-efi-acpi-aarch64.xml | 4 +-
.../firmware-manual-efi-acpi-q35.args | 1 -
.../firmware-manual-efi-acpi-q35.xml | 4 +-
.../firmware-manual-efi-no-path.xml | 5 +-
.../firmware-manual-efi-noacpi-aarch64.args | 1 -
.../firmware-manual-efi-noacpi-aarch64.xml| 4 +-
.../firmware-manual-efi-noacpi-q35.xml| 4 +-
...e-manual-efi-nvram-file.x86_64-latest.args | 4 +-
.../firmware-manual-efi-nvram-file.xml| 6 +--
...efi-nvram-network-iscsi.x86_64-latest.args | 4 +-
...irmware-manual-efi-nvram-network-iscsi.xml | 9 +---
...l-efi-nvram-network-nbd.x86_64-latest.args | 4 +-
.../firmware-manual-efi-nvram-network-nbd.xml | 9 +---
...nual-efi-nvram-template.x86_64-latest.args | 4 +-
.../firmware-manual-efi-nvram-template.xml| 6 +--
.../firmware-manual-efi-secure.args | 9 +---
.../firmware-manual-efi-secure.xml| 19 +--
.../qemuxml2argvdata/firmware-manual-efi.args | 7 +--
.../qemuxml2argvdata/firmware-manual-efi.xml | 21 +---
.../firmware-manual-noefi-acpi-aarch64.xml| 7 +--
.../firmware-manual-noefi-acpi-q35.args | 4 --
.../firmware-manual-noefi-acpi-q35.xml| 7 +--
.../firmware-manual-noefi-noacpi-aarch64.args | 4 --
.../firmware-manual-noefi-noacpi-aarch64.xml | 7 +--
.../firmware-manual-noefi-noacpi-q35.args | 4 --
.../firmware-manual-noefi-noacpi-q35.xml | 7 +--
.../firmware-auto-bios.x86_64-latest.xml | 43 ++-
...rmware-auto-efi-aarch64.aarch64-latest.xml | 12 ++---
...e-auto-efi-loader-secure.x86_64-latest.xml | 43 ++-
...uto-efi-no-enrolled-keys.x86_64-latest.xml | 37 -
.../firmware-auto-efi.x86_64-latest.xml | 43 ++-
...re-manual-efi-nvram-file.x86_64-latest.xml | 9 +---
...-efi-nvram-network-iscsi.x86_64-latest.xml | 11 ++--
...al-efi-nvram-network-nbd.x86_64-latest.xml | 11 ++--
.../firmware-manual-efi.xml | 21 ++--
52 files changed, 111 insertions(+), 634 deletions(-)
mode change 12 => 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-no-enrolled-keys.x86_64-latest.xml
diff --git a/tests/qemuxml2argvdata/firmware-auto-bios.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-bios.x86_64-latest.args
index dd159e2604..1d45a8cfba 100644
--- a/tests/qemuxml2argvdata/firmware-auto-bios.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-bios.x86_64-latest.args
@@ -26,17 +26,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc \
-no-shutdown \
--global ICH9-LPC.disable_s3=0 \
--global ICH9-LPC.disable_s4=1 \
--boot menu=on,strict=on \
--device
'{"driver":"i82801b11-bridge","id":"pci.1","bus":"pcie.0","addr":"0x1e"}' \
--device
'{"driver":"pci-bridge","chassis_nr":2,"id":"pci.2","bus":"pci.1","addr":"0x0"}'
\
--device
'{"driver":"ioh3420","port":8,"chassis":3,"id":"pci.3","bus":"pcie.0","addr":"0x1"}'
\
--device
'{"driver":"ich9-usb-ehci1","id":"usb","bus":"pcie.0","addr":"0x1d.0x7"}' \
--device
'{"driver":"ich9-usb-uhci1","masterbus":"usb.0","firstport":0,"bus":"pcie.0","multifunction":true,"addr":"0x1d"}'
\
--device
'{"driver"
[libvirt PATCH 18/28] conf: Add return value to virDomainDefPostParseOs()
There are currently no failure scenarios for the function, but
we're about to add one.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3977b5040f..d88d352fb6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4873,7 +4873,7 @@ virDomainDefPostParseMemory(virDomainDef *def,
}
-static void
+static int
virDomainDefPostParseOs(virDomainDef *def)
{
if (def->os.firmwareFeatures &&
@@ -4887,13 +4887,15 @@ virDomainDefPostParseOs(virDomainDef *def)
}
if (!def->os.loader)
-return;
+return 0;
if (def->os.loader->path &&
def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_NONE) {
/* By default, loader is type of 'rom' */
def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_ROM;
}
+
+return 0;
}
@@ -6214,7 +6216,8 @@ virDomainDefPostParseCommon(virDomainDef *def,
if (virDomainDefPostParseMemory(def, data->parseFlags) < 0)
return -1;
-virDomainDefPostParseOs(def);
+if (virDomainDefPostParseOs(def) < 0)
+return -1;
virDomainDefPostParseMemtune(def);
--
2.35.3
[libvirt PATCH 09/28] conf: Move virDomainLoaderDefParseXML()
Pure code movement, needed to prepare for upcoming changes.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 57 +-
1 file changed, 29 insertions(+), 28 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 709ca53790..6f5f370696 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17991,34 +17991,6 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDef
*def)
return 0;
}
-static int
-virDomainLoaderDefParseXML(xmlNodePtr node,
- virDomainLoaderDef *loader,
- bool fwAutoSelect)
-{
-if (!fwAutoSelect) {
-if (virXMLPropTristateBool(node, "readonly", VIR_XML_PROP_NONE,
- >readonly) < 0)
-return -1;
-
-if (virXMLPropEnum(node, "type", virDomainLoaderTypeFromString,
- VIR_XML_PROP_NONZERO, >type) < 0)
-return -1;
-
-if (!(loader->path = virXMLNodeContentString(node)))
-return -1;
-
-if (STREQ(loader->path, ""))
-VIR_FREE(loader->path);
-}
-
-if (virXMLPropTristateBool(node, "secure", VIR_XML_PROP_NONE,
- >secure) < 0)
-return -1;
-
-return 0;
-}
-
static int
virDomainNvramDefParseXML(virDomainLoaderDef *loader,
@@ -18065,6 +18037,35 @@ virDomainNvramDefParseXML(virDomainLoaderDef *loader,
}
+static int
+virDomainLoaderDefParseXML(xmlNodePtr node,
+ virDomainLoaderDef *loader,
+ bool fwAutoSelect)
+{
+if (!fwAutoSelect) {
+if (virXMLPropTristateBool(node, "readonly", VIR_XML_PROP_NONE,
+ >readonly) < 0)
+return -1;
+
+if (virXMLPropEnum(node, "type", virDomainLoaderTypeFromString,
+ VIR_XML_PROP_NONZERO, >type) < 0)
+return -1;
+
+if (!(loader->path = virXMLNodeContentString(node)))
+return -1;
+
+if (STREQ(loader->path, ""))
+VIR_FREE(loader->path);
+}
+
+if (virXMLPropTristateBool(node, "secure", VIR_XML_PROP_NONE,
+ >secure) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
virDomainSchedulerParseCommonAttrs(xmlNodePtr node,
virProcessSchedPolicy *policy,
--
2.35.3
[libvirt PATCH 26/28] qemu_firmware: enrolled-keys requires secure-boot
No sane firmware build will fail this check, but just to be on
the safe side let's check anyway.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 5b2fa51a9c..c477b45d62 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1269,6 +1269,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
size_t i;
bool requiresSMM = false;
bool supportsSecureBoot = false;
+bool hasEnrolledKeys = false;
for (i = 0; i < fw->nfeatures; i++) {
switch (fw->features[i]) {
@@ -1278,12 +1279,14 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
case QEMU_FIRMWARE_FEATURE_SECURE_BOOT:
supportsSecureBoot = true;
break;
+case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS:
+hasEnrolledKeys = true;
+break;
case QEMU_FIRMWARE_FEATURE_NONE:
case QEMU_FIRMWARE_FEATURE_ACPI_S3:
case QEMU_FIRMWARE_FEATURE_ACPI_S4:
case QEMU_FIRMWARE_FEATURE_AMD_SEV:
case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES:
-case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS:
case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC:
case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC:
case QEMU_FIRMWARE_FEATURE_LAST:
@@ -1291,14 +1294,17 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
}
}
-if (supportsSecureBoot != requiresSMM) {
+if ((supportsSecureBoot != requiresSMM) ||
+(hasEnrolledKeys && !supportsSecureBoot)) {
VIR_WARN("Firmware description '%s' has invalid set of features: "
- "%s = %d, %s = %d",
+ "%s = %d, %s = %d, %s = %d",
filename,
qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_REQUIRES_SMM),
requiresSMM,
qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_SECURE_BOOT),
- supportsSecureBoot);
+ supportsSecureBoot,
+
qemuFirmwareFeatureTypeToString(QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS),
+ hasEnrolledKeys);
}
}
--
2.35.3
[libvirt PATCH 28/28] NEWS: Document improvements to firmware autoselection
Signed-off-by: Andrea Bolognani --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 9a92fb4fcb..63e0388e47 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -29,6 +29,11 @@ v8.5.0 (unreleased) * **Improvements** + * conf: Improved firmware autoselection + +The firmware autoselection feature now behaves more intuitively, reports +better error messages on failure and comes with high-level documentation. + * **Bug fixes** -- 2.35.3
[libvirt PATCH 14/28] conf: Rename virDomainLoaderDefParseXML() argument
We're going to start passing multiple nodes to the function in
a moment, so we need a more specific name.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d6a33a0f81..d60985cb85 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18043,29 +18043,29 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
static int
virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
- xmlNodePtr node,
+ xmlNodePtr loaderNode,
xmlXPathContextPtr ctxt,
virDomainXMLOption *xmlopt,
unsigned int flags,
bool fwAutoSelect)
{
if (!fwAutoSelect) {
-if (virXMLPropTristateBool(node, "readonly", VIR_XML_PROP_NONE,
+if (virXMLPropTristateBool(loaderNode, "readonly", VIR_XML_PROP_NONE,
>readonly) < 0)
return -1;
-if (virXMLPropEnum(node, "type", virDomainLoaderTypeFromString,
+if (virXMLPropEnum(loaderNode, "type", virDomainLoaderTypeFromString,
VIR_XML_PROP_NONZERO, >type) < 0)
return -1;
-if (!(loader->path = virXMLNodeContentString(node)))
+if (!(loader->path = virXMLNodeContentString(loaderNode)))
return -1;
if (STREQ(loader->path, ""))
VIR_FREE(loader->path);
}
-if (virXMLPropTristateBool(node, "secure", VIR_XML_PROP_NONE,
+if (virXMLPropTristateBool(loaderNode, "secure", VIR_XML_PROP_NONE,
>secure) < 0)
return -1;
@@ -18467,16 +18467,16 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
virDomainXMLOption *xmlopt,
unsigned int flags)
{
-xmlNodePtr loader_node = virXPathNode("./os/loader[1]", ctxt);
+xmlNodePtr loaderNode = virXPathNode("./os/loader[1]", ctxt);
const bool fwAutoSelect = def->os.firmware !=
VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
-if (!loader_node)
+if (!loaderNode)
return 0;
def->os.loader = g_new0(virDomainLoaderDef, 1);
if (virDomainLoaderDefParseXML(def->os.loader,
- loader_node,
+ loaderNode,
ctxt, xmlopt, flags,
fwAutoSelect) < 0)
return -1;
--
2.35.3
[libvirt PATCH 06/28] tests: Don't set NVRAM path manually
This does the opposite of
commit 392292cd99ee275f986e9e21f325a9fee8e8bbfe
Author: Daniel P. Berrangé
Date: Wed Feb 23 12:45:51 2022 +
tests: don't use auto-generated NVRAM path in tests
in order to minimize input files.
We're going to add a test case specifically covering the use of
custom NVRAM paths with firmware autoselection in an upcoming
commit.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvdata/firmware-auto-bios.xml | 1 -
.../firmware-auto-efi-loader-secure.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.xml | 1 -
tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/firmware-auto-efi.xml| 1 -
tests/qemuxml2xmloutdata/firmware-auto-bios.x86_64-latest.xml | 1 -
.../firmware-auto-efi-loader-secure.x86_64-latest.xml | 1 -
tests/qemuxml2xmloutdata/firmware-auto-efi.x86_64-latest.xml| 1 -
8 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/tests/qemuxml2argvdata/firmware-auto-bios.xml
b/tests/qemuxml2argvdata/firmware-auto-bios.xml
index 0abbddb22e..1318f68243 100644
--- a/tests/qemuxml2argvdata/firmware-auto-bios.xml
+++ b/tests/qemuxml2argvdata/firmware-auto-bios.xml
@@ -6,7 +6,6 @@
hvm
-/some/user/nvram/path/guest_VARS.fd
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.x86_64-latest.args
index 37564db12c..7479b05af4 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.x86_64-latest.args
@@ -12,7 +12,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-fedora/master-key.aes"}'
\
-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.secboot.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
\
--blockdev
'{"driver":"file","filename":"/some/user/nvram/path/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/fedora_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
\
-machine
pc-q35-4.0,usb=off,smm=on,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram
\
-accel kvm \
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.xml
b/tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.xml
index 1b94c25f32..34fa6d090e 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.xml
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-secure.xml
@@ -6,7 +6,6 @@
hvm
-/some/user/nvram/path/guest_VARS.fd
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args
index 51aa5c0303..885c83445d 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-efi.x86_64-latest.args
@@ -12,7 +12,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-fedora/master-key.aes"}'
\
-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.secboot.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
\
--blockdev
'{"driver":"file","filename":"/some/user/nvram/path/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"driver
[libvirt PATCH 08/28] tests: Add more firmware tests
Note that some of these new tests are displaying incorrect or
suboptimal behavior. When we address those in upcoming patches,
this will be highlighted by changes in the test data.
Signed-off-by: Andrea Bolognani
---
...-auto-efi-enrolled-keys.x86_64-latest.args | 35 ++
.../firmware-auto-efi-enrolled-keys.xml | 20 +++
...are-auto-efi-no-secboot.x86_64-latest.args | 35 ++
.../firmware-auto-efi-no-secboot.xml | 20 +++
...firmware-auto-efi-nvram.x86_64-latest.args | 35 ++
.../firmware-auto-efi-nvram.xml | 18 ++
...rmware-auto-efi-secboot.x86_64-latest.args | 35 ++
.../firmware-auto-efi-secboot.xml | 20 +++
tests/qemuxml2argvtest.c | 4 +++
...e-auto-efi-enrolled-keys.x86_64-latest.xml | 36 +++
...ware-auto-efi-no-secboot.x86_64-latest.xml | 36 +++
.../firmware-auto-efi-nvram.x86_64-latest.xml | 33 +
...irmware-auto-efi-secboot.x86_64-latest.xml | 36 +++
tests/qemuxml2xmltest.c | 4 +++
14 files changed, 367 insertions(+)
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-no-secboot.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-no-secboot.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-nvram.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-nvram.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-secboot.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-secboot.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-enrolled-keys.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-no-secboot.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-nvram.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-secboot.x86_64-latest.xml
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.x86_64-latest.args
new file mode 100644
index 00..885c83445d
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.x86_64-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-fedora \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-fedora/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-fedora/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-fedora/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=fedora,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-fedora/master-key.aes"}'
\
+-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.secboot.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
\
+-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/fedora_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
\
+-machine
pc-q35-4.0,usb=off,smm=on,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=pc.ram
\
+-accel kvm \
+-cpu qemu64 \
+-m 8 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":8388608}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.xml
b/tests/qemuxml2argvdata/firmware-auto-efi-enrolled-keys.xml
new file mode 100644
index 00..b6ee05447f
--- /dev/nu
[libvirt PATCH 15/28] conf: Use nodes in virDomainLoaderDefParseXMLNvram()
This makes the function more consistent with
virDomainLoaderDefParseXML() by preferring the virXMLProp
class of functions to XPath access.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 49 --
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d60985cb85..858242b3ae 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17994,43 +17994,45 @@
virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDef *def)
static int
virDomainLoaderDefParseXMLNvram(virDomainLoaderDef *loader,
+xmlNodePtr nvramNode,
+xmlNodePtr nvramSourceNode,
xmlXPathContextPtr ctxt,
virDomainXMLOption *xmlopt,
unsigned int flags,
bool fwAutoSelect)
{
-g_autofree char *nvramType = virXPathString("string(./os/nvram/@type)",
ctxt);
g_autoptr(virStorageSource) src = virStorageSourceNew();
+int typePresent;
+
+if (!nvramNode)
+return 0;
if (!fwAutoSelect)
-loader->nvramTemplate =
virXPathString("string(./os/nvram[1]/@template)", ctxt);
+loader->nvramTemplate = virXMLPropString(nvramNode, "template");
src->format = VIR_STORAGE_FILE_RAW;
-if (!nvramType) {
-char *nvramPath = NULL;
-
-if (!(nvramPath = virXPathString("string(./os/nvram[1])", ctxt)))
-return 0; /* no nvram */
+if ((typePresent = virXMLPropEnum(nvramNode, "type",
+ virStorageTypeFromString,
VIR_XML_PROP_NONE,
+ >type)) < 0)
+return -1;
-src->path = nvramPath;
-src->type = VIR_STORAGE_TYPE_FILE;
-} else {
-xmlNodePtr sourceNode;
+if (!typePresent) {
+g_autofree char *path = NULL;
-if ((src->type = virStorageTypeFromString(nvramType)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown disk type '%s'"), nvramType);
+if (!(path = virXMLNodeContentString(nvramNode)))
return -1;
-}
-if (!(sourceNode = virXPathNode("./os/nvram/source[1]", ctxt))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Missing source element for nvram"));
+if (STREQ(path, ""))
+return 0;
+
+src->type = VIR_STORAGE_TYPE_FILE;
+src->path = g_steal_pointer();
+} else {
+if (!nvramSourceNode)
return -1;
-}
-if (virDomainStorageSourceParse(sourceNode, ctxt, src, flags, xmlopt)
< 0)
+if (virDomainStorageSourceParse(nvramSourceNode, ctxt, src, flags,
xmlopt) < 0)
return -1;
loader->newStyleNVRAM = true;
@@ -18044,6 +18046,8 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
static int
virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
xmlNodePtr loaderNode,
+ xmlNodePtr nvramNode,
+ xmlNodePtr nvramSourceNode,
xmlXPathContextPtr ctxt,
virDomainXMLOption *xmlopt,
unsigned int flags,
@@ -18070,6 +18074,7 @@ virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
return -1;
if (virDomainLoaderDefParseXMLNvram(loader,
+nvramNode, nvramSourceNode,
ctxt, xmlopt, flags,
fwAutoSelect) < 0)
return -1;
@@ -18468,6 +18473,8 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
unsigned int flags)
{
xmlNodePtr loaderNode = virXPathNode("./os/loader[1]", ctxt);
+xmlNodePtr nvramNode = virXPathNode("./os/nvram[1]", ctxt);
+xmlNodePtr nvramSourceNode = virXPathNode("./os/nvram/source[1]", ctxt);
const bool fwAutoSelect = def->os.firmware !=
VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
if (!loaderNode)
@@ -18476,7 +18483,7 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
def->os.loader = g_new0(virDomainLoaderDef, 1);
if (virDomainLoaderDefParseXML(def->os.loader,
- loaderNode,
+ loaderNode, nvramNode, nvramSourceNode,
ctxt, xmlopt, flags,
fwAutoSelect) < 0)
return -1;
--
2.35.3
[libvirt PATCH 21/28] conf: Refactor virDomainDefOSValidate()
This makes it explicit that there are two possible scenarios
(whether or not firmware autoselection is in use) and will make
upcoming changes cleaner to implement.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_validate.c | 28 +++-
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 33b6f47159..1f6c32a816 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1598,21 +1598,23 @@ static int
virDomainDefOSValidate(const virDomainDef *def,
virDomainXMLOption *xmlopt)
{
-if (def->os.firmware &&
-!(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT)) {
-virReportError(VIR_ERR_XML_DETAIL, "%s",
- _("firmware auto selection not implemented for this
driver"));
-return -1;
-}
+virDomainLoaderDef *loader = def->os.loader;
-if (!def->os.loader)
-return 0;
+if (def->os.firmware) {
+if (!(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT))
{
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("firmware auto selection not implemented for this
driver"));
+return -1;
+}
+} else {
+if (!loader)
+return 0;
-if (!def->os.loader->path &&
-def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) {
-virReportError(VIR_ERR_XML_DETAIL, "%s",
- _("no loader path specified and firmware auto selection
disabled"));
-return -1;
+if (!loader->path) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("no loader path specified and firmware auto
selection disabled"));
+return -1;
+}
}
return 0;
--
2.35.3
[libvirt PATCH 10/28] conf: Rename virDomainLoaderDefParseXMLNvram()
The previous name was identical, modulo the case, to the
completely unrelated virDomainNVRAMDefParseXML().
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6f5f370696..1cb162f67c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17993,10 +17993,10 @@
virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDef *def)
static int
-virDomainNvramDefParseXML(virDomainLoaderDef *loader,
- xmlXPathContextPtr ctxt,
- virDomainXMLOption *xmlopt,
- unsigned int flags)
+virDomainLoaderDefParseXMLNvram(virDomainLoaderDef *loader,
+xmlXPathContextPtr ctxt,
+virDomainXMLOption *xmlopt,
+unsigned int flags)
{
g_autofree char *nvramType = virXPathString("string(./os/nvram/@type)",
ctxt);
g_autoptr(virStorageSource) src = virStorageSourceNew();
@@ -18468,7 +18468,7 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
fwAutoSelect) < 0)
return -1;
-if (virDomainNvramDefParseXML(def->os.loader, ctxt, xmlopt, flags) < 0)
+if (virDomainLoaderDefParseXMLNvram(def->os.loader, ctxt, xmlopt, flags) <
0)
return -1;
if (!fwAutoSelect)
--
2.35.3
[libvirt PATCH 04/28] tests: Rename and reorganize firmware tests
Group all tests related to firmware selection together and give
them consistent names that leave room for further tests to be
added in an upcoming commit.
Signed-off-by: Andrea Bolognani
---
tests/qemusecuritytest.c | 6 +--
... => firmware-auto-bios.x86_64-latest.args} | 0
...rmware-bios.xml => firmware-auto-bios.xml} | 0
...ware-auto-efi-aarch64.aarch64-latest.args} | 0
...-efi.xml => firmware-auto-efi-aarch64.xml} | 0
...auto-efi-loader-secure.x86_64-latest.args} | 0
...ml => firmware-auto-efi-loader-secure.xml} | 0
...o-efi-no-enrolled-keys.x86_64-latest.args} | 0
...=> firmware-auto-efi-no-enrolled-keys.xml} | 0
...s => firmware-auto-efi.x86_64-latest.args} | 0
...firmware-efi.xml => firmware-auto-efi.xml} | 0
...anual-bios-rw-implicit.x86_64-latest.args} | 0
...l => firmware-manual-bios-rw-implicit.xml} | 0
...irmware-manual-bios-rw.x86_64-latest.args} | 0
...ram-rw.xml => firmware-manual-bios-rw.xml} | 0
.../{bios.args => firmware-manual-bios.args} | 0
.../{bios.xml => firmware-manual-bios.xml}| 0
... => firmware-manual-efi-acpi-aarch64.args} | 0
...l => firmware-manual-efi-acpi-aarch64.xml} | 0
...args => firmware-manual-efi-acpi-q35.args} | 0
...i.xml => firmware-manual-efi-acpi-q35.xml} | 0
...th.err => firmware-manual-efi-no-path.err} | 0
...th.xml => firmware-manual-efi-no-path.xml} | 0
...> firmware-manual-efi-noacpi-aarch64.args} | 0
...=> firmware-manual-efi-noacpi-aarch64.xml} | 0
...err => firmware-manual-efi-noacpi-q35.err} | 0
...xml => firmware-manual-efi-noacpi-q35.xml} | 0
...-manual-efi-nvram-file.x86_64-latest.args} | 0
...xml => firmware-manual-efi-nvram-file.xml} | 0
...-efi-nvram-network-iscsi.x86_64-4.1.0.err} | 0
...fi-nvram-network-iscsi.x86_64-latest.args} | 0
...rmware-manual-efi-nvram-network-iscsi.xml} | 0
...-efi-nvram-network-nbd.x86_64-latest.args} | 0
...firmware-manual-efi-nvram-network-nbd.xml} | 0
...ual-efi-nvram-template.x86_64-latest.args} | 0
...=> firmware-manual-efi-nvram-template.xml} | 0
...e.args => firmware-manual-efi-secure.args} | 0
...ure.xml => firmware-manual-efi-secure.xml} | 0
...os-nvram.args => firmware-manual-efi.args} | 0
...bios-nvram.xml => firmware-manual-efi.xml} | 0
...=> firmware-manual-noefi-acpi-aarch64.err} | 0
...=> firmware-manual-noefi-acpi-aarch64.xml} | 0
...gs => firmware-manual-noefi-acpi-q35.args} | 0
...xml => firmware-manual-noefi-acpi-q35.xml} | 0
...firmware-manual-noefi-noacpi-aarch64.args} | 0
... firmware-manual-noefi-noacpi-aarch64.xml} | 0
... => firmware-manual-noefi-noacpi-q35.args} | 0
...l => firmware-manual-noefi-noacpi-q35.xml} | 0
tests/qemuxml2argvtest.c | 52 +--
...l => firmware-auto-bios.x86_64-latest.xml} | 0
...mware-auto-efi-aarch64.aarch64-latest.xml} | 0
...-auto-efi-loader-secure.x86_64-latest.xml} | 0
...uto-efi-no-enrolled-keys.x86_64-latest.xml | 1 +
...ml => firmware-auto-efi.x86_64-latest.xml} | 0
...e-manual-efi-nvram-file.x86_64-latest.xml} | 0
...efi-nvram-network-iscsi.x86_64-latest.xml} | 0
...l-efi-nvram-network-nbd.x86_64-latest.xml} | 0
...bios-nvram.xml => firmware-manual-efi.xml} | 0
...are-efi-no-enrolled-keys.x86_64-latest.xml | 1 -
tests/qemuxml2xmltest.c | 20 +++
60 files changed, 39 insertions(+), 41 deletions(-)
rename tests/qemuxml2argvdata/{os-firmware-bios.x86_64-latest.args =>
firmware-auto-bios.x86_64-latest.args} (100%)
rename tests/qemuxml2argvdata/{os-firmware-bios.xml => firmware-auto-bios.xml}
(100%)
rename tests/qemuxml2argvdata/{aarch64-os-firmware-efi.aarch64-latest.args =>
firmware-auto-efi-aarch64.aarch64-latest.args} (100%)
rename tests/qemuxml2argvdata/{aarch64-os-firmware-efi.xml =>
firmware-auto-efi-aarch64.xml} (100%)
rename tests/qemuxml2argvdata/{os-firmware-efi-secboot.x86_64-latest.args =>
firmware-auto-efi-loader-secure.x86_64-latest.args} (100%)
rename tests/qemuxml2argvdata/{os-firmware-efi-secboot.xml =>
firmware-auto-efi-loader-secure.xml} (100%)
rename
tests/qemuxml2argvdata/{os-firmware-efi-no-enrolled-keys.x86_64-latest.args =>
firmware-auto-efi-no-enrolled-keys.x86_64-latest.args} (100%)
rename tests/qemuxml2argvdata/{os-firmware-efi-no-enrolled-keys.xml =>
firmware-auto-efi-no-enrolled-keys.xml} (100%)
rename tests/qemuxml2argvdata/{os-firmware-efi.x86_64-latest.args =>
firmware-auto-efi.x86_64-latest.args} (100%)
rename tests/qemuxml2argvdata/{os-firmware-efi.xml => firmware-auto-efi.xml}
(100%)
rename tests/qemuxml2argvdata/{bios-nvram-rw-implicit.x86_64-latest.args =>
firmware-manual-bios-rw-implicit.x86_64-latest.args} (100%)
rename tests/qemuxml2argvdata/{bios-nvram-rw-implicit.xml =>
firmware-manual-bios-rw-implicit.xml} (100%)
rename tests/qemuxml2argvdata/{bios-nvram-rw
[libvirt PATCH 07/28] tests: Don't use loader.secure=no with firmware autoselection
This currently has not effect whatsoever, so it's just cluttering the input files. We're going to add specific handling for this scenario, as well as a test case covering it, in an upcoming commit. Signed-off-by: Andrea Bolognani --- tests/qemuxml2argvdata/firmware-auto-bios.xml | 1 - tests/qemuxml2argvdata/firmware-auto-efi.xml | 1 - tests/qemuxml2xmloutdata/firmware-auto-bios.x86_64-latest.xml | 1 - tests/qemuxml2xmloutdata/firmware-auto-efi.x86_64-latest.xml | 1 - 4 files changed, 4 deletions(-) diff --git a/tests/qemuxml2argvdata/firmware-auto-bios.xml b/tests/qemuxml2argvdata/firmware-auto-bios.xml index 1318f68243..06bb0bea9d 100644 --- a/tests/qemuxml2argvdata/firmware-auto-bios.xml +++ b/tests/qemuxml2argvdata/firmware-auto-bios.xml @@ -5,7 +5,6 @@ 1 hvm - diff --git a/tests/qemuxml2argvdata/firmware-auto-efi.xml b/tests/qemuxml2argvdata/firmware-auto-efi.xml index 33bd7b0ac1..55b9be1aec 100644 --- a/tests/qemuxml2argvdata/firmware-auto-efi.xml +++ b/tests/qemuxml2argvdata/firmware-auto-efi.xml @@ -5,7 +5,6 @@ 1 hvm - diff --git a/tests/qemuxml2xmloutdata/firmware-auto-bios.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-auto-bios.x86_64-latest.xml index b744234cf1..722294089e 100644 --- a/tests/qemuxml2xmloutdata/firmware-auto-bios.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/firmware-auto-bios.x86_64-latest.xml @@ -6,7 +6,6 @@ 1 hvm - diff --git a/tests/qemuxml2xmloutdata/firmware-auto-efi.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-auto-efi.x86_64-latest.xml index 35d49b7c62..7e2e40036e 100644 --- a/tests/qemuxml2xmloutdata/firmware-auto-efi.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/firmware-auto-efi.x86_64-latest.xml @@ -6,7 +6,6 @@ 1 hvm - -- 2.35.3
[libvirt PATCH 12/28] conf: Move nvramTemplate parsing
It belongs to virDomainLoaderDefParseXMLNvram(), where the other
parts of the element are handled.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f19f6eb63c..fcb468b465 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17996,11 +17996,15 @@ static int
virDomainLoaderDefParseXMLNvram(virDomainLoaderDef *loader,
xmlXPathContextPtr ctxt,
virDomainXMLOption *xmlopt,
-unsigned int flags)
+unsigned int flags,
+bool fwAutoSelect)
{
g_autofree char *nvramType = virXPathString("string(./os/nvram/@type)",
ctxt);
g_autoptr(virStorageSource) src = virStorageSourceNew();
+if (!fwAutoSelect)
+loader->nvramTemplate =
virXPathString("string(./os/nvram[1]/@template)", ctxt);
+
src->format = VIR_STORAGE_FILE_RAW;
if (!nvramType) {
@@ -18468,12 +18472,11 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
fwAutoSelect) < 0)
return -1;
-if (virDomainLoaderDefParseXMLNvram(def->os.loader, ctxt, xmlopt, flags) <
0)
+if (virDomainLoaderDefParseXMLNvram(def->os.loader,
+ctxt, xmlopt, flags,
+fwAutoSelect) < 0)
return -1;
-if (!fwAutoSelect)
-def->os.loader->nvramTemplate =
virXPathString("string(./os/nvram[1]/@template)", ctxt);
-
return 0;
}
--
2.35.3
[libvirt PATCH 03/28] tests: Drop bios-nvram-os-interleave test
This was introduced in
commit 5882064084a733a661759f8f3461f7cbc259175e
Author: Martin Kletzander
Date: Wed Feb 25 15:45:26 2015 +0100
tests: Add test for os interleaving
to ensure a recent change in the schema was behaving correctly.
Seven years later, it no longer seems very useful to keep it
around.
Signed-off-by: Andrea Bolognani
---
.../bios-nvram-os-interleave.xml | 40 --
.../bios-nvram-os-interleave.xml | 52 ---
tests/qemuxml2xmltest.c | 1 -
3 files changed, 93 deletions(-)
delete mode 100644 tests/qemuxml2argvdata/bios-nvram-os-interleave.xml
delete mode 100644 tests/qemuxml2xmloutdata/bios-nvram-os-interleave.xml
diff --git a/tests/qemuxml2argvdata/bios-nvram-os-interleave.xml
b/tests/qemuxml2argvdata/bios-nvram-os-interleave.xml
deleted file mode 100644
index d6c86c661c..00
--- a/tests/qemuxml2argvdata/bios-nvram-os-interleave.xml
+++ /dev/null
@@ -1,40 +0,0 @@
-
- test-bios
- 362d1fc1-df7d-193e-5c18-49a71bd1da66
- 1048576
- 1048576
- 1
-
-/usr/share/OVMF/OVMF_CODE.fd
-/some/user/nvram/path/guest_VARS.fd
-hvm
-
-
-
-
-
-
-
- destroy
- restart
- restart
-
-/usr/bin/qemu-system-x86_64
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/tests/qemuxml2xmloutdata/bios-nvram-os-interleave.xml
b/tests/qemuxml2xmloutdata/bios-nvram-os-interleave.xml
deleted file mode 100644
index 6a40866b0b..00
--- a/tests/qemuxml2xmloutdata/bios-nvram-os-interleave.xml
+++ /dev/null
@@ -1,52 +0,0 @@
-
- test-bios
- 362d1fc1-df7d-193e-5c18-49a71bd1da66
- 1048576
- 1048576
- 1
-
-hvm
-/usr/share/OVMF/OVMF_CODE.fd
-/some/user/nvram/path/guest_VARS.fd
-
-
-
-
-
-
-
- destroy
- restart
- restart
-
-/usr/bin/qemu-system-x86_64
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 4ad6b4dac1..714d36c0a0 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1068,7 +1068,6 @@ mymain(void)
DO_TEST_CAPS_LATEST("numatune-memnode-restrictive-mode");
DO_TEST_NOCAPS("bios-nvram");
-DO_TEST_NOCAPS("bios-nvram-os-interleave");
DO_TEST_CAPS_LATEST("bios-nvram-network-iscsi");
DO_TEST_CAPS_LATEST("bios-nvram-network-nbd");
DO_TEST_CAPS_LATEST("bios-nvram-file");
--
2.35.3
[libvirt PATCH 24/28] conf: Reject features when using manual firmware selection
Signed-off-by: Andrea Bolognani
---
src/conf/domain_validate.c| 7 +++
...ware-manual-efi-features.x86_64-latest.err | 1 +
.../firmware-manual-efi-features.xml | 21 +++
tests/qemuxml2argvtest.c | 1 +
4 files changed, 30 insertions(+)
create mode 100644
tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-manual-efi-features.xml
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 87fdb677d1..4e9cbec5ce 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1655,6 +1655,13 @@ virDomainDefOSValidate(const virDomainDef *def,
return -1;
}
} else {
+if (def->os.firmwareFeatures) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("cannot use feature-based firmware autoselection "
+ "when firmware autoselection is disabled"));
+return -1;
+}
+
if (!loader)
return 0;
diff --git
a/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.err
b/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.err
new file mode 100644
index 00..98412de1e1
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.err
@@ -0,0 +1 @@
+cannot use feature-based firmware autoselection when firmware autoselection is
disabled
diff --git a/tests/qemuxml2argvdata/firmware-manual-efi-features.xml
b/tests/qemuxml2argvdata/firmware-manual-efi-features.xml
new file mode 100644
index 00..69cc71eb2a
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-manual-efi-features.xml
@@ -0,0 +1,21 @@
+
+ test
+ 362d1fc1-df7d-193e-5c18-49a71bd1da66
+ 1048576
+ 1
+
+hvm
+/usr/share/OVMF/OVMF_CODE.fd
+
+
+
+
+
+
+
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 473e00ffa7..4ca1131377 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1192,6 +1192,7 @@ mymain(void)
QEMU_CAPS_DEVICE_ISA_SERIAL);
DO_TEST_NOCAPS("firmware-manual-efi");
DO_TEST_PARSE_ERROR_NOCAPS("firmware-manual-efi-no-path");
+DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-manual-efi-features");
DO_TEST_CAPS_LATEST("firmware-manual-bios-rw");
DO_TEST_CAPS_LATEST("firmware-manual-bios-rw-implicit");
DO_TEST("firmware-manual-efi-secure",
--
2.35.3
[libvirt PATCH 01/28] tests: Remove firmware bits from unrelated tests
The pci-bridge-many-disks test case is not related to firmware handling at all, so we can trim it without losing any coverage. Signed-off-by: Andrea Bolognani --- tests/qemuxml2argvdata/pci-bridge-many-disks.args | 1 - tests/qemuxml2argvdata/pci-bridge-many-disks.xml | 1 - tests/qemuxml2xmloutdata/pci-bridge-many-disks.xml | 1 - 3 files changed, 3 deletions(-) diff --git a/tests/qemuxml2argvdata/pci-bridge-many-disks.args b/tests/qemuxml2argvdata/pci-bridge-many-disks.args index 4c98d395af..9fdb261fb8 100644 --- a/tests/qemuxml2argvdata/pci-bridge-many-disks.args +++ b/tests/qemuxml2argvdata/pci-bridge-many-disks.args @@ -14,7 +14,6 @@ QEMU_AUDIO_DRV=none \ -machine pc,usb=off,dump-guest-core=off \ -accel tcg \ -cpu qemu64,kvmclock=off \ --bios /usr/share/seabios/bios.bin \ -m 3907 \ -overcommit mem-lock=off \ -smp 1,sockets=1,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/pci-bridge-many-disks.xml b/tests/qemuxml2argvdata/pci-bridge-many-disks.xml index 5000574073..1eac85f092 100644 --- a/tests/qemuxml2argvdata/pci-bridge-many-disks.xml +++ b/tests/qemuxml2argvdata/pci-bridge-many-disks.xml @@ -5,7 +5,6 @@ 400 hvm -/usr/share/seabios/bios.bin diff --git a/tests/qemuxml2xmloutdata/pci-bridge-many-disks.xml b/tests/qemuxml2xmloutdata/pci-bridge-many-disks.xml index 9584c81ae9..12caf1fb9c 100644 --- a/tests/qemuxml2xmloutdata/pci-bridge-many-disks.xml +++ b/tests/qemuxml2xmloutdata/pci-bridge-many-disks.xml @@ -6,7 +6,6 @@ 1 hvm -/usr/share/seabios/bios.bin -- 2.35.3
[libvirt PATCH 11/28] conf: Move setting type for NVRAM source
When the 'type' attribute is present we'd end up overwriting
this value via virDomainStorageSourceParse(). Moving this
assignment makes the current code clearer and will also help
with upcoming changes.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1cb162f67c..f19f6eb63c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18001,7 +18001,6 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
g_autofree char *nvramType = virXPathString("string(./os/nvram/@type)",
ctxt);
g_autoptr(virStorageSource) src = virStorageSourceNew();
-src->type = VIR_STORAGE_TYPE_FILE;
src->format = VIR_STORAGE_FILE_RAW;
if (!nvramType) {
@@ -18011,6 +18010,7 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
return 0; /* no nvram */
src->path = nvramPath;
+src->type = VIR_STORAGE_TYPE_FILE;
} else {
xmlNodePtr sourceNode;
--
2.35.3
[libvirt PATCH 02/28] tests: Use firmware autoselection on aarch64
This simplifies the test data without negatively impacting test
coverage.
Signed-off-by: Andrea Bolognani
---
.../virtio-iommu-aarch64.aarch64-latest.args| 2 +-
tests/qemuxml2argvdata/virtio-iommu-aarch64.xml | 6 ++
.../virtio-iommu-aarch64.aarch64-latest.xml | 6 ++
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/tests/qemuxml2argvdata/virtio-iommu-aarch64.aarch64-latest.args
b/tests/qemuxml2argvdata/virtio-iommu-aarch64.aarch64-latest.args
index a9e45ab87f..9c1de8ae9b 100644
--- a/tests/qemuxml2argvdata/virtio-iommu-aarch64.aarch64-latest.args
+++ b/tests/qemuxml2argvdata/virtio-iommu-aarch64.aarch64-latest.args
@@ -14,7 +14,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
\
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
\
--machine
virt,usb=off,dump-guest-core=off,gic-version=2,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=mach-virt.ram
\
+-machine
virt-6.0,usb=off,dump-guest-core=off,gic-version=2,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format,memory-backend=mach-virt.ram
\
-accel tcg \
-cpu cortex-a15 \
-m 1024 \
diff --git a/tests/qemuxml2argvdata/virtio-iommu-aarch64.xml
b/tests/qemuxml2argvdata/virtio-iommu-aarch64.xml
index 3e89cb2dac..8d252bfcf9 100644
--- a/tests/qemuxml2argvdata/virtio-iommu-aarch64.xml
+++ b/tests/qemuxml2argvdata/virtio-iommu-aarch64.xml
@@ -3,10 +3,8 @@
1ccfd97d-5eb4-478a-bbe6-88d254c16db7
1048576
1
-
-hvm
-/usr/share/AAVMF/AAVMF_CODE.fd
-/var/lib/libvirt/qemu/nvram/guest_VARS.fd
+
+hvm
diff --git a/tests/qemuxml2xmloutdata/virtio-iommu-aarch64.aarch64-latest.xml
b/tests/qemuxml2xmloutdata/virtio-iommu-aarch64.aarch64-latest.xml
index c6560e9a91..19b881ce31 100644
--- a/tests/qemuxml2xmloutdata/virtio-iommu-aarch64.aarch64-latest.xml
+++ b/tests/qemuxml2xmloutdata/virtio-iommu-aarch64.aarch64-latest.xml
@@ -4,10 +4,8 @@
1048576
1048576
1
-
-hvm
-/usr/share/AAVMF/AAVMF_CODE.fd
-/var/lib/libvirt/qemu/nvram/guest_VARS.fd
+
+hvm
--
2.35.3
[libvirt PATCH 00/28] Improve firmware autoselection
The main motivation behind this series was making it as simple as
possible ("one click") to enable Secure Boot for a VM.
In the process I ended up fixing, improving and cleaning up various
parts of the firmware selection interface.
GitLab branch: https://gitlab.com/abologna/libvirt/-/commits/firmware
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/571485540
Andrea Bolognani (28):
tests: Remove firmware bits from unrelated tests
tests: Use firmware autoselection on aarch64
tests: Drop bios-nvram-os-interleave test
tests: Rename and reorganize firmware tests
tests: Use minimal hardware for firmware tests
tests: Don't set NVRAM path manually
tests: Don't use loader.secure=no with firmware autoselection
tests: Add more firmware tests
conf: Move virDomainLoaderDefParseXML()
conf: Rename virDomainLoaderDefParseXMLNvram()
conf: Move setting type for NVRAM source
conf: Move nvramTemplate parsing
conf: Handle NVRAM in virDomainLoaderDefParseXML()
conf: Rename virDomainLoaderDefParseXML() argument
conf: Use nodes in virDomainLoaderDefParseXMLNvram()
conf: Always parse NVRAM path if present
conf: Enable secure-boot when enrolled-keys is enabled
conf: Add return value to virDomainDefPostParseOs()
conf: Reject enrolled-keys=yes with secure-boot=no
conf: Always parse all firmware information
conf: Refactor virDomainDefOSValidate()
conf: Validate firmware configuration more thoroughly
conf: Always parse firmware features
conf: Reject features when using manual firmware selection
qemu_firmware: Enable loader.secure when requires-smm
qemu_firmware: enrolled-keys requires secure-boot
docs: Add kbase page for Secure Boot
NEWS: Document improvements to firmware autoselection
NEWS.rst | 5 +
docs/kbase/index.rst | 3 +
docs/kbase/meson.build| 1 +
docs/kbase/secureboot.rst | 102 ++
src/conf/domain_conf.c| 182 ++
src/conf/domain_validate.c| 83 ++--
src/qemu/qemu_firmware.c | 16 +-
tests/qemusecuritytest.c | 6 +-
.../aarch64-os-firmware-efi.xml | 31 ---
.../bios-nvram-os-interleave.xml | 40
.../bios-nvram-rw-implicit.xml| 35
tests/qemuxml2argvdata/bios-nvram-rw.xml | 35
tests/qemuxml2argvdata/bios-nvram-secure.xml | 35
tests/qemuxml2argvdata/bios.xml | 37
...firmware-auto-bios-nvram.x86_64-latest.err | 1 +
.../firmware-auto-bios-nvram.xml | 18 ++
... => firmware-auto-bios.x86_64-latest.args} | 12 +-
tests/qemuxml2argvdata/firmware-auto-bios.xml | 17 ++
...ware-auto-efi-aarch64.aarch64-latest.args} | 6 +-
...uefi.xml => firmware-auto-efi-aarch64.xml} | 12 +-
...enrolled-keys-no-secboot.x86_64-latest.err | 1 +
...ware-auto-efi-enrolled-keys-no-secboot.xml | 21 ++
...auto-efi-enrolled-keys.x86_64-latest.args} | 14 +-
.../firmware-auto-efi-enrolled-keys.xml | 20 ++
...auto-efi-loader-insecure.x86_64-latest.err | 1 +
.../firmware-auto-efi-loader-insecure.xml | 18 ++
...are-auto-efi-loader-path.x86_64-latest.err | 1 +
.../firmware-auto-efi-loader-path.xml | 18 ++
...auto-efi-loader-secure.x86_64-latest.args} | 15 +-
.../firmware-auto-efi-loader-secure.xml | 18 ++
...o-efi-no-enrolled-keys.x86_64-latest.args} | 3 -
.../firmware-auto-efi-no-enrolled-keys.xml| 20 ++
...re-auto-efi-no-secboot.x86_64-latest.args} | 3 -
.../firmware-auto-efi-no-secboot.xml | 20 ++
...irmware-auto-efi-nvram.x86_64-latest.args} | 10 +-
.../firmware-auto-efi-nvram.xml | 18 ++
...mware-auto-efi-secboot.x86_64-latest.args} | 8 +-
.../firmware-auto-efi-secboot.xml | 20 ++
...s => firmware-auto-efi.x86_64-latest.args} | 8 +-
tests/qemuxml2argvdata/firmware-auto-efi.xml | 17 ++
...anual-bios-rw-implicit.x86_64-latest.args} | 8 +-
...l => firmware-manual-bios-rw-implicit.xml} | 7 +-
...irmware-manual-bios-rw.x86_64-latest.args} | 8 +-
...o-path.xml => firmware-manual-bios-rw.xml} | 7 +-
.../{bios.args => firmware-manual-bios.args} | 11 +-
.../qemuxml2argvdata/firmware-manual-bios.xml | 15 ++
... => firmware-manual-efi-acpi-aarch64.args} | 1 -
...l => firmware-manual-efi-acpi-aarch64.xml} | 4 +-
...args => firmware-manual-efi-acpi-q35.args} | 1 -
...i.xml => firmware-manual-efi-acpi-q35.xml} | 4 +-
...ware-manual-efi-features.x86_64-latest.err | 1 +
...e.xml => firmware-manual-efi-features.xml} | 12 +-
...th.err => firmware-manual-efi-no-path.err} | 0
...th.xml => firmware-manual-efi-no-path.xml} | 5 +-
...> firmware-manual-efi-noacpi-aarch64.args} | 1 -
...=> firmware-manual-efi-noacpi-aarch64.xml} | 4 +-
...err => fir
Re: [PATCH] po/LINGUAS: Fix sorting
On Tue, Jun 14, 2022 at 01:52:28PM +0100, Daniel P. Berrangé wrote: > On Tue, Jun 14, 2022 at 02:39:46PM +0200, Peter Krempa wrote: > > Pushed as a build fix. Whether that syntax check is a sensible use of > > CPU cycles is for another discussion. > > po/LINGUAS is created by weblate automatically with no human > interaction, so if we don't accept whatever weblate gives > us we won't be able to merge changes from weblate > > ...except that due to an accident with our CI we've not > blocked on that. > > Our CI is accidentally triggering on both branch pushes > and merge requests, and the merge request trigger doesnt > run the syntax-check job. > > We need to drop that syntax-check rule, and also fix our > CI. I added the syntax-check rule without realizing that the file was automatically generated. I agree that we should drop it. But at least it made us spot an issue in our CI, so it wasn't all for nothing :) -- Andrea Bolognani / Red Hat / Virtualization
[PATCH] qemu: Fix alignment in qemuFirmwareMappingFlashFormat()
Signed-off-by: Andrea Bolognani --- Pushed as trivial. src/qemu/qemu_firmware.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 1dd5c09636..c8f462bfcf 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -751,7 +751,7 @@ qemuFirmwareMappingFlashFormat(virJSONValue *mapping, return -1; if (virJSONValueObjectAppend(mapping, - "nvram-template", + "nvram-template", _template) < 0) return -1; } -- 2.35.3
Re: Some questions regarding firmware handling in the qemu driver
On Tue, Jun 14, 2022 at 03:12:29PM +0200, Gerd Hoffmann wrote: > > I think we need an ARM expert to explain the rules about SecureBoot > > on aarch64. Given SMM doesn't exist outside x86, it may be fine to > > just enable secureboot unconditionally on aarch64 and have it be > > genuinely secure. I simply don't know enough in this respect. > > Unlikely. The firmware needs some way to store state (i.e. uefi > variables) somewhere where the guest OS can't tamper with it. > > On x86 SMM mode provides that. Flash can be configured to be writable > in SMM mode only, so the guest OS can't change things directly but has > to route all update requests through the firmware. > > The arm architecture has a concept called TrustZone which can as far I > know can provide simliar guarantees. Checking upstream edk2 git log > finds me this... > > commit 6b46d77243e02d23ce922803998e01277fe9f399 > Author: Supreeth Venkatesh > Date: Fri Jul 13 23:05:27 2018 +0800 > > StandaloneMmPkg/Core: Implementation of Standalone MM Core Module. > > Management Mode (MM) is a generic term used to describe a secure > execution environment provided by the CPU and related silicon that is > entered when the CPU detects a MMI. For x86 systems, this can be > implemented with System Management Mode (SMM). For ARM systems, this can > be implemented with TrustZone (TZ). > [ ... snip ... ] > > ... but not much beyond that. So I don't think edk2 has support > implemented, and I'm also not sure where we stand with qemu. > > I suspect it'll be quite a bit of work to get everything going. Thanks for the additional information, Gerd! It looks like the only sensible course of action is to keep the current checks in place, effectively preventing the use of secure boot on aarch64, until Someone™ implements TZ support across the stack. -- Andrea Bolognani / Red Hat / Virtualization
Re: Test failures on macOS 12
On Fri, May 06, 2022 at 03:00:14AM -0700, Andrea Bolognani wrote: > I'm trying to enable CI coverage for macOS 12, but I'm running into a > couple of issues that I'm not sure how to handle. > > Note that the test suite currently passes on macOS 11[1], so these > failures have to be a consequence to changes made to macOS that we > haven't yet learned how to cope with. > > The first one is in vircryptotest: > > Encrypt aes265cbc ... Expected ciphertext doesn't match > > I've added some debug statements and it looks like the generated data > is different every time, which seems like a pretty good indication > that virrandommock is not being picked up correctly. This is not the > only test program that uses that specific mock though, so I'm not > sure what makes it fail when all the others are succeeding. > > The other issue is in qemuxml2argvtest: > > error : virCommandWait:2752 : internal error: Child process > (/usr/libexec/qemu/vhost-user/test-vhost-user-gpu --print-capabilities) > unexpected fatal signal 6: dyld[8896]: symbol not found in flat > namespace '_virQEMUCapsGet' > error : qemuVhostUserFillDomainGPU:394 : operation failed: Unable to > find a satisfying vhost-user-gpu > > So the various virFileWrapperAddPrefix() calls that cause the > contents of tests/qemuvhostuserdata/ to override the host's own > vhostuser configuration are still effective, but for some reason the > trivial test-vhost-user-gpu shell script can't be run successfully > because an internal libvirt symbol can't be found somehow? Confusing. > > Roman, does any of this ring a bell? Any chance you could > investigate? macOS 12 has been out for a while now so I'd be very > keen to have it added to CI. Roman, any chance you can find some time to investigate the test failures documented above? I'm afraid I've reached the limit of my ability to debug macOS-specific behavior :( -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 2/3] conf: Fix virDomainDefOSValidate()
Even when the os.loader element is absent, we still have to
validate that the user is not attempting to use firmware
autoselection with a driver that doesn't implement the feature.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_validate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index f3910f08a4..61d4586580 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1586,9 +1586,6 @@ static int
virDomainDefOSValidate(const virDomainDef *def,
virDomainXMLOption *xmlopt)
{
-if (!def->os.loader)
-return 0;
-
if (def->os.firmware &&
!(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT)) {
virReportError(VIR_ERR_XML_DETAIL, "%s",
@@ -1596,6 +1593,9 @@ virDomainDefOSValidate(const virDomainDef *def,
return -1;
}
+if (!def->os.loader)
+return 0;
+
if (!def->os.loader->path &&
def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) {
virReportError(VIR_ERR_XML_DETAIL, "%s",
--
2.35.3
[libvirt PATCH 3/3] qemu: Simplify handling of virTristateBool values
We explicitly check whether the value is YES or NO, which makes
it unnecessary to make sure it's not ABSENT beforehand.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 38 --
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 51223faadf..e459ed9a6c 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1081,31 +1081,25 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
if (def->os.firmwareFeatures) {
reqSecureBoot =
def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_SECURE_BOOT];
-if (reqSecureBoot != VIR_TRISTATE_BOOL_ABSENT) {
-if (reqSecureBoot == VIR_TRISTATE_BOOL_YES && !supportsSecureBoot)
{
-VIR_DEBUG("User requested Secure Boot, firmware '%s' doesn't
support it",
- path);
-return false;
-}
-
-if (reqSecureBoot == VIR_TRISTATE_BOOL_NO && supportsSecureBoot) {
-VIR_DEBUG("User refused Secure Boot, firmware '%s' supports
it", path);
-return false;
-}
+if (reqSecureBoot == VIR_TRISTATE_BOOL_YES && !supportsSecureBoot) {
+VIR_DEBUG("User requested Secure Boot, firmware '%s' doesn't
support it",
+ path);
+return false;
+}
+if (reqSecureBoot == VIR_TRISTATE_BOOL_NO && supportsSecureBoot) {
+VIR_DEBUG("User refused Secure Boot, firmware '%s' supports it",
path);
+return false;
}
reqEnrolledKeys =
def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_ENROLLED_KEYS];
-if (reqEnrolledKeys != VIR_TRISTATE_BOOL_ABSENT) {
-if (reqEnrolledKeys == VIR_TRISTATE_BOOL_YES && !hasEnrolledKeys) {
-VIR_DEBUG("User requested Enrolled keys, firmware '%s' doesn't
have them",
- path);
-return false;
-}
-
-if (reqEnrolledKeys == VIR_TRISTATE_BOOL_NO && hasEnrolledKeys) {
-VIR_DEBUG("User refused Enrolled keys, firmware '%s' has
them", path);
-return false;
-}
+if (reqEnrolledKeys == VIR_TRISTATE_BOOL_YES && !hasEnrolledKeys) {
+VIR_DEBUG("User requested Enrolled keys, firmware '%s' doesn't
have them",
+ path);
+return false;
+}
+if (reqEnrolledKeys == VIR_TRISTATE_BOOL_NO && hasEnrolledKeys) {
+VIR_DEBUG("User refused Enrolled keys, firmware '%s' has them",
path);
+return false;
}
}
--
2.35.3
[libvirt PATCH 0/3] firmware: Small fixes and cleanups
Andrea Bolognani (3): vmx: Declare support for firmware autoselection conf: Fix virDomainDefOSValidate() qemu: Simplify handling of virTristateBool values src/conf/domain_validate.c | 6 +++--- src/qemu/qemu_firmware.c | 38 -- src/vmx/vmx.c | 1 + 3 files changed, 20 insertions(+), 25 deletions(-) -- 2.35.3
[libvirt PATCH 1/3] vmx: Declare support for firmware autoselection
The feature was implemented in commits b4e34d1083bc and
9bb6e4e739fa but the corresponding feature flag was not set in
the driver, so other parts of of libvirt wouldn't be able to
know about it.
Signed-off-by: Andrea Bolognani
---
src/vmx/vmx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index c391caa910..57a1622445 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -639,6 +639,7 @@ static virDomainDefParserConfig virVMXDomainDefParserConfig
= {
.domainPostParseCallback = virVMXDomainDefPostParse,
.features = (VIR_DOMAIN_DEF_FEATURE_WIDE_SCSI |
VIR_DOMAIN_DEF_FEATURE_NAME_SLASH |
+ VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT |
VIR_DOMAIN_DEF_FEATURE_NO_BOOT_ORDER),
.defArch = VIR_ARCH_I686,
};
--
2.35.3
Re: [libvirt PATCH v2 0/7] ci: Drop Fedora 34, add Fedora 36
On Thu, Jun 09, 2022 at 08:11:17AM +0200, Erik Skultety wrote: > On Tue, Jun 07, 2022 at 01:06:41PM +0200, Erik Skultety wrote: > > On Tue, Jun 07, 2022 at 02:34:50AM -0700, Andrea Bolognani wrote: > > > Can I push the remaining two patches now, or are the issues that made > > > it not possible at the time still relevant? > > > > Yes, the problem still persists. > > FYI, once we merge [1] we can merge this one as well. > [1] https://listman.redhat.com/archives/libvir-list/2022-June/232322.html Those are in now. Can you confirm I can push the remaining patches? -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/2] ci: integration: Set 'safe.directory' when installing QEMU from git
On Thu, Jun 09, 2022 at 12:46:38PM +0200, Erik Skultety wrote:
> On Thu, Jun 09, 2022 at 11:07:57AM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jun 09, 2022 at 06:01:34AM -0400, Andrea Bolognani wrote:
> > > On Wed, Jun 08, 2022 at 08:03:07AM +0200, Erik Skultety wrote:
> > > > FWIW we could alternatively update the submodules manually, but we'd
> > > > have list
> > > > them explicitly, IOW:
> > > > $ git clone qemu ...
> > > > $ cd qemu.git
> > > > $ scripts/git-submodule.sh ui/keycodemapdb dtc slirp
> > >
> > > We could avoid hardcoding the names of the submodules by using
> > > something along the lines of
> > >
> > > $ ./scripts/git-submodule.sh update $(git submodule | awk '{print
> > > $2}' | grep -Ev '^(meson|roms/.*|tests/.*)$')
> > >
> > > A bit of a mouthful, but should be solid enough.
> > >
> > > > $ mkdir build && cd build
> > > > $ ../configure ... --with-git-submodules=ignore
> > >
> > > Using
> > >
> > > --with-git-submodules=validate
> > >
> > > would work too, since we'd have updated the submodules beforehand.
> >
> > 'validate' will still cause QEMU to run git commands to check
> > the submodule state, so I presume it'll still hit the problem
> > of ownership.
>
> Yes, only the 'ignore' option value is viable here.
>
> > > I think I would prefer this approach to changing the git
> > > configuration for the root user.
> >
> > I was going to say the opposite. Updating the root user git config
> > is harmless since our integration suite is intended to always run
> > inside a single use throwaway VM. IOW, we already assume the VM is
> > compromised at the end of every test cycle.
>
> Yes, thanks for bringing it up, ^this is the main reason why I didn't propose
> the submodule approach in the first place.
Alrighty,
Reviewed-by: Andrea Bolognani
to the original patch then :)
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/2] ci: integration: Set 'safe.directory' when installing QEMU from git
On Wed, Jun 08, 2022 at 08:03:07AM +0200, Erik Skultety wrote:
> FWIW we could alternatively update the submodules manually, but we'd have list
> them explicitly, IOW:
> $ git clone qemu ...
> $ cd qemu.git
> $ scripts/git-submodule.sh ui/keycodemapdb dtc slirp
We could avoid hardcoding the names of the submodules by using
something along the lines of
$ ./scripts/git-submodule.sh update $(git submodule | awk '{print
$2}' | grep -Ev '^(meson|roms/.*|tests/.*)$')
A bit of a mouthful, but should be solid enough.
> $ mkdir build && cd build
> $ ../configure ... --with-git-submodules=ignore
Using
--with-git-submodules=validate
would work too, since we'd have updated the submodules beforehand.
I think I would prefer this approach to changing the git
configuration for the root user.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 1/2] ci: integration: SELinux relabel the QEMU we installed from git
On Wed, Jun 08, 2022 at 07:59:36AM +0200, Erik Skultety wrote: > Signed-off-by: Erik Skultety > --- > ci/integration-template.yml | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: Some questions regarding firmware handling in the qemu driver
On Tue, Jun 07, 2022 at 02:57:17PM -0600, Jim Fehlig wrote: > Hi All, > > I received a bug report (private, sorry) about inability to "deploy uefi > virtual machine with secureboot enabled on aarch64 kvm host". Indeed the > qemu driver has some checks that would prohibit using secure boot with > aarch64 virt machines, e.g. > > https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_validate.c#L571 > > However it appears qemu does not restrict booting a firmware with keys > enrolled and secure boot enabled. E.g. > > qemu-system-aarch64 -m 4096 -cpu host -accel kvm -smp 4 -M virt -drive > if=pflash,format=raw,readonly=on,file=/usr/share/qemu/aavmf-aarch64-opensuse-code.bin > -drive > if=pflash,format=raw,file=/vm_images/jim/images/test/test-vars-store.bin ... > > seems to work fine and within the guest I see db keys loaded by kernel > > [4.782777] integrity: Loading X.509 certificate: UEFI:db > [4.789494] integrity: Loaded X.509 cert 'Build time autogenerated kernel > key: 44e3470bd0c5eb190e3292dfc42db061521184ee' > [4.789548] integrity: Loading X.509 certificate: UEFI:db > [4.789701] integrity: Loaded X.509 cert 'openSUSE Secure Boot Signkey: > 0332fa9cbf0d88bf21924b0de82a09a54d5defc8' > [4.789710] integrity: Loading X.509 certificate: UEFI:db > [4.789841] integrity: Loaded X.509 cert 'SUSE Linux Enterprise Secure > Boot Signkey: 3fb077b6cebc6ff2522e1c148c57c777c788e3e7' > > Can we consider easing the secure boot restrictions in > qemuValidateDomainDefBoot? Will such a configuration refuse to boot an unsigned guest OS? Is it reasonably tamper-proof (see below)? If the answer to both of these question is yes, then relaxing the check sounds reasonable. > Experimenting with the behavior on x86 raised other questions: > > libvirt requires the firmware to support SMM to enable secure boot. But is > SMM a strict requirement for secure boot? IIUC, lack of SMM makes the > securely booted stack less secure since it is easier to tamper with it, but > it does not prevent securely booting the components. I've been looking into this recently with the help of a colleague who's way more knowledgeable about the topic than I could possibly ever be and the short explanation is that, if you don't have SMM enabled and the loader marked as secure, then the guest OS will be able to modify the chain of trust stored in NVRAM, thus defeating the purpose of enabling secure boot in the first place. I don't think we should relax these requirements, as doing so has the potential to lure users into a false sense of security. > When selecting firmwares manually and marking the loader secure, VM creation > fails unless SMM is explicitly set in . E.g. the following will > fail with "unsupported configuration: Secure boot requires SMM feature > enabled" > > > hvm > type="pflash">/usr/share/qemu/ovmf-x86_64-smm-code.bin > > > > > even though the descriptor file for /usr/share/qemu/ovmf-x86_64-smm-code.bin > advertises secure-boot and requires-smm. Is this just a case of trying to > mix old style explicit firmware selection vs firmware auto-select? I.e., if > selecting the firmware explicitly, the onus is on the user to also specify > any related and required config? The manual firmware selection interface is too complex to be reasonably useful to regular users, so I think it's fair to consider it a development tool / legacy interface at this point. Anyone who's not a libvirt or firmware developer should really use the feature-based firmware selection interface instead. I have a couple of improvements to the automatic firmware selection feature and the corresponding documentation in mind. Hopefully I'll get around to post patches over the next few days. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH v2 0/7] ci: Drop Fedora 34, add Fedora 36
On Thu, May 26, 2022 at 04:24:41PM +0200, Erik Skultety wrote: > On Thu, May 26, 2022 at 04:01:45PM +0200, Andrea Bolognani wrote: > > Test pipeline: > > > > https://gitlab.com/abologna/libvirt/-/pipelines/548848259 > > > > Only patches 1-5 should be pushed until the issues outlined in > > Go ahead and push 1-5 ;). Can I push the remaining two patches now, or are the issues that made it not possible at the time still relevant? -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH] docs: contact: recommend the TLS port for IRC
On Wed, Jun 01, 2022 at 10:05:26AM +0200, Ján Tomko wrote: > Signed-off-by: Ján Tomko > --- > docs/contact.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 0/4] ci: reduce number of jobs in the pipeline
On Fri, May 27, 2022 at 10:06:16AM -0400, Daniel P. Berrangé wrote: > Come June 1st (aka very very soon) GitLab will reduce the > CI minutes quota available from 2000 to 400 for public > projects. > > The wallclock minutes for CI are the quota divided by > a cost factor currently 0.008. IOW, after June 1st > our allowance reduces from 250,000 wallclock minutes > to 50,000. > > While I had intended that we join the OSS program, I'm > unhappy with the financial liability the T require > us to agree to for tha > > Usage stats show we're currently consuming 70-80,000 > minutes a month, so we need to cut our usage by at > least 30%. > > The main libvirt project is responsible for the vast > majority of usage, so is the biggest quick win, but > we need to economise across all our repos. > > With this series, we increase the number of jobs in > the pipeline from 82 to 100, but we mark a great many > of them as manual jobs, so they never run unless a > user explicitly triggers them. > > So we have only 54 jobs executing instead of the > original 82. The remaining 46 jobs are optional. > This is a decent win in usage, but we probably need > to cut a little more later to give us breathing space > > Daniel P. Berrangé (4): > ci: refresh with lcitool manifest > ci: disable native builds on certain distros > ci: move Ubuntu GCC santizers build to 20.04 > ci: eliminate many cross arch CI builds I'm not entirely convinced this is the exact set of trade-offs we want, but the new limit is going to be active starting tomorrow so it's probably a good idea to merge this right away and tweak things further later, so Reviewed-by: Andrea Bolognani Erik, are you okay with this plan? If so, we can push the patches. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH v2 5/7] ci: Don't mark any Fedora 36 job as optional
On Thu, May 26, 2022 at 04:23:44PM +0200, Erik Skultety wrote:
> On Thu, May 26, 2022 at 04:01:50PM +0200, Andrea Bolognani wrote:
> > +++ b/ci/manifest.yml
> > @@ -157,7 +157,6 @@ targets:
> >- arch: x86_64
> >
> >- arch: mingw32
> > -allow-failure: true
>
> There must be a hysterical raisin for this (not that I object to the patch),
> maybe there was a bug in mingw toolchain giving us a hard time? I don't think
> it has anything to do with the given OS's stability as the commit message
> suggest, but if you don't remember and don't feel like digging, sure you can
> keep the commit message as is.
I think it was simply carried over by mistake when the MinGW jobs
were added to stable Fedora after only having been executed on
Rawhide (where 'allow-failure: true' makes perfect sense) up until
that point.
The relevant commit is
commit c7edcb320be3ae6cfa3230f4d3b2c867db49a613
Author: Daniel P. Berrangé
Date: Tue Nov 23 12:12:25 2021 +
ci: run a mingw64 job on stable Fedora
Both of the current mingw jobs are marked as 'allow_failure' because
they are running against Fedora rawhide which is an unstable distro.
We need at least one mingw job to be gating to more reliably detect
problems.
This introduces dockerfiles for both mingw variants on Fedora 35
and sets the mingw64 build to run on Fedora 34, and mingw32 on
Fedora rawhide.
Reviewed-by: Michal Privoznik
Signed-off-by: Daniel P. Berrangé
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 0/4] ci: Drop Fedora 34, add Fedora 36
On Thu, May 26, 2022 at 03:07:52PM +0200, Erik Skultety wrote: > On Thu, May 26, 2022 at 05:15:28AM -0700, Andrea Bolognani wrote: > > On Thu, May 26, 2022 at 12:05:23PM +0200, Erik Skultety wrote: > > > Please don't push this yet, because the integration tests would fail (as > > > you > > > didn't didn't run them in your pipeline) > > > > I didn't think I would be able to. Can anyone trigger a run of the > > integration tests? Can group members? > > Any member can trigger the scheduled pipeline in the main repo, but you'd have > to set up your own runner for your fork to do it. So Technically™ I could run it, but in practice I couldn't :) > Yeah, usually I can do it within minutes, but the pipeline failed [1]. I think > in the case of F35 there may be one more problem with your patches Yup, I changed the Fedora 35 build job so that it would no longer publish RPMs as artifacts, and that obviously broke the integration job. > Andrea, I take it you'll repost your series without the F36 integration > changes > so that I can ACK separately, right? https://listman.redhat.com/archives/libvir-list/2022-May/231856.html :) -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH v2 6/7] ci: Add Fedora 36 to integration tests
This requires publishing the RPMs as artifacts from the regular build job. Signed-off-by: Andrea Bolognani --- ci/gitlab/builds.yml | 4 ci/integration.yml | 16 ci/manifest.yml | 4 3 files changed, 24 insertions(+) diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml index d136b0776e..5fda5be9fe 100644 --- a/ci/gitlab/builds.yml +++ b/ci/gitlab/builds.yml @@ -150,6 +150,10 @@ x86_64-fedora-36: allow_failure: false variables: NAME: fedora-36 + artifacts: +expire_in: 1 day +paths: + - libvirt-rpms x86_64-fedora-rawhide: diff --git a/ci/integration.yml b/ci/integration.yml index ecaa03fc3a..5b60b41e85 100644 --- a/ci/integration.yml +++ b/ci/integration.yml @@ -64,3 +64,19 @@ fedora-35-upstream-qemu-tests: LIBVIRT_CI_INTEGRATION_RUNNER_TAG: redhat-vm-host tags: - $LIBVIRT_CI_INTEGRATION_RUNNER_TAG + +fedora-36-tests: + extends: .integration_tests + needs: +- x86_64-fedora-36 +- project: libvirt/libvirt-perl + job: x86_64-fedora-36 + ref: master + artifacts: true + variables: +# needed by libvirt-gitlab-executor +DISTRO: fedora-36 +# can be overridden in forks to set a different runner tag +LIBVIRT_CI_INTEGRATION_RUNNER_TAG: redhat-vm-host + tags: +- $LIBVIRT_CI_INTEGRATION_RUNNER_TAG diff --git a/ci/manifest.yml b/ci/manifest.yml index ad0ec9a53d..bb7af19de4 100644 --- a/ci/manifest.yml +++ b/ci/manifest.yml @@ -155,6 +155,10 @@ targets: fedora-36: jobs: - arch: x86_64 +artifacts: + expire_in: 1 day + paths: +- libvirt-rpms - arch: mingw32 builds: false -- 2.35.3
[libvirt PATCH v2 4/7] ci: Move MinGW jobs to Fedora 36
Signed-off-by: Andrea Bolognani
---
...gw32.Dockerfile => fedora-36-cross-mingw32.Dockerfile} | 2 +-
...gw64.Dockerfile => fedora-36-cross-mingw64.Dockerfile} | 2 +-
ci/gitlab/builds.yml | 6 +++---
ci/gitlab/containers.yml | 8
ci/manifest.yml | 6 --
5 files changed, 13 insertions(+), 11 deletions(-)
rename ci/containers/{fedora-35-cross-mingw32.Dockerfile =>
fedora-36-cross-mingw32.Dockerfile} (98%)
rename ci/containers/{fedora-35-cross-mingw64.Dockerfile =>
fedora-36-cross-mingw64.Dockerfile} (98%)
diff --git a/ci/containers/fedora-35-cross-mingw32.Dockerfile
b/ci/containers/fedora-36-cross-mingw32.Dockerfile
similarity index 98%
rename from ci/containers/fedora-35-cross-mingw32.Dockerfile
rename to ci/containers/fedora-36-cross-mingw32.Dockerfile
index 3cc5e7b59b..76659d696a 100644
--- a/ci/containers/fedora-35-cross-mingw32.Dockerfile
+++ b/ci/containers/fedora-36-cross-mingw32.Dockerfile
@@ -4,7 +4,7 @@
#
# https://gitlab.com/libvirt/libvirt-ci
-FROM registry.fedoraproject.org/fedora:35
+FROM registry.fedoraproject.org/fedora:36
RUN dnf install -y nosync && \
echo -e '#!/bin/sh\n\
diff --git a/ci/containers/fedora-35-cross-mingw64.Dockerfile
b/ci/containers/fedora-36-cross-mingw64.Dockerfile
similarity index 98%
rename from ci/containers/fedora-35-cross-mingw64.Dockerfile
rename to ci/containers/fedora-36-cross-mingw64.Dockerfile
index b6662f6da7..0bccf0738c 100644
--- a/ci/containers/fedora-35-cross-mingw64.Dockerfile
+++ b/ci/containers/fedora-36-cross-mingw64.Dockerfile
@@ -4,7 +4,7 @@
#
# https://gitlab.com/libvirt/libvirt-ci
-FROM registry.fedoraproject.org/fedora:35
+FROM registry.fedoraproject.org/fedora:36
RUN dnf install -y nosync && \
echo -e '#!/bin/sh\n\
diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml
index b55143018b..d136b0776e 100644
--- a/ci/gitlab/builds.yml
+++ b/ci/gitlab/builds.yml
@@ -334,15 +334,15 @@ s390x-debian-sid:
NAME: debian-sid
-mingw64-fedora-35:
+mingw64-fedora-36:
extends: .cross_build_job
needs:
-- job: mingw64-fedora-35-container
+- job: mingw64-fedora-36-container
optional: true
allow_failure: false
variables:
CROSS: mingw64
-NAME: fedora-35
+NAME: fedora-36
mingw32-fedora-rawhide:
diff --git a/ci/gitlab/containers.yml b/ci/gitlab/containers.yml
index 635bc516e3..5b8a91c295 100644
--- a/ci/gitlab/containers.yml
+++ b/ci/gitlab/containers.yml
@@ -297,18 +297,18 @@ s390x-debian-sid-container:
NAME: debian-sid-cross-s390x
-mingw32-fedora-35-container:
+mingw32-fedora-36-container:
extends: .container_job
allow_failure: true
variables:
-NAME: fedora-35-cross-mingw32
+NAME: fedora-36-cross-mingw32
-mingw64-fedora-35-container:
+mingw64-fedora-36-container:
extends: .container_job
allow_failure: false
variables:
-NAME: fedora-35-cross-mingw64
+NAME: fedora-36-cross-mingw64
mingw32-fedora-rawhide-container:
diff --git a/ci/manifest.yml b/ci/manifest.yml
index 8c43d2e917..e85a01018e 100644
--- a/ci/manifest.yml
+++ b/ci/manifest.yml
@@ -152,14 +152,16 @@ targets:
paths:
- libvirt-rpms
+ fedora-36:
+jobs:
+ - arch: x86_64
+
- arch: mingw32
allow-failure: true
builds: false
- arch: mingw64
- fedora-36: x86_64
-
fedora-rawhide:
jobs:
- arch: x86_64
--
2.35.3
[libvirt PATCH v2 7/7] ci: Move upstream QEMU integration test to Fedora 36
Signed-off-by: Andrea Bolognani --- ci/integration.yml | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ci/integration.yml b/ci/integration.yml index 5b60b41e85..43f22813c9 100644 --- a/ci/integration.yml +++ b/ci/integration.yml @@ -49,24 +49,24 @@ fedora-35-tests: tags: - $LIBVIRT_CI_INTEGRATION_RUNNER_TAG -fedora-35-upstream-qemu-tests: - extends: .integration_tests_upstream_qemu +fedora-36-tests: + extends: .integration_tests needs: -- x86_64-fedora-35 +- x86_64-fedora-36 - project: libvirt/libvirt-perl - job: x86_64-fedora-35 + job: x86_64-fedora-36 ref: master artifacts: true variables: # needed by libvirt-gitlab-executor -DISTRO: fedora-35 +DISTRO: fedora-36 # can be overridden in forks to set a different runner tag LIBVIRT_CI_INTEGRATION_RUNNER_TAG: redhat-vm-host tags: - $LIBVIRT_CI_INTEGRATION_RUNNER_TAG -fedora-36-tests: - extends: .integration_tests +fedora-36-upstream-qemu-tests: + extends: .integration_tests_upstream_qemu needs: - x86_64-fedora-36 - project: libvirt/libvirt-perl -- 2.35.3
[libvirt PATCH v2 1/7] ci: Drop Fedora 34
Signed-off-by: Andrea Bolognani Reviewed-by: Erik Skultety --- ci/containers/fedora-34.Dockerfile | 109 - ci/gitlab.yml | 20 -- ci/integration.yml | 16 - ci/manifest.yml| 8 --- 4 files changed, 153 deletions(-) delete mode 100644 ci/containers/fedora-34.Dockerfile diff --git a/ci/containers/fedora-34.Dockerfile b/ci/containers/fedora-34.Dockerfile deleted file mode 100644 index 6949231a62..00 --- a/ci/containers/fedora-34.Dockerfile +++ /dev/null @@ -1,109 +0,0 @@ -# THIS FILE WAS AUTO-GENERATED -# -# $ lcitool manifest ci/manifest.yml -# -# https://gitlab.com/libvirt/libvirt-ci - -FROM registry.fedoraproject.org/fedora:34 - -RUN dnf install -y nosync && \ -echo -e '#!/bin/sh\n\ -if test -d /usr/lib64\n\ -then\n\ -export LD_PRELOAD=/usr/lib64/nosync/nosync.so\n\ -else\n\ -export LD_PRELOAD=/usr/lib/nosync/nosync.so\n\ -fi\n\ -exec "$@"' > /usr/bin/nosync && \ -chmod +x /usr/bin/nosync && \ -nosync dnf update -y && \ -nosync dnf install -y \ -audit-libs-devel \ -augeas \ -bash-completion \ -ca-certificates \ -ccache \ -clang \ -codespell \ -cpp \ -cppi \ -cyrus-sasl-devel \ -device-mapper-devel \ -diffutils \ -dwarves \ -ebtables \ -firewalld-filesystem \ -fuse-devel \ -gcc \ -gettext \ -git \ -glib2-devel \ -glibc-devel \ -glibc-langpack-en \ -glusterfs-api-devel \ -gnutls-devel \ -grep \ -iproute \ -iproute-tc \ -iptables \ -iscsi-initiator-utils \ -kmod \ -libacl-devel \ -libattr-devel \ -libblkid-devel \ -libcap-ng-devel \ -libcurl-devel \ -libiscsi-devel \ -libnl3-devel \ -libpcap-devel \ -libpciaccess-devel \ -librbd-devel \ -libselinux-devel \ -libssh-devel \ -libssh2-devel \ -libtirpc-devel \ -libwsman-devel \ -libxml2 \ -libxml2-devel \ -libxslt \ -lvm2 \ -make \ -meson \ -netcf-devel \ -nfs-utils \ -ninja-build \ -numactl-devel \ -numad \ -parted-devel \ -perl-base \ -pkgconfig \ -polkit \ -python3 \ -python3-docutils \ -python3-flake8 \ -qemu-img \ -readline-devel \ -rpcgen \ -rpm-build \ -sanlock-devel \ -scrub \ -sed \ -sheepdog \ -systemd-devel \ -systemtap-sdt-devel \ -wireshark-devel \ -xen-devel \ -yajl-devel && \ -nosync dnf autoremove -y && \ -nosync dnf clean all -y && \ -rpm -qa | sort > /packages.txt && \ -mkdir -p /usr/libexec/ccache-wrappers && \ -ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \ -ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/clang && \ -ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc - -ENV LANG "en_US.UTF-8" -ENV MAKE "/usr/bin/make" -ENV NINJA "/usr/bin/ninja" -ENV PYTHON "/usr/bin/python3" -ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" diff --git a/ci/gitlab.yml b/ci/gitlab.yml index 07ad36809d..f4e3f189c0 100644 --- a/ci/gitlab.yml +++ b/ci/gitlab.yml @@ -142,13 +142,6 @@ x86_64-debian-sid-container: NAME: debian-sid -x86_64-fedora-34-container: - extends: .container_job - allow_failure: false - variables: -NAME: fedora-34 - - x86_64-fedora-35-container: extends: .container_job allow_failure: false @@ -509,19 +502,6 @@ x86_64-debian-sid: NAME: debian-sid -x86_64-fedora-34: - extends: .native_build_job - needs: -- x86_64-fedora-34-container - allow_failure: false - variables: -NAME: fedora-34 - artifacts: -expire_in: 1 day -paths: - - libvirt-rpms - - x86_64-fedora-35: extends: .native_build_job needs: diff --git a/ci/integration.yml b/ci/integration.yml index 4978d3cf02..ecaa03fc3a 100644 --- a/ci/integration.yml +++ b/ci/integration.yml @@ -33,22 +33,6 @@ centos-stream-9-tests: tags: - $LIBVIRT_CI_INTEGRATION_RUNNER_TAG -fedora-34-tests: - extends: .integration_tests - needs: -- x86_64-fedora-34 -- project: libvirt/libvirt-perl - job: x86_64-fedora-34 - ref: master - artifacts: true - variables: -# needed by libvirt-gitlab-executor -DISTRO: fedora-34 -# can be overridden in forks to set a different runner tag -LIBVIRT_CI_INTEGRATION_RUNNER_TAG: redhat-vm-host - tags: -- $LIBVIRT_CI_INTEGRATION_RUNNER_TAG - fedora-35-tests: extends: .integration_tests needs: diff --git a/ci/manifes
[libvirt PATCH v2 5/7] ci: Don't mark any Fedora 36 job as optional
It's a stable distro, so we expect all jobs to succeed. Signed-off-by: Andrea Bolognani --- ci/gitlab/containers.yml | 2 +- ci/manifest.yml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ci/gitlab/containers.yml b/ci/gitlab/containers.yml index 5b8a91c295..37c179c439 100644 --- a/ci/gitlab/containers.yml +++ b/ci/gitlab/containers.yml @@ -299,7 +299,7 @@ s390x-debian-sid-container: mingw32-fedora-36-container: extends: .container_job - allow_failure: true + allow_failure: false variables: NAME: fedora-36-cross-mingw32 diff --git a/ci/manifest.yml b/ci/manifest.yml index e85a01018e..ad0ec9a53d 100644 --- a/ci/manifest.yml +++ b/ci/manifest.yml @@ -157,7 +157,6 @@ targets: - arch: x86_64 - arch: mingw32 -allow-failure: true builds: false - arch: mingw64 -- 2.35.3
[libvirt PATCH v2 2/7] ci: Refresh generated files
Notable changes:
* 'lcitool manifest' now generates GitLab CI rules spread
across a bunch of files;
* container images are built less frequently for the main
repository.
Signed-off-by: Andrea Bolognani
Reviewed-by: Erik Skultety
---
ci/gitlab.yml | 747 +-
ci/gitlab/build-templates.yml | 45 ++
ci/gitlab/builds.yml | 392
ci/gitlab/container-templates.yml | 52 +++
ci/gitlab/containers.yml | 318 +
ci/gitlab/sanity-checks.yml | 18 +
6 files changed, 831 insertions(+), 741 deletions(-)
create mode 100644 ci/gitlab/build-templates.yml
create mode 100644 ci/gitlab/builds.yml
create mode 100644 ci/gitlab/container-templates.yml
create mode 100644 ci/gitlab/containers.yml
create mode 100644 ci/gitlab/sanity-checks.yml
diff --git a/ci/gitlab.yml b/ci/gitlab.yml
index f4e3f189c0..379a4c 100644
--- a/ci/gitlab.yml
+++ b/ci/gitlab.yml
@@ -4,744 +4,9 @@
#
# https://gitlab.com/libvirt/libvirt-ci
-
-.container_job:
- image: docker:stable
- stage: containers
- needs: []
- services:
-- docker:dind
- before_script:
-- export TAG="$CI_REGISTRY_IMAGE/ci-$NAME:latest"
-- export COMMON_TAG="$CI_REGISTRY/libvirt/libvirt/ci-$NAME:latest"
-- docker info
-- docker login registry.gitlab.com -u "$CI_REGISTRY_USER" -p
"$CI_REGISTRY_PASSWORD"
- script:
-- docker pull "$TAG" || docker pull "$COMMON_TAG" || true
-- docker build --cache-from "$TAG" --cache-from "$COMMON_TAG" --tag "$TAG"
-f "ci/containers/$NAME.Dockerfile" ci/containers
-- docker push "$TAG"
- after_script:
-- docker logout
-
-
-.gitlab_native_build_job:
- image: $CI_REGISTRY_IMAGE/ci-$NAME:latest
- stage: builds
-
-
-.gitlab_cross_build_job:
- image: $CI_REGISTRY_IMAGE/ci-$NAME-cross-$CROSS:latest
- stage: builds
-
-
-.cirrus_build_job:
- stage: builds
- image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master
- needs: []
- script:
-- source ci/cirrus/$NAME.vars
-- sed -e "s|[@]CI_REPOSITORY_URL@|$CI_REPOSITORY_URL|g"
- -e "s|[@]CI_COMMIT_REF_NAME@|$CI_COMMIT_REF_NAME|g"
- -e "s|[@]CI_COMMIT_SHA@|$CI_COMMIT_SHA|g"
- -e "s|[@]CIRRUS_VM_INSTANCE_TYPE@|$CIRRUS_VM_INSTANCE_TYPE|g"
- -e "s|[@]CIRRUS_VM_IMAGE_SELECTOR@|$CIRRUS_VM_IMAGE_SELECTOR|g"
- -e "s|[@]CIRRUS_VM_IMAGE_NAME@|$CIRRUS_VM_IMAGE_NAME|g"
- -e "s|[@]UPDATE_COMMAND@|$UPDATE_COMMAND|g"
- -e "s|[@]UPGRADE_COMMAND@|$UPGRADE_COMMAND|g"
- -e "s|[@]INSTALL_COMMAND@|$INSTALL_COMMAND|g"
- -e "s|[@]PATH@|$PATH_EXTRA${PATH_EXTRA:+:}\$PATH|g"
- -e "s|[@]PKG_CONFIG_PATH@|$PKG_CONFIG_PATH|g"
- -e "s|[@]PKGS@|$PKGS|g"
- -e "s|[@]MAKE@|$MAKE|g"
- -e "s|[@]PYTHON@|$PYTHON|g"
- -e "s|[@]PIP3@|$PIP3|g"
- -e "s|[@]PYPI_PKGS@|$PYPI_PKGS|g"
- -e "s|[@]XML_CATALOG_FILES@|$XML_CATALOG_FILES|g"
- ci/cirrus/$NAME.yml
-- cat ci/cirrus/$NAME.yml
-- cirrus-run -v --show-build-log always ci/cirrus/$NAME.yml
- rules:
-- if: "$CIRRUS_GITHUB_REPO && $CIRRUS_API_TOKEN"
-
-
-check-dco:
- stage: sanity_checks
- needs: []
- image: registry.gitlab.com/libvirt/libvirt-ci/check-dco:master
- script:
-- /check-dco libvirt
- except:
-variables:
- - $CI_PROJECT_NAMESPACE == 'libvirt'
- variables:
-GIT_DEPTH: 1000
-
-
-# Native container jobs
-
-x86_64-almalinux-8-container:
- extends: .container_job
- allow_failure: false
- variables:
-NAME: almalinux-8
-
-
-x86_64-alpine-314-container:
- extends: .container_job
- allow_failure: false
- variables:
-NAME: alpine-314
-
-
-x86_64-alpine-315-container:
- extends: .container_job
- allow_failure: false
- variables:
-NAME: alpine-315
-
-
-x86_64-alpine-edge-container:
- extends: .container_job
- allow_failure: true
- variables:
-NAME: alpine-edge
-
-
-x86_64-centos-stream-8-container:
- extends: .container_job
- allow_failure: false
- variables:
-NAME: centos-stream-8
-
-
-x86_64-centos-stream-9-container:
- extends: .container_job
- allow_failure: false
- variables:
-NAME: centos-stream-9
-
-
-x86_64-debian-10-container:
- extends: .container_job
- allow_failure: false
- variables:
-NAME: debian-10
-
-
-x86_64-debian-11-container:
- extends: .container_job
- allow_failure: false
- variables:
-NAME: debian-11
-
-
-x86_64-debian-sid-container:
- extends: .container_job
- allow_failure: true
- variables:
-NAME: debian-sid
-
-
-x86_64-fedora-35-container:
- extends: .container_job
- allow_failure: false
- variables:
-NAME: fedora-35
-
-
-x86_64-fedora
[libvirt PATCH v2 3/7] ci: Add Fedora 36
The target is intentionally not added to the integration tests at this time, because the corresponding VM template is not yet available on the runner. A later patch will take care of that. Signed-off-by: Andrea Bolognani --- ci/containers/fedora-36.Dockerfile | 108 + ci/gitlab/builds.yml | 10 +++ ci/gitlab/containers.yml | 7 ++ ci/manifest.yml| 2 + 4 files changed, 127 insertions(+) create mode 100644 ci/containers/fedora-36.Dockerfile diff --git a/ci/containers/fedora-36.Dockerfile b/ci/containers/fedora-36.Dockerfile new file mode 100644 index 00..f99ba502c5 --- /dev/null +++ b/ci/containers/fedora-36.Dockerfile @@ -0,0 +1,108 @@ +# THIS FILE WAS AUTO-GENERATED +# +# $ lcitool manifest ci/manifest.yml +# +# https://gitlab.com/libvirt/libvirt-ci + +FROM registry.fedoraproject.org/fedora:36 + +RUN dnf install -y nosync && \ +echo -e '#!/bin/sh\n\ +if test -d /usr/lib64\n\ +then\n\ +export LD_PRELOAD=/usr/lib64/nosync/nosync.so\n\ +else\n\ +export LD_PRELOAD=/usr/lib/nosync/nosync.so\n\ +fi\n\ +exec "$@"' > /usr/bin/nosync && \ +chmod +x /usr/bin/nosync && \ +nosync dnf update -y && \ +nosync dnf install -y \ +audit-libs-devel \ +augeas \ +bash-completion \ +ca-certificates \ +ccache \ +clang \ +codespell \ +cpp \ +cppi \ +cyrus-sasl-devel \ +device-mapper-devel \ +diffutils \ +dwarves \ +ebtables \ +firewalld-filesystem \ +fuse-devel \ +gcc \ +gettext \ +git \ +glib2-devel \ +glibc-devel \ +glibc-langpack-en \ +glusterfs-api-devel \ +gnutls-devel \ +grep \ +iproute \ +iproute-tc \ +iptables \ +iscsi-initiator-utils \ +kmod \ +libacl-devel \ +libattr-devel \ +libblkid-devel \ +libcap-ng-devel \ +libcurl-devel \ +libiscsi-devel \ +libnl3-devel \ +libpcap-devel \ +libpciaccess-devel \ +librbd-devel \ +libselinux-devel \ +libssh-devel \ +libssh2-devel \ +libtirpc-devel \ +libwsman-devel \ +libxml2 \ +libxml2-devel \ +libxslt \ +lvm2 \ +make \ +meson \ +nfs-utils \ +ninja-build \ +numactl-devel \ +numad \ +parted-devel \ +perl-base \ +pkgconfig \ +polkit \ +python3 \ +python3-docutils \ +python3-flake8 \ +qemu-img \ +readline-devel \ +rpcgen \ +rpm-build \ +sanlock-devel \ +scrub \ +sed \ +sheepdog \ +systemd-devel \ +systemtap-sdt-devel \ +wireshark-devel \ +xen-devel \ +yajl-devel && \ +nosync dnf autoremove -y && \ +nosync dnf clean all -y && \ +rpm -qa | sort > /packages.txt && \ +mkdir -p /usr/libexec/ccache-wrappers && \ +ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \ +ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/clang && \ +ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc + +ENV LANG "en_US.UTF-8" +ENV MAKE "/usr/bin/make" +ENV NINJA "/usr/bin/ninja" +ENV PYTHON "/usr/bin/python3" +ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" diff --git a/ci/gitlab/builds.yml b/ci/gitlab/builds.yml index 8d8ac7bc8a..b55143018b 100644 --- a/ci/gitlab/builds.yml +++ b/ci/gitlab/builds.yml @@ -142,6 +142,16 @@ x86_64-fedora-35: - libvirt-rpms +x86_64-fedora-36: + extends: .native_build_job + needs: +- job: x86_64-fedora-36-container + optional: true + allow_failure: false + variables: +NAME: fedora-36 + + x86_64-fedora-rawhide: extends: .native_build_job needs: diff --git a/ci/gitlab/containers.yml b/ci/gitlab/containers.yml index 12ab6e4808..635bc516e3 100644 --- a/ci/gitlab/containers.yml +++ b/ci/gitlab/containers.yml @@ -77,6 +77,13 @@ x86_64-fedora-35-container: NAME: fedora-35 +x86_64-fedora-36-container: + extends: .container_job + allow_failure: false + variables: +NAME: fedora-36 + + x86_64-fedora-rawhide-container: extends: .container_job allow_failure: true diff --git a/ci/manifest.yml b/ci/manifest.yml index 47bed03130..8c43d2e917 100644 --- a/ci/manifest.yml +++ b/ci/manifest.yml @@ -158,6 +158,8 @@ targets: - arch: mingw64 + fedora-36: x86_64 + fedora-rawhide: jobs: - arch: x86_64 -- 2.35.3
[libvirt PATCH v2 0/7] ci: Drop Fedora 34, add Fedora 36
Test pipeline:
https://gitlab.com/abologna/libvirt/-/pipelines/548848259
Only patches 1-5 should be pushed until the issues outlined in
https://listman.redhat.com/archives/libvir-list/2022-May/231851.html
have been addressed.
Changes from [v1]
* handle integration tests separately;
* don't stop publishing RPMs from the Fedora 35 job
[v1] https://listman.redhat.com/archives/libvir-list/2022-May/231838.html
Andrea Bolognani (7):
ci: Drop Fedora 34
ci: Refresh generated files
ci: Add Fedora 36
ci: Move MinGW jobs to Fedora 36
ci: Don't mark any Fedora 36 job as optional
ci: Add Fedora 36 to integration tests
ci: Move upstream QEMU integration test to Fedora 36
...ile => fedora-36-cross-mingw32.Dockerfile} | 2 +-
...ile => fedora-36-cross-mingw64.Dockerfile} | 2 +-
...ora-34.Dockerfile => fedora-36.Dockerfile} | 3 +-
ci/gitlab.yml | 767 +-
ci/gitlab/build-templates.yml | 45 +
ci/gitlab/builds.yml | 406 +
ci/gitlab/container-templates.yml | 52 ++
ci/gitlab/containers.yml | 325
ci/gitlab/sanity-checks.yml | 18 +
ci/integration.yml| 24 +-
ci/manifest.yml | 5 +-
11 files changed, 869 insertions(+), 780 deletions(-)
rename ci/containers/{fedora-35-cross-mingw32.Dockerfile =>
fedora-36-cross-mingw32.Dockerfile} (98%)
rename ci/containers/{fedora-35-cross-mingw64.Dockerfile =>
fedora-36-cross-mingw64.Dockerfile} (98%)
rename ci/containers/{fedora-34.Dockerfile => fedora-36.Dockerfile} (97%)
create mode 100644 ci/gitlab/build-templates.yml
create mode 100644 ci/gitlab/builds.yml
create mode 100644 ci/gitlab/container-templates.yml
create mode 100644 ci/gitlab/containers.yml
create mode 100644 ci/gitlab/sanity-checks.yml
--
2.35.3
Re: [libvirt PATCH 0/4] ci: Drop Fedora 34, add Fedora 36
On Thu, May 26, 2022 at 12:05:23PM +0200, Erik Skultety wrote: > On Thu, May 26, 2022 at 11:05:30AM +0200, Andrea Bolognani wrote: > > Test pipeline: > > > > https://gitlab.com/abologna/libvirt/-/pipelines/548549670 > > > > Matching libvirt-perl MR, needed because the integration tests > > download artifacts from there: > > > > https://gitlab.com/libvirt/libvirt-perl/-/merge_requests/63 > > Reviewed-by: Erik Skultety > > Please don't push this yet, because the integration tests would fail (as you > didn't didn't run them in your pipeline) I didn't think I would be able to. Can anyone trigger a run of the integration tests? Can group members? > I need to create a template > image for fedora-36 first on the baremetal host. I'll ping you when it's done. I think we will start seeing failures in the Fedora 34 integration jobs now that the libvirt-perl MR has been accepted and thus the corresponding job is gone. Or is GitLab CI smart enough to go and pick the artifacts from the last existing occurrence of the job? I can split things so that Fedora 34 is dropped and Fedora 36 is only added to the non-integration part of the pipeline at first, and then the integration job is added in a separate commit. The bulk could be pushed right away, with the last bit waiting for you to prepare the VM template. Or is that a quick enough job that it doesn't matter in practice and I'm massively overthinking this? O:-) -- Andrea Bolognani / Red Hat / Virtualization
