Re: [libvirt PATCH v2] tests: Undo recent breakages
On Mon, Mar 06, 2023 at 04:29:19PM +, Daniel P. Berrangé wrote: > On Mon, Mar 06, 2023 at 05:23:46PM +0100, Andrea Bolognani wrote: > > Turns out that those overrides I recently removed where actually > > there for a reason, and there was a motivation behind creating > > the driver config as unprivileged too O:-) > > FWIW, this is going to be a super frustrating description to > someone reading the commit message in 6 months time, wondering > if we can remove this code, as you've not told the reader what > the root cause problem actually was Fair enough, but too late to change it now... I'll try to fix the problem myself, and hopefully it will take less than six months :) -- Andrea Bolognani / Red Hat / Virtualization
Re: passt SELinux labelling (was: Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start)
On Mon, Mar 06, 2023 at 09:03:42AM +, Daniel P. Berrangé wrote: > On Fri, Mar 03, 2023 at 07:46:27PM -0500, Laine Stump wrote: > > On 3/3/23 1:36 PM, Daniel P. Berrangé wrote: > > > On Fri, Mar 03, 2023 at 10:18:39AM -0800, Andrea Bolognani wrote: > > > > I still don't understand why we can't simply execute passt and let > > > > the domain transition defined in the policy take care of switching to > > > > the appropriate label from us, like we do for dnsmasq and other > > > > tools? Why do we need to do things differently for passt? > > > > > > That won't get the per-VM label applied. It will end up running > > > passt_t:s0:c0.c1023, but we want it to be passt_t:s0:c342,155. > > > To transition from non-MCS to MCS, you have to explicitly tell > > > the kernel what to do instead of relying on the plain automatic > > > transition. > > > > Since you've brought up dnsmasq as an example, note that the dnsmasq > > processes don't have any MCS (which I guess is the right thing, since a > > single dnsmasq might be used by many/any/all guests, contrasting with passt, > > or the slirp-helper or tpm, which have one instance per guest device. This > > does make dnsmasq a "not ideal" example when figuring out what is needed for > > passt though (in some ways, but not others)(I think? I still can't claim to > > fully grok all the details of this). > > Yes, you are correct. > > dbus/slirp/swtpm are better matches for the passt scenario, though they > are also not useful examples since we've ignored the SELinux labeling > needs in all of them, so they all undermine svirt if used. Got it! The MCS bit is indeed the part that I had been missing. -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH v2] tests: Undo recent breakages
Turns out that those overrides I recently removed where actually
there for a reason, and there was a motivation behind creating
the driver config as unprivileged too O:-)
Until a solution that can both ensure predictable output and
avoid code duplication is developed, go back to the previous
approach.
Fixes: 2f56f69f7f7e ("tests: Create privileged config for QEMU driver")
Fixes: 0f49b6cc6b81 ("tests: Drop no longer necessary overrides")
Fixes: 0b464cd84ff3 ("tests: Drop more QEMU driver config overrides")
Signed-off-by: Andrea Bolognani
---
Pushed under the build breaker rule.
Pipeline:
https://gitlab.com/abologna/libvirt/-/pipelines/797459727
Note that the aarch64-macos-12 job is marked as failed, but the
underlying Cirrus CI job
https://cirrus-ci.com/task/4640605947559936
finished successfully.
tests/testutilsqemu.c | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index d3cdbdb4d5..d715252aee 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -631,7 +631,7 @@ int qemuTestDriverInit(virQEMUDriver *driver)
driver->hostarch = virArchFromHost();
-cfg = virQEMUDriverConfigNew(true, NULL);
+cfg = virQEMUDriverConfigNew(false, NULL);
if (!cfg)
goto error;
driver->config = cfg;
@@ -641,6 +641,24 @@ int qemuTestDriverInit(virQEMUDriver *driver)
VIR_FREE(cfg->stateDir);
VIR_FREE(cfg->configDir);
+/* Override paths to ensure predictable output
+ *
+ * FIXME Find a way to achieve the same result while avoiding
+ * code duplication
+ */
+VIR_FREE(cfg->libDir);
+cfg->libDir = g_strdup("/var/lib/libvirt/qemu");
+VIR_FREE(cfg->channelTargetDir);
+cfg->channelTargetDir = g_strdup("/var/lib/libvirt/qemu/channel/target");
+VIR_FREE(cfg->memoryBackingDir);
+cfg->memoryBackingDir = g_strdup("/var/lib/libvirt/qemu/ram");
+VIR_FREE(cfg->nvramDir);
+cfg->nvramDir = g_strdup("/var/lib/libvirt/qemu/nvram");
+VIR_FREE(cfg->passtStateDir);
+cfg->passtStateDir = g_strdup("/var/run/libvirt/qemu/passt");
+VIR_FREE(cfg->dbusStateDir);
+cfg->dbusStateDir = g_strdup("/var/run/libvirt/qemu/dbus");
+
if (!g_mkdtemp(statedir)) {
fprintf(stderr, "Cannot create fake stateDir");
goto error;
--
2.39.2
[libvirt PATCH] tests: Undo recent breakages
Turns out that those overrides that I recently removed where
actually there for a reason :)
Until a solution that can both ensure predictable output and
avoid code duplication is developed, return to the (slightly
improved) status quo.
Fixes: 0f49b6cc6b81 ("tests: Drop no longer necessary overrides")
Fixes: 0b464cd84ff3 ("tests: Drop more QEMU driver config overrides")
Signed-off-by: Andrea Bolognani
---
I will push this under the build breaker rule as soon as the
corresponding pipeline[1] has successfully completed.
[1] https://gitlab.com/abologna/libvirt/-/pipelines/797404546
tests/testutilsqemu.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index d3cdbdb4d5..e7030cf318 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -641,6 +641,24 @@ int qemuTestDriverInit(virQEMUDriver *driver)
VIR_FREE(cfg->stateDir);
VIR_FREE(cfg->configDir);
+/* Override paths to ensure predictable output
+ *
+ * FIXME Find a way to achieve the same result while avoiding all
+ * the duplicated code
+ */
+VIR_FREE(cfg->libDir);
+cfg->libDir = g_strdup("/var/lib/libvirt/qemu");
+VIR_FREE(cfg->channelTargetDir);
+cfg->channelTargetDir = g_strdup("/var/lib/libvirt/qemu/channel/target");
+VIR_FREE(cfg->memoryBackingDir);
+cfg->memoryBackingDir = g_strdup("/var/lib/libvirt/qemu/ram");
+VIR_FREE(cfg->nvramDir);
+cfg->nvramDir = g_strdup("/var/lib/libvirt/qemu/nvram");
+VIR_FREE(cfg->passtStateDir);
+cfg->passtStateDir = g_strdup("/var/run/libvirt/qemu/passt");
+VIR_FREE(cfg->dbusStateDir);
+cfg->dbusStateDir = g_strdup("/var/run/libvirt/qemu/dbus");
+
if (!g_mkdtemp(statedir)) {
fprintf(stderr, "Cannot create fake stateDir");
goto error;
--
2.39.2
Re: [libvirt PATCH 00/14] tests: Improve QEMU driver config handling
On Mon, Mar 06, 2023 at 01:49:50PM +0100, Martin Kletzander wrote: > On Mon, Mar 06, 2023 at 04:41:28AM -0800, Andrea Bolognani wrote: > > A test called "controller-order" is definitely unrelated to SPICE > > TLS. For "q35-virt-manager-basic", I guess you could argue either > > way? It feels more unrelated than related to me. > > No, of course, that's not the point, it is not trying to test spice with > tls. But because the spice-tls tests are not next to each other (and > would probably make a bigger mess in the order anyway) it *feels* > arbitrary when the spiceTLS is set and unset, it's fine the way it is. Gotcha :) > > Or were you referring to the various VIR_FREE(*TLSx509secretUUID) > > calls that I'm adding? Because that was indeed intended to be a > > separate commit, which I seem to have accidentally squashed in O:-) Posted that change as a separate patch, as it was originally intended, here: https://listman.redhat.com/archives/libvir-list/2023-March/238448.html Please let me know if I can use that instead of the combined version from v1 that you ACKed. -- Andrea Bolognani / Red Hat / Virtualization
[PATCH 05+1/14] tests: Limit use of TLSx509secretUUIDs
These are intended to be used for just a few specific tests,
but since we don't always free them up afterwards they could
end up accidentally affecting subsequent tests as well.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvtest.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index deb1dc2667..afa22275b8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1296,6 +1296,8 @@ mymain(void)
DO_TEST_CAPS_LATEST("disk-network-tlsx509-nbd-hostname");
DO_TEST_CAPS_VER("disk-network-tlsx509-vxhs", "5.0.0");
DO_TEST_CAPS_LATEST("disk-network-http");
+VIR_FREE(driver.config->nbdTLSx509secretUUID);
+VIR_FREE(driver.config->vxhsTLSx509secretUUID);
driver.config->vxhsTLS = 0;
DO_TEST_CAPS_LATEST("disk-no-boot");
DO_TEST_CAPS_LATEST("disk-nvme");
@@ -1574,6 +1576,7 @@ mymain(void)
DO_TEST("serial-tcp-tlsx509-secret-chardev",
QEMU_CAPS_DEVICE_ISA_SERIAL);
DO_TEST_CAPS_LATEST("serial-tcp-tlsx509-secret-chardev");
+VIR_FREE(driver.config->chardevTLSx509secretUUID);
driver.config->chardevTLS = 0;
DO_TEST("serial-many-chardev",
QEMU_CAPS_DEVICE_ISA_SERIAL);
--
2.39.2
Re: [libvirt PATCH 00/14] tests: Improve QEMU driver config handling
On Mon, Mar 06, 2023 at 12:02:03PM +0100, Martin Kletzander wrote: > On Mon, Mar 06, 2023 at 11:10:33AM +0100, Andrea Bolognani wrote: > > Move more settings to common code and more closely match > > real-world configurations. > > > > Andrea Bolognani (14): > > tests: Poison more XDG variables > > tests: Drop unnecessary configuration overrides > > tests: Drop unnecessary free > > tests: Increase scope for SASLdirs > > tests: Limit use of SPICE TLS > > The changes to driver.config->spiceTLS look very arbitrary, but that's > no biggie. I feel that it's consistent with how we set all the other *TLS variables to 1 right before the tests that are specifically about testing TLS with vnc/chardev/whatever, and back to 0 immediately afterwards. A test called "controller-order" is definitely unrelated to SPICE TLS. For "q35-virt-manager-basic", I guess you could argue either way? It feels more unrelated than related to me. Or were you referring to the various VIR_FREE(*TLSx509secretUUID) calls that I'm adding? Because that was indeed intended to be a separate commit, which I seem to have accidentally squashed in O:-) -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 08/14] tests: Drop no longer necessary overrides
Creating a privileged config ensures these are already set
correctly.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvtest.c | 6 --
tests/qemuxml2xmltest.c | 3 ---
2 files changed, 9 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 22c0642d25..892c74f917 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -830,8 +830,6 @@ mymain(void)
linuxCaps = driver.caps;
macOSCaps = testQemuCapsInitMacOS();
-VIR_FREE(driver.config->defaultTLSx509certdir);
-driver.config->defaultTLSx509certdir = g_strdup("/etc/pki/qemu");
VIR_FREE(driver.config->vncTLSx509certdir);
driver.config->vncTLSx509certdir = g_strdup("/etc/pki/libvirt-vnc");
VIR_FREE(driver.config->spiceTLSx509certdir);
@@ -857,10 +855,6 @@ mymain(void)
driver.config->hugetlbfs[0].deflt = true;
driver.config->hugetlbfs[1].size = 1048576;
driver.config->spicePassword = g_strdup("123456");
-VIR_FREE(driver.config->memoryBackingDir);
-driver.config->memoryBackingDir = g_strdup("/var/lib/libvirt/qemu/ram");
-VIR_FREE(driver.config->nvramDir);
-driver.config->nvramDir = g_strdup("/var/lib/libvirt/qemu/nvram");
virFileWrapperAddPrefix(SYSCONFDIR "/qemu/firmware",
abs_srcdir "/qemufirmwaredata/etc/qemu/firmware");
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7ccf3ab97f..8860e11006 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -149,9 +149,6 @@ mymain(void)
cfg = virQEMUDriverGetConfig();
-VIR_FREE(cfg->nvramDir);
-cfg->nvramDir = g_strdup("/var/lib/libvirt/qemu/nvram");
-
if (!(conn = virGetConnect()))
goto cleanup;
--
2.39.2
[libvirt PATCH 13/14] tests: Set SASLdirs to default values
We use standard paths for almost everything else.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvdata/graphics-spice-sasl.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/graphics-vnc-sasl.x86_64-latest.args | 2 +-
.../graphics-vnc-tls-secret.x86_64-5.2.0.args | 2 +-
.../graphics-vnc-tls-secret.x86_64-latest.args| 2 +-
tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args| 2 +-
tests/testutilsqemu.c | 4 ++--
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/tests/qemuxml2argvdata/graphics-spice-sasl.x86_64-latest.args
b/tests/qemuxml2argvdata/graphics-spice-sasl.x86_64-latest.args
index 5e0a6f28a0..2700493826 100644
--- a/tests/qemuxml2argvdata/graphics-spice-sasl.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/graphics-spice-sasl.x86_64-latest.args
@@ -6,7 +6,7 @@ LOGNAME=test \
XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-SASL_CONF_PATH=/root/.sasl2 \
+SASL_CONF_PATH=/etc/sasl2 \
/usr/bin/qemu-system-x86_64 \
-name guest=QEMUGuest1,debug-threads=on \
-S \
diff --git a/tests/qemuxml2argvdata/graphics-vnc-sasl.x86_64-latest.args
b/tests/qemuxml2argvdata/graphics-vnc-sasl.x86_64-latest.args
index 6f7cf2eb21..2bc599d1c8 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-sasl.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/graphics-vnc-sasl.x86_64-latest.args
@@ -6,7 +6,7 @@ LOGNAME=test \
XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-SASL_CONF_PATH=/root/.sasl2 \
+SASL_CONF_PATH=/etc/sasl2 \
/usr/bin/qemu-system-x86_64 \
-name guest=QEMUGuest1,debug-threads=on \
-S \
diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-5.2.0.args
b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-5.2.0.args
index fb3ebd8f5a..9e4e16b888 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-5.2.0.args
+++ b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-5.2.0.args
@@ -6,7 +6,7 @@ LOGNAME=test \
XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-SASL_CONF_PATH=/root/.sasl2 \
+SASL_CONF_PATH=/etc/sasl2 \
/usr/bin/qemu-system-x86_64 \
-name guest=QEMUGuest1,debug-threads=on \
-S \
diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
index d7e577797e..c564bd9d9b 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/graphics-vnc-tls-secret.x86_64-latest.args
@@ -6,7 +6,7 @@ LOGNAME=test \
XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-SASL_CONF_PATH=/root/.sasl2 \
+SASL_CONF_PATH=/etc/sasl2 \
/usr/bin/qemu-system-x86_64 \
-name guest=QEMUGuest1,debug-threads=on \
-S \
diff --git a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args
b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args
index a9371d1786..8ad660e11f 100644
--- a/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/graphics-vnc-tls.x86_64-latest.args
@@ -6,7 +6,7 @@ LOGNAME=test \
XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-SASL_CONF_PATH=/root/.sasl2 \
+SASL_CONF_PATH=/etc/sasl2 \
/usr/bin/qemu-system-x86_64 \
-name guest=QEMUGuest1,debug-threads=on \
-S \
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 663ea44e89..1fabd8764e 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -699,9 +699,9 @@ int qemuTestDriverInit(virQEMUDriver *driver)
cfg->backupTLSx509certdir = g_strdup("/etc/pki/libvirt-backup");
VIR_FREE(cfg->vncSASLdir);
-cfg->vncSASLdir = g_strdup("/root/.sasl2");
+cfg->vncSASLdir = g_strdup("/etc/sasl2");
VIR_FREE(cfg->spiceSASLdir);
-cfg->spiceSASLdir = g_strdup("/root/.sasl2");
+cfg->spiceSASLdir = g_strdup("/etc/sasl2");
VIR_FREE(cfg->spicePassword);
cfg->spicePassword = g_strdup("123456");
--
2.39.2
[libvirt PATCH 12/14] tests: Set TLSx509certdirs to default values
For almost all directories, the value we set matches the one
a standard deployment would use, but in a couple of cases they
deviate from that. Keep things consistent.
Signed-off-by: Andrea Bolognani
---
.../disk-network-tlsx509-nbd-hostname.x86_64-latest.args | 2 +-
.../disk-network-tlsx509-nbd.x86_64-5.2.0.args| 2 +-
.../disk-network-tlsx509-nbd.x86_64-latest.args | 2 +-
.../disk-network-tlsx509-vxhs.x86_64-5.0.0.args | 4 ++--
tests/testutilsqemu.c | 4 ++--
5 files changed, 7 insertions(+), 7 deletions(-)
diff --git
a/tests/qemuxml2argvdata/disk-network-tlsx509-nbd-hostname.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-network-tlsx509-nbd-hostname.x86_64-latest.args
index 870ec42482..f1fc42404c 100644
---
a/tests/qemuxml2argvdata/disk-network-tlsx509-nbd-hostname.x86_64-latest.args
+++
b/tests/qemuxml2argvdata/disk-network-tlsx509-nbd-hostname.x86_64-latest.args
@@ -29,7 +29,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-boot strict=on \
-device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
-object
'{"qom-type":"secret","id":"objlibvirt-1-storage_tls0-secret0","data":"9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1","keyid":"masterKey0","iv":"AAECAwQFBgcICQoLDA0ODw==","format":"base64"}'
\
--object
'{"qom-type":"tls-creds-x509","id":"objlibvirt-1-storage_tls0","dir":"/etc/pki/libvirt-nbd/dummy,path","endpoint":"client","verify-peer":true,"passwordid":"objlibvirt-1-storage_tls0-secret0"}'
\
+-object
'{"qom-type":"tls-creds-x509","id":"objlibvirt-1-storage_tls0","dir":"/etc/pki/libvirt-nbd","endpoint":"client","verify-peer":true,"passwordid":"objlibvirt-1-storage_tls0-secret0"}'
\
-blockdev
'{"driver":"nbd","server":{"type":"inet","host":"example.com","port":"1234"},"tls-creds":"objlibvirt-1-storage_tls0","tls-hostname":"test-hostname","node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-1-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"raw","file":"libvirt-1-storage"}'
\
-device
'{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x7","drive":"libvirt-1-format","id":"virtio-disk3","bootindex":1,"write-cache":"on"}'
\
diff --git a/tests/qemuxml2argvdata/disk-network-tlsx509-nbd.x86_64-5.2.0.args
b/tests/qemuxml2argvdata/disk-network-tlsx509-nbd.x86_64-5.2.0.args
index 8ed921dda6..9dae69a016 100644
--- a/tests/qemuxml2argvdata/disk-network-tlsx509-nbd.x86_64-5.2.0.args
+++ b/tests/qemuxml2argvdata/disk-network-tlsx509-nbd.x86_64-5.2.0.args
@@ -29,7 +29,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-boot strict=on \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
-object
secret,id=objlibvirt-1-storage_tls0-secret0,data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64
\
--object
tls-creds-x509,id=objlibvirt-1-storage_tls0,dir=/etc/pki/libvirt-nbd/dummy,,path,endpoint=client,verify-peer=on,passwordid=objlibvirt-1-storage_tls0-secret0
\
+-object
tls-creds-x509,id=objlibvirt-1-storage_tls0,dir=/etc/pki/libvirt-nbd,endpoint=client,verify-peer=on,passwordid=objlibvirt-1-storage_tls0-secret0
\
-blockdev
'{"driver":"nbd","server":{"type":"inet","host":"example.com","port":"1234"},"tls-creds":"objlibvirt-1-storage_tls0","node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-1-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"raw","file":"libvirt-1-storage"}'
\
-device
virtio-blk-pci,bus=pci.0,addr=0x7,drive=libvirt-1-format,id=virtio-disk3,bootin
[libvirt PATCH 10/14] tests: Move more QEMU driver settings to common code
None of these settings is specific to the xml2argv test. Moving
them to the common code ensures the behavior of the QEMU driver
is consistent across all QEMU tests.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvtest.c | 26 --
tests/testutilsqemu.c| 28
2 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 892c74f917..2562da6445 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -830,32 +830,6 @@ mymain(void)
linuxCaps = driver.caps;
macOSCaps = testQemuCapsInitMacOS();
-VIR_FREE(driver.config->vncTLSx509certdir);
-driver.config->vncTLSx509certdir = g_strdup("/etc/pki/libvirt-vnc");
-VIR_FREE(driver.config->spiceTLSx509certdir);
-driver.config->spiceTLSx509certdir = g_strdup("/etc/pki/libvirt-spice");
-VIR_FREE(driver.config->chardevTLSx509certdir);
-driver.config->chardevTLSx509certdir =
g_strdup("/etc/pki/libvirt-chardev");
-VIR_FREE(driver.config->vxhsTLSx509certdir);
-driver.config->vxhsTLSx509certdir =
g_strdup("/etc/pki/libvirt-vxhs/dummy,path");
-VIR_FREE(driver.config->nbdTLSx509certdir);
-driver.config->nbdTLSx509certdir =
g_strdup("/etc/pki/libvirt-nbd/dummy,path");
-
-VIR_FREE(driver.config->vncSASLdir);
-driver.config->vncSASLdir = g_strdup("/root/.sasl2");
-VIR_FREE(driver.config->spiceSASLdir);
-driver.config->spiceSASLdir = g_strdup("/root/.sasl2");
-
-VIR_FREE(driver.config->hugetlbfs);
-driver.config->hugetlbfs = g_new0(virHugeTLBFS, 2);
-driver.config->nhugetlbfs = 2;
-driver.config->hugetlbfs[0].mnt_dir = g_strdup("/dev/hugepages2M");
-driver.config->hugetlbfs[1].mnt_dir = g_strdup("/dev/hugepages1G");
-driver.config->hugetlbfs[0].size = 2048;
-driver.config->hugetlbfs[0].deflt = true;
-driver.config->hugetlbfs[1].size = 1048576;
-driver.config->spicePassword = g_strdup("123456");
-
virFileWrapperAddPrefix(SYSCONFDIR "/qemu/firmware",
abs_srcdir "/qemufirmwaredata/etc/qemu/firmware");
virFileWrapperAddPrefix(PREFIX "/share/qemu/firmware",
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 7c14bdfcd7..9b6be28fa1 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -683,6 +683,34 @@ int qemuTestDriverInit(virQEMUDriver *driver)
qemuTestSetHostCPU(driver, driver->hostarch, NULL);
+VIR_FREE(cfg->vncTLSx509certdir);
+cfg->vncTLSx509certdir = g_strdup("/etc/pki/libvirt-vnc");
+VIR_FREE(cfg->spiceTLSx509certdir);
+cfg->spiceTLSx509certdir = g_strdup("/etc/pki/libvirt-spice");
+VIR_FREE(cfg->chardevTLSx509certdir);
+cfg->chardevTLSx509certdir = g_strdup("/etc/pki/libvirt-chardev");
+VIR_FREE(cfg->vxhsTLSx509certdir);
+cfg->vxhsTLSx509certdir = g_strdup("/etc/pki/libvirt-vxhs/dummy,path");
+VIR_FREE(cfg->nbdTLSx509certdir);
+cfg->nbdTLSx509certdir = g_strdup("/etc/pki/libvirt-nbd/dummy,path");
+
+VIR_FREE(cfg->vncSASLdir);
+cfg->vncSASLdir = g_strdup("/root/.sasl2");
+VIR_FREE(cfg->spiceSASLdir);
+cfg->spiceSASLdir = g_strdup("/root/.sasl2");
+
+VIR_FREE(cfg->spicePassword);
+cfg->spicePassword = g_strdup("123456");
+
+VIR_FREE(cfg->hugetlbfs);
+cfg->hugetlbfs = g_new0(virHugeTLBFS, 2);
+cfg->nhugetlbfs = 2;
+cfg->hugetlbfs[0].mnt_dir = g_strdup("/dev/hugepages2M");
+cfg->hugetlbfs[1].mnt_dir = g_strdup("/dev/hugepages1G");
+cfg->hugetlbfs[0].size = 2048;
+cfg->hugetlbfs[0].deflt = true;
+cfg->hugetlbfs[1].size = 1048576;
+
driver->privileged = true;
return 0;
--
2.39.2
[libvirt PATCH 11/14] tests: Set more TLSx509certdirs
We were missing a couple.
Signed-off-by: Andrea Bolognani
---
tests/testutilsqemu.c | 4
1 file changed, 4 insertions(+)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 9b6be28fa1..1ce76ac855 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -693,6 +693,10 @@ int qemuTestDriverInit(virQEMUDriver *driver)
cfg->vxhsTLSx509certdir = g_strdup("/etc/pki/libvirt-vxhs/dummy,path");
VIR_FREE(cfg->nbdTLSx509certdir);
cfg->nbdTLSx509certdir = g_strdup("/etc/pki/libvirt-nbd/dummy,path");
+VIR_FREE(cfg->migrateTLSx509certdir);
+cfg->migrateTLSx509certdir = g_strdup("/etc/pki/libvirt-migrate");
+VIR_FREE(cfg->backupTLSx509certdir);
+cfg->backupTLSx509certdir = g_strdup("/etc/pki/libvirt-backup");
VIR_FREE(cfg->vncSASLdir);
cfg->vncSASLdir = g_strdup("/root/.sasl2");
--
2.39.2
[libvirt PATCH 09/14] tests: Add convenience variable for QEMU driver config
This makes the code less clunky.
Signed-off-by: Andrea Bolognani
---
tests/testutilsqemu.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 55d680ac84..7c14bdfcd7 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -609,6 +609,7 @@ qemuTestCapsCacheInsertMacOS(virFileCache *cache,
int qemuTestDriverInit(virQEMUDriver *driver)
{
+virQEMUDriverConfig *cfg = NULL;
virSecurityManager *mgr = NULL;
char statedir[] = STATEDIRTEMPLATE;
char configdir[] = CONFIGDIRTEMPLATE;
@@ -626,34 +627,36 @@ int qemuTestDriverInit(virQEMUDriver *driver)
return -1;
driver->hostarch = virArchFromHost();
-driver->config = virQEMUDriverConfigNew(true, NULL);
-if (!driver->config)
+
+cfg = virQEMUDriverConfigNew(true, NULL);
+if (!cfg)
goto error;
+driver->config = cfg;
/* Do this early so that qemuTestDriverFree() doesn't see (unlink) the real
* dirs. */
-VIR_FREE(driver->config->stateDir);
-VIR_FREE(driver->config->configDir);
+VIR_FREE(cfg->stateDir);
+VIR_FREE(cfg->configDir);
/* Overwrite some default paths so it's consistent for tests. */
-VIR_FREE(driver->config->libDir);
-VIR_FREE(driver->config->channelTargetDir);
-driver->config->libDir = g_strdup("/tmp/lib");
-driver->config->channelTargetDir = g_strdup("/tmp/channel");
+VIR_FREE(cfg->libDir);
+VIR_FREE(cfg->channelTargetDir);
+cfg->libDir = g_strdup("/tmp/lib");
+cfg->channelTargetDir = g_strdup("/tmp/channel");
if (!g_mkdtemp(statedir)) {
fprintf(stderr, "Cannot create fake stateDir");
goto error;
}
-driver->config->stateDir = g_strdup(statedir);
+cfg->stateDir = g_strdup(statedir);
if (!g_mkdtemp(configdir)) {
fprintf(stderr, "Cannot create fake configDir");
goto error;
}
-driver->config->configDir = g_strdup(configdir);
+cfg->configDir = g_strdup(configdir);
driver->caps = testQemuCapsInit();
if (!driver->caps)
--
2.39.2
[libvirt PATCH 05/14] tests: Limit use of SPICE TLS
Follow the example of other similar settings and only enable it
for the few test cases that are actually about the specific
functionality, disabling it immediately afterwards.
A few test cases that were completely unrelated to SPICE TLS no
longer see the effects of having the feature enabled.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvdata/controller-order.args| 2 +-
.../q35-virt-manager-basic.x86_64-4.2.0.args| 2 +-
.../q35-virt-manager-basic.x86_64-latest.args | 2 +-
tests/qemuxml2argvtest.c| 13 -
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/tests/qemuxml2argvdata/controller-order.args
b/tests/qemuxml2argvdata/controller-order.args
index 242639591c..cc45b1f946 100644
--- a/tests/qemuxml2argvdata/controller-order.args
+++ b/tests/qemuxml2argvdata/controller-order.args
@@ -43,7 +43,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-fdr/.config \
-device
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
\
-device usb-tablet,id=input0,bus=usb.0,port=1.2 \
-audiodev '{"id":"audio1","driver":"spice"}' \
--spice
port=5901,tls-port=5902,addr=0.0.0.0,x509-dir=/etc/pki/libvirt-spice,seamless-migration=on
\
+-spice port=5901,addr=0.0.0.0,seamless-migration=on \
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
-device intel-hda,id=sound0,bus=pci.0,addr=0x4 \
-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0,audiodev=audio1 \
diff --git a/tests/qemuxml2argvdata/q35-virt-manager-basic.x86_64-4.2.0.args
b/tests/qemuxml2argvdata/q35-virt-manager-basic.x86_64-4.2.0.args
index 042f965183..86da1e46ab 100644
--- a/tests/qemuxml2argvdata/q35-virt-manager-basic.x86_64-4.2.0.args
+++ b/tests/qemuxml2argvdata/q35-virt-manager-basic.x86_64-4.2.0.args
@@ -49,7 +49,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-virt-manager-basic/.config
\
-device
virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1,name=com.redhat.spice.0
\
-device usb-tablet,id=input0,bus=usb.0,port=1 \
-audiodev '{"id":"audio1","driver":"spice"}' \
--spice
port=5901,tls-port=5902,addr=127.0.0.1,x509-dir=/etc/pki/libvirt-spice,image-compression=off,seamless-migration=on
\
+-spice port=5901,addr=127.0.0.1,image-compression=off,seamless-migration=on \
-device
qxl-vga,id=video0,max_outputs=1,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,bus=pcie.0,addr=0x1
\
-device ich9-intel-hda,id=sound0,bus=pcie.0,addr=0x1b \
-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0,audiodev=audio1 \
diff --git a/tests/qemuxml2argvdata/q35-virt-manager-basic.x86_64-latest.args
b/tests/qemuxml2argvdata/q35-virt-manager-basic.x86_64-latest.args
index de4443c4ab..4b53442846 100644
--- a/tests/qemuxml2argvdata/q35-virt-manager-basic.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/q35-virt-manager-basic.x86_64-latest.args
@@ -49,7 +49,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-virt-manager-basic/.config
\
-device
'{"driver":"virtserialport","bus":"virtio-serial0.0","nr":2,"chardev":"charchannel1","id":"channel1","name":"com.redhat.spice.0"}'
\
-device '{"driver":"usb-tablet","id":"input0","bus":"usb.0","port":"1"}' \
-audiodev '{"id":"audio1","driver":"spice"}' \
--spice
port=5901,tls-port=5902,addr=127.0.0.1,x509-dir=/etc/pki/libvirt-spice,image-compression=off,seamless-migration=on
\
+-spice port=5901,addr=127.0.0.1,image-compression=off,seamless-migration=on \
-device
'{"driver":"qxl-vga","id":"video0","max_outputs":1,"ram_size":67108864,"vram_size":67108864,"vram64_size_mb":0,"vgamem_mb":16,"bus":"pcie.0","addr":"0x1"}'
\
-device
'{"driver":"ich9-intel-hda","id":"sound0","bus":"pcie.0","addr":"0x1b"}' \
-device
'{"driver":"hda-duplex","id":"sound0-codec0","bus":"sound0.0","cad":0,"audiodev":"audio1"}'
\
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 2f2983da79..afa22275b8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -858,7 +858,6 @@ mymain(void)
driver.config->hugetlbfs[0].size = 2048;
driver.config->hugetlbfs[0].deflt = true;
driver.config->hugetlbfs[1].size = 1048576;
-driver.config->spiceTLS = 1;
driver.config->spicePassword = g_strdup("123456");
VIR_FREE(driver.config->memoryBackingDir);
driver.config->memoryBackingDir = g_strdup("/var/
[libvirt PATCH 06/14] tests: Set the QEMU driver as privileged in common code
Most test programs were already doing this, and moving it to
the common code ensures we see consistent behavior across all
QEMU tests.
Signed-off-by: Andrea Bolognani
---
tests/qemumemlocktest.c| 2 --
tests/qemumigrationcookiexmltest.c | 2 --
tests/qemustatusxml2xmltest.c | 2 --
tests/qemuxml2argvtest.c | 2 --
tests/qemuxml2xmltest.c| 1 -
tests/testutilsqemu.c | 2 ++
6 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c
index a03ed20a45..2f4fe786f5 100644
--- a/tests/qemumemlocktest.c
+++ b/tests/qemumemlocktest.c
@@ -51,8 +51,6 @@ mymain(void)
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
-driver.privileged = true;
-
# define DO_TEST(name, memlock) \
do { \
static struct testInfo info = { \
diff --git a/tests/qemumigrationcookiexmltest.c
b/tests/qemumigrationcookiexmltest.c
index 0e5f956a2a..2f0d84148b 100644
--- a/tests/qemumigrationcookiexmltest.c
+++ b/tests/qemumigrationcookiexmltest.c
@@ -413,8 +413,6 @@ mymain(void)
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
-driver.privileged = true;
-
if (!(conn = virGetConnect()))
goto cleanup;
diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c
index 7587c4c34f..408955a8f5 100644
--- a/tests/qemustatusxml2xmltest.c
+++ b/tests/qemustatusxml2xmltest.c
@@ -87,8 +87,6 @@ mymain(void)
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
-driver.privileged = true;
-
if (!(conn = virGetConnect()))
goto cleanup;
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index afa22275b8..22c0642d25 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -830,8 +830,6 @@ mymain(void)
linuxCaps = driver.caps;
macOSCaps = testQemuCapsInitMacOS();
-driver.privileged = true;
-
VIR_FREE(driver.config->defaultTLSx509certdir);
driver.config->defaultTLSx509certdir = g_strdup("/etc/pki/qemu");
VIR_FREE(driver.config->vncTLSx509certdir);
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 69bff80376..7ccf3ab97f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -148,7 +148,6 @@ mymain(void)
macOSCaps = testQemuCapsInitMacOS();
cfg = virQEMUDriverGetConfig();
-driver.privileged = true;
VIR_FREE(cfg->nvramDir);
cfg->nvramDir = g_strdup("/var/lib/libvirt/qemu/nvram");
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 396803c40b..20bf32b321 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -680,6 +680,8 @@ int qemuTestDriverInit(virQEMUDriver *driver)
qemuTestSetHostCPU(driver, driver->hostarch, NULL);
+driver->privileged = true;
+
return 0;
error:
--
2.39.2
[libvirt PATCH 03/14] tests: Drop unnecessary free
The various TLSx509certdirs can be set throughout the lifetime
of the test program without issue.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvtest.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f46fc29f32..cdcd040270 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1293,7 +1293,6 @@ mymain(void)
DO_TEST_CAPS_VER("disk-network-tlsx509-vxhs", "5.0.0");
DO_TEST_CAPS_LATEST("disk-network-http");
driver.config->vxhsTLS = 0;
-VIR_FREE(driver.config->vxhsTLSx509certdir);
DO_TEST_CAPS_LATEST("disk-no-boot");
DO_TEST_CAPS_LATEST("disk-nvme");
DO_TEST_CAPS_VER("disk-vhostuser-numa", "4.2.0");
@@ -1396,7 +1395,6 @@ mymain(void)
VIR_FREE(driver.config->vncTLSx509secretUUID);
driver.config->vncSASL = driver.config->vncTLSx509verify =
driver.config->vncTLS = 0;
VIR_FREE(driver.config->vncSASLdir);
-VIR_FREE(driver.config->vncTLSx509certdir);
DO_TEST_CAPS_LATEST("graphics-vnc-egl-headless");
DO_TEST("graphics-sdl",
@@ -1566,14 +1564,11 @@ mymain(void)
DO_TEST("serial-tcp-tlsx509-chardev-notls",
QEMU_CAPS_DEVICE_ISA_SERIAL);
DO_TEST_CAPS_LATEST("serial-tcp-tlsx509-chardev-notls");
-VIR_FREE(driver.config->chardevTLSx509certdir);
-driver.config->chardevTLSx509certdir =
g_strdup("/etc/pki/libvirt-chardev");
driver.config->chardevTLSx509secretUUID =
g_strdup("6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea");
DO_TEST("serial-tcp-tlsx509-secret-chardev",
QEMU_CAPS_DEVICE_ISA_SERIAL);
DO_TEST_CAPS_LATEST("serial-tcp-tlsx509-secret-chardev");
driver.config->chardevTLS = 0;
-VIR_FREE(driver.config->chardevTLSx509certdir);
DO_TEST("serial-many-chardev",
QEMU_CAPS_DEVICE_ISA_SERIAL);
DO_TEST_NOCAPS("parallel-tcp-chardev");
@@ -2983,7 +2978,6 @@ mymain(void)
DO_TEST_CAPS_LATEST("crypto-builtin");
-VIR_FREE(driver.config->nbdTLSx509certdir);
qemuTestDriverFree();
virFileWrapperClearPrefixes();
--
2.39.2
[libvirt PATCH 04/14] tests: Increase scope for SASLdirs
Just like TLSx509certdirs, these can be set throughout the
lifetime of the test program.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvtest.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index cdcd040270..2f2983da79 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -845,6 +845,11 @@ mymain(void)
VIR_FREE(driver.config->nbdTLSx509certdir);
driver.config->nbdTLSx509certdir =
g_strdup("/etc/pki/libvirt-nbd/dummy,path");
+VIR_FREE(driver.config->vncSASLdir);
+driver.config->vncSASLdir = g_strdup("/root/.sasl2");
+VIR_FREE(driver.config->spiceSASLdir);
+driver.config->spiceSASLdir = g_strdup("/root/.sasl2");
+
VIR_FREE(driver.config->hugetlbfs);
driver.config->hugetlbfs = g_new0(virHugeTLBFS, 2);
driver.config->nhugetlbfs = 2;
@@ -1383,8 +1388,6 @@ mymain(void)
DO_TEST_CAPS_LATEST("graphics-vnc-socket-new-cmdline");
driver.config->vncSASL = 1;
-VIR_FREE(driver.config->vncSASLdir);
-driver.config->vncSASLdir = g_strdup("/root/.sasl2");
DO_TEST_CAPS_LATEST("graphics-vnc-sasl");
driver.config->vncTLS = 1;
driver.config->vncTLSx509verify = 1;
@@ -1394,7 +1397,6 @@ mymain(void)
DO_TEST_CAPS_LATEST("graphics-vnc-tls-secret");
VIR_FREE(driver.config->vncTLSx509secretUUID);
driver.config->vncSASL = driver.config->vncTLSx509verify =
driver.config->vncTLS = 0;
-VIR_FREE(driver.config->vncSASLdir);
DO_TEST_CAPS_LATEST("graphics-vnc-egl-headless");
DO_TEST("graphics-sdl",
@@ -1405,9 +1407,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("graphics-spice");
DO_TEST_CAPS_LATEST("graphics-spice-no-args");
driver.config->spiceSASL = 1;
-driver.config->spiceSASLdir = g_strdup("/root/.sasl2");
DO_TEST_CAPS_LATEST("graphics-spice-sasl");
-VIR_FREE(driver.config->spiceSASLdir);
driver.config->spiceSASL = 0;
DO_TEST_CAPS_LATEST("graphics-spice-agentmouse");
DO_TEST_CAPS_LATEST("graphics-spice-compression");
--
2.39.2
[libvirt PATCH 07/14] tests: Create privileged config for QEMU driver
Our QEMU test suite effectively covers the qemu:///system
scenario, and we have to partially replace the unprivileged
config with its privileged equivalent after the fact to keep up
the illusion.
Instead of jumping through these extra hoops, we can simply
start with a privileged configuration matching the privileged
driver we're creating for test programs.
This change highlights that we were missing a couple of
overrides, specifically in the tests for passt and dbus. Now
that we're creating a privileged configuration, this kind of
issue shouldn't be able to slip into the test suite.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2argvdata/graphics-dbus.args| 2 +-
tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args | 2 +-
tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args | 2 +-
tests/testutilsqemu.c| 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qemuxml2argvdata/graphics-dbus.args
b/tests/qemuxml2argvdata/graphics-dbus.args
index a804ae06ee..89b4ff78c6 100644
--- a/tests/qemuxml2argvdata/graphics-dbus.args
+++ b/tests/qemuxml2argvdata/graphics-dbus.args
@@ -25,6 +25,6 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-no-acpi \
-boot strict=on \
-usb \
--display
dbus,addr=unix:path=/bad-test-used-env-xdg-runtime-dir/libvirt/qemu/run/dbus/-1-QEMUGuest1-dbus.sock
\
+-display
dbus,addr=unix:path=/var/run/libvirt/qemu/dbus/-1-QEMUGuest1-dbus.sock \
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args
b/tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args
index 037dabb87d..eb165abb90 100644
--- a/tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args
+++ b/tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args
@@ -30,7 +30,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-blockdev
'{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}'
\
-device
'{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}'
\
--netdev
'{"type":"stream","addr":{"type":"unix","path":"/bad-test-used-env-xdg-runtime-dir/libvirt/qemu/run/passt/-1-QEMUGuest1-net0.socket"},"server":false,"id":"hostnet0"}'
\
+-netdev
'{"type":"stream","addr":{"type":"unix","path":"/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net0.socket"},"server":false,"id":"hostnet0"}'
\
-device
'{"driver":"rtl8139","netdev":"hostnet0","id":"net0","mac":"00:11:22:33:44:55","bus":"pci.0","addr":"0x2"}'
\
-audiodev '{"id":"audio1","driver":"none"}' \
-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
b/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
index f84bec2ec1..2b0659e4bb 100644
--- a/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
@@ -30,7 +30,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
-blockdev
'{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
\
-blockdev
'{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}'
\
-device
'{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}'
\
--netdev
'{"type":"stream","addr":{"type":"unix","path":"/bad-test-used-env-xdg-runtime-dir/libvirt/qemu/run/passt/-1-QEMUGuest1-net0.socket"},"server":false,"reconnect":5,"id":"hostnet0"}'
\
+-netdev
'{"type":"stream","addr":{"type":"unix","path":"/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net0.socket"},"server":false,"reconnect":5,"id":"hostnet0"}'
\
-device
'{"driver":"rtl8139","netdev":"hostnet0","id":"net0","mac":"00:11:22:33:44:55","bus":"pci.0","addr":"0x2"}'
\
-audiodev '{"id":"audio1","driver":"none"}' \
-sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 20bf32b321..55d680ac84 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -626,7 +626,7 @@ int qemuTestDriverInit(virQEMUDriver *driver)
return -1;
driver->hostarch = virArchFromHost();
-driver->config = virQEMUDriverConfigNew(false, NULL);
+driver->config = virQEMUDriverConfigNew(true, NULL);
if (!driver->config)
goto error;
--
2.39.2
[libvirt PATCH 01/14] tests: Poison more XDG variables
We use these in QEMU command lines, so we should poison them
to catch test suite issues.
Signed-off-by: Andrea Bolognani
---
tests/testutils.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/testutils.c b/tests/testutils.c
index e8cb8e6737..d77b9e8db2 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -820,6 +820,9 @@ int virTestMain(int argc,
g_setenv("HOME", "/bad-test-used-env-home", TRUE);
g_setenv("XDG_RUNTIME_DIR", "/bad-test-used-env-xdg-runtime-dir", TRUE);
+g_setenv("XDG_DATA_HOME", "/bad-test-used-env-xdg-data-home", TRUE);
+g_setenv("XDG_CACHE_HOME", "/bad-test-used-env-xdg-cache-home", TRUE);
+g_setenv("XDG_CONFIG_HOME", "/bad-test-used-env-xdg-config-home", TRUE);
va_start(ap, func);
while ((lib = va_arg(ap, const char *))) {
--
2.39.2
[libvirt PATCH 02/14] tests: Drop unnecessary configuration overrides
They are not used by the specific test.
Signed-off-by: Andrea Bolognani
---
tests/qemuhotplugtest.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 2e8ac05e5e..6e3d4dd807 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -673,11 +673,6 @@ mymain(void)
virEventRegisterDefaultImpl();
-VIR_FREE(driver.config->spiceListen);
-VIR_FREE(driver.config->vncListen);
-/* some dummy values from 'config file' */
-driver.config->spicePassword = g_strdup("123456");
-
if (!(driver.domainEventState = virObjectEventStateNew()))
return EXIT_FAILURE;
--
2.39.2
[libvirt PATCH 00/14] tests: Improve QEMU driver config handling
Move more settings to common code and more closely match real-world configurations. Andrea Bolognani (14): tests: Poison more XDG variables tests: Drop unnecessary configuration overrides tests: Drop unnecessary free tests: Increase scope for SASLdirs tests: Limit use of SPICE TLS tests: Set the QEMU driver as privileged in common code tests: Create privileged config for QEMU driver tests: Drop no longer necessary overrides tests: Add convenience variable for QEMU driver config tests: Move more QEMU driver settings to common code tests: Set more TLSx509certdirs tests: Set TLSx509certdirs to default values tests: Set SASLdirs to default values tests: Drop more QEMU driver config overrides tests/qemuhotplugtest.c | 5 -- .../qemuhotplug-qemu-agent-detach.xml | 2 +- .../qemuhotplug-base+qemu-agent.xml | 2 +- ...uhotplug-base-live+disk-scsi-multipath.xml | 2 +- .../qemuhotplug-base-live+qemu-agent.xml | 2 +- tests/qemumemlocktest.c | 2 - tests/qemumigrationcookiexmltest.c| 2 - .../blockjob-mirror-in.xml| 2 +- .../qemustatusxml2xmldata/vcpus-multi-in.xml | 2 +- tests/qemustatusxml2xmltest.c | 2 - .../aarch64-aavmf-virtio-mmio.args| 10 ++-- .../aarch64-cpu-passthrough.args | 10 ++-- ...fault-cpu-kvm-virt-4.2.aarch64-latest.args | 10 ++-- ...fault-cpu-tcg-virt-4.2.aarch64-latest.args | 10 ++-- .../aarch64-features-sve.aarch64-latest.args | 10 ++-- tests/qemuxml2argvdata/aarch64-gic-host.args | 10 ++-- .../aarch64-gic-none-tcg.args | 10 ++-- tests/qemuxml2argvdata/aarch64-gic-v2.args| 10 ++-- tests/qemuxml2argvdata/aarch64-gic-v3.args| 10 ++-- .../aarch64-kvm-32-on-64.args | 10 ++-- .../qemuxml2argvdata/aarch64-pci-serial.args | 10 ++-- .../aarch64-tpm.aarch64-latest.args | 10 ++-- .../aarch64-traditional-pci.args | 10 ++-- .../aarch64-usb-controller-nec-xhci.args | 10 ++-- .../aarch64-usb-controller-qemu-xhci.args | 10 ++-- .../aarch64-video-default.args| 10 ++-- .../aarch64-video-virtio-gpu-pci.args | 10 ++-- .../aarch64-virt-2.6-virtio-pci-default.args | 10 ++-- .../aarch64-virt-default-nic.args | 10 ++-- .../aarch64-virt-graphics.aarch64-latest.args | 10 ++-- .../aarch64-virt-headless.aarch64-latest.args | 10 ++-- .../qemuxml2argvdata/aarch64-virt-virtio.args | 10 ++-- ...ch64-virtio-pci-default.aarch64-4.2.0.args | 10 ++-- ...h64-virtio-pci-default.aarch64-latest.args | 10 ++-- .../aarch64-virtio-pci-manual-addresses.args | 10 ++-- tests/qemuxml2argvdata/acpi-table.args| 10 ++-- .../arm-vexpressa9-basic.args | 10 ++-- .../arm-vexpressa9-nodevs.args| 10 ++-- .../arm-vexpressa9-virtio.args| 10 ++-- tests/qemuxml2argvdata/arm-virt-virtio.args | 10 ++-- .../audio-alsa-best.x86_64-latest.args| 10 ++-- .../audio-alsa-full.x86_64-latest.args| 10 ++-- .../audio-alsa-minimal.x86_64-latest.args | 10 ++-- .../audio-coreaudio-best.x86_64-latest.args | 10 ++-- .../audio-coreaudio-full.x86_64-latest.args | 10 ++-- ...audio-coreaudio-minimal.x86_64-latest.args | 10 ++-- ...udio-default-nographics.x86_64-latest.args | 10 ++-- tests/qemuxml2argvdata/audio-default-sdl.args | 10 ++-- .../audio-default-sdl.x86_64-latest.args | 10 ++-- .../qemuxml2argvdata/audio-default-spice.args | 10 ++-- .../audio-default-spice.x86_64-latest.args| 10 ++-- tests/qemuxml2argvdata/audio-default-vnc.args | 10 ++-- .../audio-default-vnc.x86_64-latest.args | 10 ++-- .../audio-file-best.x86_64-latest.args| 10 ++-- .../audio-file-full.x86_64-latest.args| 10 ++-- .../audio-file-minimal.x86_64-latest.args | 10 ++-- .../audio-jack-full.x86_64-latest.args| 10 ++-- .../audio-jack-minimal.x86_64-latest.args | 10 ++-- .../audio-many-backends.x86_64-latest.args| 10 ++-- .../audio-none-best.x86_64-latest.args| 10 ++-- .../audio-none-full.x86_64-latest.args| 10 ++-- .../audio-none-minimal.x86_64-latest.args | 10 ++-- .../audio-oss-best.x86_64-latest.args | 10 ++-- .../audio-oss-full.x86_64-latest.args | 10 ++-- .../audio-oss-minimal.x86_64-latest.args | 10 ++-- .../audio-pulseaudio-best.x86_64-latest.args | 10 ++-- .../audio-pulseaudio-full.x86_64-latest.args | 10 ++-- ...udio-pulseaudio-minimal.x86_64-latest.args | 10 ++-- .../audio-sdl-best.x86_64-latest.args | 10 ++-- .../audio-sdl-full.x86_64-latest.args | 10 ++-- .../audio-sdl-minimal.x86_64-latest.args | 10 ++-- .../audio-spice-best.x86_64-latest.args | 10 ++-- .../audio-spice-full.x86_64-latest.args | 10 ++-- .../audio-spice-minimal.x86_64-latest.args| 10 ++-- tests/qemuxml2argvdata/autoindex.args | 10 ++-- .../balloon-ccw
Re: passt SELinux labelling (was: Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start)
On Fri, Mar 03, 2023 at 06:06:05PM +, Daniel P. Berrangé wrote: > On Fri, Mar 03, 2023 at 09:56:55AM -0800, Andrea Bolognani wrote: > > Right, but wouldn't the idea of poking at the filesystem to retrieve > > the label from the binary (passt_exec_t) and then applying a text > > transformation to obtain the runtime label (passt_t) go directly > > against the idea of not hardcoding information about a specific > > policy implementation into libvirt? > > I'm not suggesting applying a text transformation. The example code > using libselinux I described in the other reply actually askes the > kernel to tell us what the target type will be when a process > labelled passt_exec_t is execd. Yeah, that's a lot better. > > As I understand it, such a policy would allow virtqemud (virtd_t) to > > execute passt (passt_exec_t) and automatically result in a transition > > of the process to the desired context (passt_t). > > Yes, and I'm saying we must ask the kernel to tell us what that target > context will be for the loaded policy, given the source file context. I still don't understand why we can't simply execute passt and let the domain transition defined in the policy take care of switching to the appropriate label from us, like we do for dnsmasq and other tools? Why do we need to do things differently for passt? -- Andrea Bolognani / Red Hat / Virtualization
Re: passt SELinux labelling (was: Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start)
On Fri, Mar 03, 2023 at 05:15:43PM +, Daniel P. Berrangé wrote: > On Fri, Mar 03, 2023 at 09:06:38AM -0800, Andrea Bolognani wrote: > > > > Since we know that we're launching passt and not some other random > > > > helper, why can't we simply use passt_t directly here? It feels like > > > > that would be less prone to issues caused by accidental (or > > > > intentional) misconfigurations. > > > > > > That ties libvirt's code to a specific policy impl which is > > > not a desirable thing. Same reason we don't hardcode svirt_t > > > as a type for QEMU, but instead query it dynamically from > > > the installed policy. > > > > Do I understand correctly that this happens in > > virSecuritySELinuxQEMUInitialize(), by parsing the contents of the > > file located via a call to selinux_virtual_domain_context_path()? > > Yes. > > > Poking around at the other files present in the same directory I see > > various formats being used, including... XML? It looks like SELinux > > implements facilities for exposing arbitrary information about the > > active policy at well-known locations, with (I assume) the explicit > > purpose of enabling this kind of interaction. > > > > So wouldn't that be the way to go for passt, and other helpers too? > > Have SELinux expose a virtual_helpers_context file, that we can parse > > to figure out the appropriate labels to use for passt and friends? > > No, I don't think so. The helpers file is a bit of a special case > that was needed because there were multiple contexts we needed to > cope with for running QEMU. > > I don't see any reason not to follow what the kernel already does > by relying on the labelled file context. Right, but wouldn't the idea of poking at the filesystem to retrieve the label from the binary (passt_exec_t) and then applying a text transformation to obtain the runtime label (passt_t) go directly against the idea of not hardcoding information about a specific policy implementation into libvirt? Looking at the SELinux policy for Fedora, the virt module contains a bunch of instances where an external policy is imported in order to give virtqemud the ability to run external commands. For example, for dnsmasq we have optional_policy(` dnsmasq_domtrans(virtd_t) dnsmasq_signal(virtd_t) dnsmasq_kill(virtd_t) dnsmasq_signull(virtd_t) dnsmasq_create_pid_dirs(virtd_t) dnsmasq_filetrans_named_content_fromdir(virtd_t, virt_var_run_t); dnsmasq_manage_pid_files(virtd_t) ') This looks pretty similar to what Stefano has proposed doing for passt in [1]: optional_policy(` passt_socket(virt_domain) ') optional_policy(` passt_domtrans(virtd_t) passt_kill(virtd_t) ') As I understand it, such a policy would allow virtqemud (virtd_t) to execute passt (passt_exec_t) and automatically result in a transition of the process to the desired context (passt_t). Why would this approach not be okay? [1] https://github.com/fedora-selinux/selinux-policy/pull/1613 -- Andrea Bolognani / Red Hat / Virtualization
Re: passt SELinux labelling (was: Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start)
On Fri, Mar 03, 2023 at 03:47:23PM +, Daniel P. Berrangé wrote: > On Fri, Mar 03, 2023 at 07:23:41AM -0800, Andrea Bolognani wrote: > > I'm in no way a SELinux expert, but the idea of figuring out the > > runtime label for the process based on information found on the > > filesystem makes me uncomfortable. The idea of using some sort of > > text transformation to get from one to the other, even more so. > > Using the label on the filesystem is precisely the right way to > do this with SELinux. It is what the kernel does every time a > binary is invokved, unless the caller has overriden the target > type. > > > Since we know that we're launching passt and not some other random > > helper, why can't we simply use passt_t directly here? It feels like > > that would be less prone to issues caused by accidental (or > > intentional) misconfigurations. > > That ties libvirt's code to a specific policy impl which is > not a desirable thing. Same reason we don't hardcode svirt_t > as a type for QEMU, but instead query it dynamically from > the installed policy. Do I understand correctly that this happens in virSecuritySELinuxQEMUInitialize(), by parsing the contents of the file located via a call to selinux_virtual_domain_context_path()? Poking around at the other files present in the same directory I see various formats being used, including... XML? It looks like SELinux implements facilities for exposing arbitrary information about the active policy at well-known locations, with (I assume) the explicit purpose of enabling this kind of interaction. So wouldn't that be the way to go for passt, and other helpers too? Have SELinux expose a virtual_helpers_context file, that we can parse to figure out the appropriate labels to use for passt and friends? -- Andrea Bolognani / Red Hat / Virtualization
Re: passt SELinux labelling (was: Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start)
On Fri, Mar 03, 2023 at 10:03:02AM -0500, Laine Stump wrote: > On 2/23/23 5:47 AM, Daniel P. Berrangé wrote: > > This really isn't difficult to do in the security manager IMHO. It is > > just a variation on the existing virSecurityManagerSetChildProcessLabel > > method, which instead of using the standard QEMU svirt_t labels, queries > > the label by calling getfilecon on the binary to be execd and appending > > the MCS label. > > It seems it's not as simple as this. > > I have an implementation of this, available at > > https://gitlab.com/lainestump/libvirt/-/commits/active-passt-6 > > The problem with it is that it doesn't end up setting the label to passt_t. > Instead, it sets it to passt_exec_t. My understanding is that, similar to > many other binaries (e.g. dnsmasq), the context type of the binary file is > passt_exec_t, and the rules in the policies use that as an indicator to > transition the process to passt_t. > > Here's the results of a few different relabelling strategies: > > In the case of Stefano's patch where libvirt's relabelling is completely > removed (it just calls virCommandRun() directly), the passt process looks > like this: > > unconfined_u:unconfined_r:passt_t:s0-s0:c0.c1023 97132 > > When I allow libvirt to do its normal relabelling, it ends up like this: > > unconfined_u:unconfined_r:svirt_t:s0:c239,c560 104213 > > When I run it with my patch (which calls getfilecon, then adds the MCS), it > looks like this: > > system_u:object_r:passt_exec_t:s0:c239,c560 > > Am I correct to think that what everyone is suggesting is that the process > ends up like this? (it's what would happen with Stefano's hard-coded hack > that this message is replying to): > > unconfined_u:unconfined_r:passt_t:s0:c239,c560 104213 > > If so, how are we to derive "passt_t" from "passt_exec_t", since both those > names are arbritrary, and some other selinux policy for some other binary > might choose names that don't simply differ by having "_exec" added/removed. > Is it possible to get the label of the process after it has been forked, and > grab the context type from there? I'm in no way a SELinux expert, but the idea of figuring out the runtime label for the process based on information found on the filesystem makes me uncomfortable. The idea of using some sort of text transformation to get from one to the other, even more so. Since we know that we're launching passt and not some other random helper, why can't we simply use passt_t directly here? It feels like that would be less prone to issues caused by accidental (or intentional) misconfigurations. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 10/33] qemu: Introduce qemuDomainDefBootPostParse()
On Thu, Mar 02, 2023 at 04:59:36PM +0100, Michal Prívozník wrote:
> On 2/15/23 11:42, Andrea Bolognani wrote:
> > +static int
> > +qemuDomainDefBootPostParse(virDomainDef *def,
> > + virQEMUDriverConfig *cfg)
> > +{
> > +if (def->os.bootloader || def->os.bootloaderArgs) {
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > + _("bootloader is not supported by QEMU"));
> > +return -1;
> > +}
>
> Strictly speaking, this is a validate check. Haven't look into the
> future, but feel free to move it there in a follow up patch.
Right. I'll look into it.
In the meantime, I've pushed the series. Thanks a ton for the review!
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] security: Add support for SUSE edk2 firmware paths
On Thu, Feb 23, 2023 at 11:13:28AM -0700, Jim Fehlig wrote: > +++ b/src/security/apparmor/libvirt-qemu > @@ -91,7 +91,7 @@ >/usr/share/proll/** r, >/usr/share/qemu-efi/** r, >/usr/share/qemu-kvm/** r, > - /usr/share/qemu/** r, > + /usr/share/qemu/** rk, >/usr/share/seabios/** r, >/usr/share/sgabios/** r, >/usr/share/slof/** r, > +++ b/src/security/virt-aa-helper.c > @@ -481,6 +481,7 @@ valid_path(const char *path, const bool readonly) > "/usr/share/AAVMF/", /* for AAVMF images */ > "/usr/share/qemu-efi/", /* for AAVMF images */ > "/usr/share/qemu-efi-aarch64/", /* for AAVMF images */ > +"/usr/share/qemu/", /* SUSE path for OVMF and AAVMF > images */ > "/usr/lib/u-boot/", /* u-boot loaders for qemu */ > "/usr/lib/riscv64-linux-gnu/opensbi" /* RISC-V SBI implementation */ Having these files in /usr/share/qemu directly looks... Kinda sketchy? That directory should belong to the QEMU package. Compare with how all the other paths listed here point to directories that are specific to the firmware at hand. I don't think this really opens up any attack vectors, so Reviewed-by: Andrea Bolognani but perhaps it would be a good idea to consider migrating edk2 images to their own directory long term? -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] qemu: Remove duplicate user/group lookup
Commit 068efae5b1a9 created a copy of this code instead of
simply moving it.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_conf.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a831783d75..680832742c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -173,12 +173,6 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool
privileged,
cfg->memoryBackingDir = g_strdup_printf("%s/ram", cfg->libDir);
cfg->swtpmStorageDir = g_strdup_printf("%s/lib/libvirt/swtpm",
LOCALSTATEDIR);
-if (!virDoesUserExist("tss") ||
-virGetUserID("tss", >swtpm_user) < 0)
-cfg->swtpm_user = 0; /* fall back to root */
-if (!virDoesGroupExist("tss") ||
-virGetGroupID("tss", >swtpm_group) < 0)
-cfg->swtpm_group = 0; /* fall back to root */
} else {
g_autofree char *rundir = NULL;
g_autofree char *cachedir = NULL;
--
2.39.2
[PATCH] qemu: Align arguments correctly
Signed-off-by: Andrea Bolognani
---
Pushed as trivial.
src/qemu/qemu_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 16d52cbbd4..a831783d75 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -98,7 +98,7 @@ VIR_ONCE_GLOBAL_INIT(virQEMUConfig);
virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
- const char *root)
+const char *root)
{
g_autoptr(virQEMUDriverConfig) cfg = NULL;
--
2.39.2
[libvirt PATCH 2/4] tests: Adopt fakerootdir helpers
Most replacements are completely straightforward but
vircgrouptest requires slightly different handling because,
instead of initializing a single fakerootdir at the start of
the test program and cleaning it up at the end, it creates
multiple different ones one after the other.
Signed-off-by: Andrea Bolognani
---
tests/qemuhotplugtest.c | 14 +++---
tests/qemumemlocktest.c | 15 +++
tests/qemustatusxml2xmltest.c | 15 +++
tests/qemuxml2argvtest.c | 15 +++
tests/qemuxml2xmltest.c | 16 +++-
tests/scsihosttest.c | 13 +++--
tests/vircgrouptest.c | 18 --
tests/virhostdevtest.c| 17 -
tests/virpcitest.c| 15 +++
9 files changed, 29 insertions(+), 109 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 215837fc8f..429928aa7f 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -656,8 +656,6 @@ testQemuHotplugCpuIndividual(const void *opaque)
return ret;
}
-#define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XX"
-
static int
mymain(void)
@@ -669,15 +667,10 @@ mymain(void)
g_autofree char *fakerootdir = NULL;
g_autoptr(virQEMUDriverConfig) cfg = NULL;
-fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE);
-
-if (!g_mkdtemp(fakerootdir)) {
-fprintf(stderr, "Cannot create fakerootdir");
-abort();
+if (!(fakerootdir = virTestFakeRootDirInit())) {
+return EXIT_FAILURE;
}
-g_setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, TRUE);
-
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
@@ -1004,8 +997,7 @@ mymain(void)
DO_TEST_CPU_INDIVIDUAL("ppc64-modern-individual", "16-22", true, true,
true);
DO_TEST_CPU_INDIVIDUAL("ppc64-modern-individual", "17", true, true, true);
-if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
-virFileDeleteTree(fakerootdir);
+virTestFakeRootDirCleanup(fakerootdir);
qemuTestDriverFree();
virObjectUnref(data.vm);
diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c
index b303f70e9d..184d8ede19 100644
--- a/tests/qemumemlocktest.c
+++ b/tests/qemumemlocktest.c
@@ -42,8 +42,6 @@ testCompareMemLock(const void *data)
return virTestCompareToULL(info->memlock,
qemuDomainGetMemLockLimitBytes(def, false));
}
-# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XX"
-
static int
mymain(void)
{
@@ -51,14 +49,8 @@ mymain(void)
g_autofree char *fakerootdir = NULL;
g_autoptr(virQEMUCaps) qemuCaps = NULL;
-fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE);
-
-if (!g_mkdtemp(fakerootdir)) {
-fprintf(stderr, "Cannot create fakerootdir");
-abort();
-}
-
-g_setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, TRUE);
+if (!(fakerootdir = virTestFakeRootDirInit()))
+return EXIT_FAILURE;
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
@@ -137,8 +129,7 @@ mymain(void)
DO_TEST("pseries-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED);
cleanup:
-if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
-virFileDeleteTree(fakerootdir);
+virTestFakeRootDirCleanup(fakerootdir);
qemuTestDriverFree();
diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c
index db82a1a980..a29aa723e6 100644
--- a/tests/qemustatusxml2xmltest.c
+++ b/tests/qemustatusxml2xmltest.c
@@ -70,8 +70,6 @@ testInfoSetStatusPaths(struct testQemuInfo *info)
}
-#define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XX"
-
static int
mymain(void)
{
@@ -87,14 +85,8 @@ mymain(void)
if (!capslatest)
return EXIT_FAILURE;
-fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE);
-
-if (!g_mkdtemp(fakerootdir)) {
-fprintf(stderr, "Cannot create fakerootdir");
-abort();
-}
-
-g_setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, TRUE);
+if (!(fakerootdir = virTestFakeRootDirInit()))
+return EXIT_FAILURE;
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
@@ -142,8 +134,7 @@ mymain(void)
DO_TEST_STATUS("backup-pull");
cleanup:
-if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
-virFileDeleteTree(fakerootdir);
+virTestFakeRootDirCleanup(fakerootdir);
qemuTestDriverFree();
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 5a33c336c8..39d1888427 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -796,8 +796,6 @@ testInfoSetPaths(struct testQemuInfo *info,
abs_srcdir, info->name, suffix ? suffix
: "");
}
-# define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XX"
-
static int
mymain(void)
{
@@ -813,14 +
[libvirt PATCH 4/4] tests: Print fakerootdir when it's preserved
Setting the LIBVIRT_SKIP_CLEANUP environment variable results
in the contents of fakerootdir being preserved for inspection.
Be more helpful towards the developer and print out the path
in this case.
Signed-off-by: Andrea Bolognani
---
tests/testutils.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/testutils.c b/tests/testutils.c
index e717895fbf..e8cb8e6737 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -790,6 +790,8 @@ virTestFakeRootDirCleanup(char *fakerootdir)
if (!g_getenv("LIBVIRT_SKIP_CLEANUP"))
virFileDeleteTree(fakerootdir);
+else
+fprintf(stderr, "Test data ready for inspection: %s\n", fakerootdir);
}
int virTestMain(int argc,
--
2.39.2
[libvirt PATCH 0/4] tests: Improve fakerootdir handling
I'd be open to squash patches 2 and 3 together, if that's considered preferable by the reviewer. Andrea Bolognani (4): tests: Introduce helpers for fakerootdir handling tests: Adopt fakerootdir helpers tests: Move fakerootdir handling to common logic tests: Print fakerootdir when it's preserved tests/qemuhotplugtest.c | 15 --- tests/qemumemlocktest.c | 15 --- tests/qemustatusxml2xmltest.c | 15 --- tests/qemuxml2argvtest.c | 15 --- tests/qemuxml2xmltest.c | 16 tests/scsihosttest.c | 14 +++--- tests/testutils.c | 34 ++ tests/testutils.h | 3 +++ tests/vircgrouptest.c | 18 -- tests/virhostdevtest.c| 16 tests/virpcitest.c| 15 --- 11 files changed, 44 insertions(+), 132 deletions(-) -- 2.39.2
[libvirt PATCH 3/4] tests: Move fakerootdir handling to common logic
Instead of having each test manually initialize and cleanup
its own fakerootdir, do that as part of the common test
initialization logic in virTestMain().
In most cases we can simply drop the relevant code from the
test program, but scsihosttest uses the value of fakerootdir
as a starting point to build another path, so we need to do
things slightly differently. In order to keep things working,
we retrieve the value from the LIBVIRT_FAKE_ROOT_DIR
environment variable, same as all the mock libraries are
already doing.
Signed-off-by: Andrea Bolognani
---
tests/qemuhotplugtest.c | 7 ---
tests/qemumemlocktest.c | 6 --
tests/qemustatusxml2xmltest.c | 6 --
tests/qemuxml2argvtest.c | 6 --
tests/qemuxml2xmltest.c | 6 --
tests/scsihosttest.c | 5 ++---
tests/testutils.c | 6 ++
tests/virhostdevtest.c| 7 ---
tests/virpcitest.c| 6 --
9 files changed, 8 insertions(+), 47 deletions(-)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 429928aa7f..2e8ac05e5e 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -664,13 +664,8 @@ mymain(void)
int ret = 0;
struct qemuHotplugTestData data = {0};
struct testQemuHotplugCpuParams cpudata;
-g_autofree char *fakerootdir = NULL;
g_autoptr(virQEMUDriverConfig) cfg = NULL;
-if (!(fakerootdir = virTestFakeRootDirInit())) {
-return EXIT_FAILURE;
-}
-
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
@@ -997,8 +992,6 @@ mymain(void)
DO_TEST_CPU_INDIVIDUAL("ppc64-modern-individual", "16-22", true, true,
true);
DO_TEST_CPU_INDIVIDUAL("ppc64-modern-individual", "17", true, true, true);
-virTestFakeRootDirCleanup(fakerootdir);
-
qemuTestDriverFree();
virObjectUnref(data.vm);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c
index 184d8ede19..a03ed20a45 100644
--- a/tests/qemumemlocktest.c
+++ b/tests/qemumemlocktest.c
@@ -46,12 +46,8 @@ static int
mymain(void)
{
int ret = 0;
-g_autofree char *fakerootdir = NULL;
g_autoptr(virQEMUCaps) qemuCaps = NULL;
-if (!(fakerootdir = virTestFakeRootDirInit()))
-return EXIT_FAILURE;
-
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
@@ -129,8 +125,6 @@ mymain(void)
DO_TEST("pseries-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED);
cleanup:
-virTestFakeRootDirCleanup(fakerootdir);
-
qemuTestDriverFree();
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
diff --git a/tests/qemustatusxml2xmltest.c b/tests/qemustatusxml2xmltest.c
index a29aa723e6..7587c4c34f 100644
--- a/tests/qemustatusxml2xmltest.c
+++ b/tests/qemustatusxml2xmltest.c
@@ -74,7 +74,6 @@ static int
mymain(void)
{
int ret = 0;
-g_autofree char *fakerootdir = NULL;
g_autoptr(GHashTable) capslatest = testQemuGetLatestCaps();
g_autoptr(GHashTable) capscache = virHashNew(virObjectUnref);
g_autoptr(virConnect) conn = NULL;
@@ -85,9 +84,6 @@ mymain(void)
if (!capslatest)
return EXIT_FAILURE;
-if (!(fakerootdir = virTestFakeRootDirInit()))
-return EXIT_FAILURE;
-
if (qemuTestDriverInit() < 0)
return EXIT_FAILURE;
@@ -134,8 +130,6 @@ mymain(void)
DO_TEST_STATUS("backup-pull");
cleanup:
-virTestFakeRootDirCleanup(fakerootdir);
-
qemuTestDriverFree();
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 39d1888427..ad4d6aa4d2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -800,7 +800,6 @@ static int
mymain(void)
{
int ret = 0;
-g_autofree char *fakerootdir = NULL;
g_autoptr(GHashTable) capslatest = testQemuGetLatestCaps();
g_autoptr(GHashTable) qapiSchemaCache = virHashNew((GDestroyNotify)
g_hash_table_unref);
g_autoptr(GHashTable) capscache = virHashNew(virObjectUnref);
@@ -811,9 +810,6 @@ mymain(void)
if (!capslatest)
return EXIT_FAILURE;
-if (!(fakerootdir = virTestFakeRootDirInit()))
-return EXIT_FAILURE;
-
/* Set the timezone because we are mocking the time() function.
* If we don't do that, then localtime() may return unpredictable
* results. In order to detect things that just work by a blind
@@ -2984,8 +2980,6 @@ mymain(void)
DO_TEST_CAPS_LATEST("crypto-builtin");
-virTestFakeRootDirCleanup(fakerootdir);
-
VIR_FREE(driver.config->nbdTLSx509certdir);
qemuTestDriverFree();
virFileWrapperClearPrefixes();
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 3ae2e65a43..88e152b65c 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -117,7 +117,6 @@ static int
mymain(void)
{
int ret = 0;
-g_autofree char *fakerootdir = NULL;
[libvirt PATCH 1/4] tests: Introduce helpers for fakerootdir handling
We have this logic open-coded all over the test suite. Provide
proper helpers implementing it.
Signed-off-by: Andrea Bolognani
---
tests/testutils.c | 26 ++
tests/testutils.h | 3 +++
2 files changed, 29 insertions(+)
diff --git a/tests/testutils.c b/tests/testutils.c
index 8fe624f029..fe7677ebfb 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -766,6 +766,32 @@ virTestSetEnvPath(void)
return 0;
}
+#define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XX"
+
+char*
+virTestFakeRootDirInit(void)
+{
+g_autofree char *fakerootdir = g_strdup(FAKEROOTDIRTEMPLATE);
+
+if (!g_mkdtemp(fakerootdir)) {
+fprintf(stderr, "Cannot create fakerootdir");
+return NULL;
+}
+
+g_setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, TRUE);
+
+return g_steal_pointer();
+}
+
+void
+virTestFakeRootDirCleanup(char *fakerootdir)
+{
+g_unsetenv("LIBVIRT_FAKE_ROOT_DIR");
+
+if (!g_getenv("LIBVIRT_SKIP_CLEANUP"))
+virFileDeleteTree(fakerootdir);
+}
+
int virTestMain(int argc,
char **argv,
int (*func)(void),
diff --git a/tests/testutils.h b/tests/testutils.h
index 869b58bd1e..cf8a346dff 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -101,6 +101,9 @@ void virTestQuiesceLibvirtErrors(bool always);
void virTestCounterReset(const char *prefix);
const char *virTestCounterNext(void);
+char *virTestFakeRootDirInit(void);
+void virTestFakeRootDirCleanup(char *fakerootdir);
+
/**
* The @func shall return EXIT_FAILURE or EXIT_SUCCESS or
* EXIT_AM_SKIP or EXIT_AM_HARDFAIL.
--
2.39.2
Re: [libvirt PATCH] NEWS: Clarify limitations of passt support
On Tue, Feb 28, 2023 at 11:28:30PM -0500, Laine Stump wrote: > On 2/28/23 4:34 PM, Andrea Bolognani wrote: > > Let users know that we're working on lifting the limitations > > and that they should not use the feature in production until > > then. > > > > Signed-off-by: Andrea Bolognani > > Reviewed-by: Laine Stump > > I think I like your version better, as it is less wordy, while also pointing > out that this is something temporary that we are actively working to fix. I didn't realize you already had posted a version before coming up with mine :/ Anyway, pushed now. -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] NEWS: Clarify limitations of passt support
Let users know that we're working on lifting the limitations and that they should not use the feature in production until then. Signed-off-by: Andrea Bolognani --- NEWS.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS.rst b/NEWS.rst index 683c147af0..b19c75ceac 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -79,7 +79,9 @@ v9.1.0 (unreleased) connectivity. (NB: On systems that use them, it is still necessary to disable -SELinux/AppArmor to start passt.) +SELinux/AppArmor to start passt. This is a temporary limitation, +and use of the feature in production is strongly discouraged +until it has been lifted.) * qemu: Fix error when attempting to change media in a CDROM drive -- 2.39.2
Re: [PATCH] NEWS: note new passt feature & bugfix for 9.1.0 release
On Tue, Feb 28, 2023 at 07:53:09PM +0100, Stefano Brivio wrote: > On Tue, 28 Feb 2023 10:06:18 -0800 Andrea Bolognani > wrote: > > On Tue, Feb 28, 2023 at 09:49:26AM -0500, Laine Stump wrote: > > > +(NB: it is still necessary to disable SELinux to start passt.) > > > > This is also true for AppArmor, so I would mention both. > > Not in general -- thankfully, no pseudorandom label is forced by > libvirt 9.1.0 with AppArmor (because there are no labels), and libvirtd > simply runs passt unconfined (scrubbing the environment): > > $ grep "/usr/bin" src/security/apparmor/usr.sbin.libvirtd.in > /usr/bin/* PUx, > > Then yes, with any recent version of Debian and openSUSE packages of > passt, passt won't be able to create the socket or its PID file in the > path libvirt asks for, because of the profile shipping with passt > itself. >From the user's point of view, what is the difference between passt not being able to start, or starting successfully but quitting immediately afterwards because it can't create some files? I don't think there's one. In both cases, you're going to see an error. > Note that I'm *not* recommending to do this, just like I'm not > recommending to disable SELinux, and I don't think it's a good idea to > suggest in release notes that users do this, either. This is a limitation of the current implementation of passt support in libvirt. We're actively working on removing it, but in the meantime it should be documented somewhere. Are the release notes the best place for that? Unclear. I don't think it's a particularly bad one. Anyone reading "you need to disable SELinux to use this feature" will surely infer that they shouldn't put it into production yet :) -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] NEWS: Mention two user-visible bug fixes
On Tue, Feb 28, 2023 at 05:23:00PM +0100, Peter Krempa wrote: > + * qemu: Fix error when attempting to change media in a CDROM drive > + > +Due to a logic bug introduced in libvirt-9.0 attempts to change media in > a > +CDROM would previously fail with an error stating that the tray isn't > open. > + > + * qemu: Properly handle block job transitions > + > +Starting with libvirt-9.0 the block job state machine improperly handled > +some job transitions which resulted into some block jobs not being > properly > +terminated. This could cause problems such as errors when detaching a > disk > +after snapshot. Maybe s/transitions which/transitions, which/ ? Either way, make sure that we still have *two* empty lines between sections after your changes have been applied. Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] NEWS: note new passt feature & bugfix for 9.1.0 release
On Tue, Feb 28, 2023 at 09:49:26AM -0500, Laine Stump wrote: > + * QEMU: properly report passt startup errors > + > +Due to how the child passt process was started, the initial > +support for passt (added in 9.0.0) would not see errors > +encountered during startup, so libvirt would continue to setup and > +start the guest; this led to a running guest with no network > +connectivity. This issue has be corrected. > + > +(NB: it is still necessary to disable SELinux to start passt.) This is also true for AppArmor, so I would mention both. Also, please make sure that there still are *two* empty lines between the section for v9.1.0 and the one for v9.0.0. The current version of the patch drops one of them. With the tweaks mentioned above, plus the stuff already pointed out by Peter, Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 1/7] qemu: capabilities: Introduce QEMU_CAPS_MACHINE_ACPI
On Mon, Feb 27, 2023 at 06:25:23PM +0100, Peter Krempa wrote:
> On Mon, Feb 27, 2023 at 08:44:57 -0800, Andrea Bolognani wrote:
> > This looks like you're checking whether -acpi itself exists as a
> > top-level option. Which it doesn't, but -no-acpi does and yet it
> > doesn't seem to be advertised in the output of
> > query-command-line-options?
>
> Well, it actually does exist in the output of
> query-command-line-options, but I have no idea what it means:
>
> virsh qemu-monitor-command --pretty cd query-command-line-options | jq
> .return[].option
>
> One of the options is "acpi".
Right, I've seen that. What I wanted to say if that passing it to
QEMU results in an error:
$ qemu-system-x86_64 -acpi
qemu-system-x86_64: -acpi: invalid option
So it's not really an option, despite being advertised as such. And
on the opposite end of the spectrum we have -no-acpi, which *does*
work when passed to QEMU but is nowhere to be found in the JSON
document returned by query-command-line-options.
> > Basically it looks like there are some serious introspection
> > shenanigans going on, and I'm not sure we can actually reliably
> > detect whether -machine acpi can be used until your QEMU patch has
> > been merged.
> >
> > Or I might just have missed something obvious! In which case, please
> > let me know what it is :)
>
> I have no idea what the 'acpi' option does but it certainly mislead me.
I'm not entirely sure, but I think it might be connected to this
bit of code in vl.c:
case QEMU_OPTION_acpitable:
opts = qemu_opts_parse_noisily(qemu_find_opts("acpi"),
optarg, true);
This is the handling for the -acpitable option, but notice how "acpi"
is mentioned as well, to perform some sort of lookup. I think this
might be the reason why "acpi" gets included in the JSON, while
"acpitable" doesn't.
Another example I've found is "smp-opts", which seems to be used to
implement the -smp option. Once again, in the JSON we find "smp-opts"
instead of "smp".
I think it would be worthwile to check with the QEMU developers and
make sure that they're aware of this behavior. Is it intended? Is it
documented anywhere? It certainly seems extremely confusing to me.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 1/7] qemu: capabilities: Introduce QEMU_CAPS_MACHINE_ACPI
On Mon, Feb 27, 2023 at 04:27:37PM +0100, Peter Krempa wrote:
> Detect that ACPI can be controlled via '-machine acpi=off' rather than
> use of -no-acpi.
First of all, thank you for picking up this work. I was planning on
doing this myself, but you've beaten me to it and I'm absolutely
ecstatic about that :)
> +++ b/src/qemu/qemu_capabilities.c
> static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
> +{ "acpi", NULL, QEMU_CAPS_MACHINE_ACPI },
> { "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
> { "machine", "hpet", QEMU_CAPS_MACHINE_HPET },
> { "sandbox", NULL, QEMU_CAPS_SECCOMP_SANDBOX },
>
> +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml
>
>
>
> +
Are you sure you're detecting this correctly? Based on an earlier
discussion about this topic[1], I would have expected the flag to
show up for QEMU 5.0.0, but *not* for QEMU 4.2.0.
This looks like you're checking whether -acpi itself exists as a
top-level option. Which it doesn't, but -no-acpi does and yet it
doesn't seem to be advertised in the output of
query-command-line-options?
Basically it looks like there are some serious introspection
shenanigans going on, and I'm not sure we can actually reliably
detect whether -machine acpi can be used until your QEMU patch has
been merged.
Or I might just have missed something obvious! In which case, please
let me know what it is :)
[1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg02217.html
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] NEWS: Document new pvpanic-pci device
On Tue, Feb 21, 2023 at 05:37:52PM +0100, Kristina Hanicova wrote: > Signed-off-by: Kristina Hanicova > --- > NEWS.rst | 5 + > 1 file changed, 5 insertions(+) Reviewed-by: Andrea Bolognani and pushed along with the patches implementing the feature. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v2 0/6] add support for pvpanic-pci device
On Mon, Feb 20, 2023 at 05:12:53PM +0100, Kristina Hanicova wrote: > v1 here: > https://listman.redhat.com/archives/libvir-list/2023-February/237622.html > > diff to v1: > * reduced test files (thanks Andrea) > * removed redundant check for address type (noticed by Peter) > * plugging the device directly into pcie.0 if its address was not > specified Just for completeness' sake, note that the use of VIR_PCI_CONNECT_INTEGRATED results in libvirt outright rejecting attempts to put the device anywhere but on pcie.0, even when the address comes directly from the user. This is technically a limitation compared to what QEMU allows, but in practice the other configurations are untested and overall it feels like a fair trade-off. We can also decide to lift this limitation in the future, if it ever comes to that. > Kristina Hanicova (6): > qemu: introduce QEMU_CAPS_DEVICE_PANIC_PCI > conf: add panic model 'pvpanic' > tests: add test cases for device pvpanic-pci > qemu: assign PCI address to device pvpanic-pci > tests: add case for pvpanic-pci without address > docs: document panic device 'pvpanic-pci' Everything looks great, so Reviewed-by: Andrea Bolognani The first patch is missing the Signed-off-by tag. Can you please confirm that you're okay with me adding it before pushing? I'm also not seeing any updates to the NEWS file. Please post that as a follow-up patch before release :) -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
On Mon, Feb 20, 2023 at 02:33:55PM +0100, Erik Skultety wrote: > On Mon, Feb 20, 2023 at 03:02:42AM -0800, Andrea Bolognani wrote: > > On Mon, Feb 20, 2023 at 11:40:34AM +0100, Erik Skultety wrote: > > > On Mon, Feb 20, 2023 at 11:21:46AM +0100, Andrea Bolognani wrote: > > > > +sc_prohibit_python_without_env: > > > > + @prohibit='#!/usr/.*/py''thon' \ > > > > > > Shouldn't this be just '#!/usr/.*/python' ? > > > > Yes, but then the syntax-check rule would flag this very line :) > > > > We could do the "exempt from syntax-check" dance, but that feels > > unnecessarily complicated in this scenario. sc_copyright_usage > > already uses a similar trick to prevent the regex to match itself. > > Reviewed-by: Erik Skultety Missing the .com here, you might have a misconfiguration somewhere. Anyway, thanks for the review! Can you please ACK the first patch in the series too? I can't push this one without it. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
On Mon, Feb 20, 2023 at 11:40:34AM +0100, Erik Skultety wrote: > On Mon, Feb 20, 2023 at 11:21:46AM +0100, Andrea Bolognani wrote: > > +sc_prohibit_python_without_env: > > + @prohibit='#!/usr/.*/py''thon' \ > > Shouldn't this be just '#!/usr/.*/python' ? Yes, but then the syntax-check rule would flag this very line :) We could do the "exempt from syntax-check" dance, but that feels unnecessarily complicated in this scenario. sc_copyright_usage already uses a similar trick to prevent the regex to match itself. -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 1/2] docs: Recommend better python3 shebang
Python scripts should always invoked the interpreter through env(1) to ensure that they work on macOS and the BSDs, and at this point not explicitly asking for Python 3 doesn't really make sense. Signed-off-by: Andrea Bolognani --- docs/hooks.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/hooks.rst b/docs/hooks.rst index 9c5f3ff456..9387676a55 100644 --- a/docs/hooks.rst +++ b/docs/hooks.rst @@ -78,7 +78,7 @@ or: :: - #!/usr/bin/python + #!/usr/bin/env python3 Other command interpreters are equally valid, as is any executable binary, so you are welcome to use your favourite languages. -- 2.39.2
[libvirt PATCH 2/2] syntax-check: Ensure Python is called via env(1)
The syntax-check rule that calls flake8 on Python scripts
expects this to be the case, and it's the best practice anyway.
Signed-off-by: Andrea Bolognani
---
build-aux/syntax-check.mk | 5 +
1 file changed, 5 insertions(+)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 96d322ee04..21f6b311ce 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -561,6 +561,11 @@ sc_require_enum_last_marker:
{ echo 'enum impl needs _LAST marker on second line' 1>&2; \
exit 1; } || :
+sc_prohibit_python_without_env:
+ @prohibit='#!/usr/.*/py''thon' \
+halt='always call python via /usr/bin/env' \
+ $(_sc_search_regexp)
+
# We're intentionally ignoring a few warnings
#
# E501: Force breaking lines at < 80 characters results in
--
2.39.2
[libvirt PATCH 0/2] syntax-check: Ensure Python is called via env(1)
As suggested in [1]. [1] https://listman.redhat.com/archives/libvir-list/2022-December/236211.html *** BLURB HERE *** Andrea Bolognani (2): docs: Recommend better python3 shebang syntax-check: Ensure Python is called via env(1) build-aux/syntax-check.mk | 5 + docs/hooks.rst| 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) -- 2.39.2
Re: [libvirt PATCH] docs: introduce a more interesting 404 error page
On Thu, Feb 16, 2023 at 02:53:13PM +, Daniel P. Berrangé wrote: > Our 404 error page is a little bit too boring. The solution is to > add more penguins ! > > This relies on MIT licensed javascript code imported from > > https://github.com/VincentGarreau/particles.js > > The image is extracted from the existing libvirt logo SVG file. > > Signed-off-by: Daniel P. Berrangé > --- > docs/404.html.in |7 +- > docs/css/libvirt.css | 12 + > docs/js/app.js | 108 +++ > docs/js/meson.build|2 + > docs/js/particles.js | 1541 > docs/logos/meson.build |2 + > docs/logos/penguin.png | Bin 0 -> 4236 bytes > docs/logos/penguin.svg | 358 ++ > docs/page.xsl |8 + > 9 files changed, 2035 insertions(+), 3 deletions(-) > create mode 100644 docs/js/app.js > create mode 100644 docs/js/particles.js > create mode 100644 docs/logos/penguin.png > create mode 100644 docs/logos/penguin.svg > > Yes, this could easily be considered an April Fools' joke, but at > the same time it is common to have a bit of fun with 404 error > pages, so it is also a real suggestion :-) +1 to the April Fools' joke, -1 to actually merging this. Sorry! -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 06/33] tests: Unify input files for firmware tests
Most of the differences, such as those in the domain name or amount of memory, are fairly harmless, but they still make it more cumbersome than necessary to directly compare different input (and output) files. More importantly, the use of unversioned machine types in some of the test cases results in the descriptor-based autoselection logic being effectively skipped, because the compatible machine types as listed in them are only the versioned variants. Signed-off-by: Andrea Bolognani --- .../firmware-auto-bios-not-stateless.xml | 4 ++-- .../firmware-auto-bios-nvram.xml | 6 +++--- ...ware-auto-bios-stateless.x86_64-latest.args | 16 .../firmware-auto-bios-stateless.xml | 4 ++-- .../firmware-auto-bios.x86_64-latest.args | 16 tests/qemuxml2argvdata/firmware-auto-bios.xml | 4 ++-- ...rmware-auto-efi-aarch64.aarch64-latest.args | 16 .../firmware-auto-efi-aarch64.xml | 4 ++-- ...mware-auto-efi-enrolled-keys-no-secboot.xml | 4 ++-- ...e-auto-efi-enrolled-keys.x86_64-latest.args | 18 +- .../firmware-auto-efi-enrolled-keys.xml| 4 ++-- .../firmware-auto-efi-loader-insecure.xml | 4 ++-- .../firmware-auto-efi-loader-path.xml | 4 ++-- ...e-auto-efi-loader-secure.x86_64-latest.args | 18 +- .../firmware-auto-efi-loader-secure.xml| 4 ++-- ...uto-efi-no-enrolled-keys.x86_64-latest.args | 18 +- .../firmware-auto-efi-no-enrolled-keys.xml | 4 ++-- ...ware-auto-efi-no-secboot.x86_64-latest.args | 18 +- .../firmware-auto-efi-no-secboot.xml | 4 ++-- .../firmware-auto-efi-nvram.x86_64-latest.args | 18 +- .../firmware-auto-efi-nvram.xml| 6 +++--- ...irmware-auto-efi-secboot.x86_64-latest.args | 18 +- .../firmware-auto-efi-secboot.xml | 4 ++-- ...mware-auto-efi-stateless.x86_64-latest.args | 16 .../firmware-auto-efi-stateless.xml| 4 ++-- .../firmware-auto-efi.x86_64-latest.args | 18 +- tests/qemuxml2argvdata/firmware-auto-efi.xml | 4 ++-- .../firmware-manual-bios-not-stateless.xml | 6 +++--- ...re-manual-bios-stateless.x86_64-latest.args | 16 .../firmware-manual-bios-stateless.xml | 6 +++--- .../firmware-manual-bios.x86_64-latest.args| 16 .../qemuxml2argvdata/firmware-manual-bios.xml | 6 +++--- ...manual-efi-acpi-aarch64.aarch64-latest.args | 6 +++--- .../firmware-manual-efi-acpi-aarch64.xml | 6 +++--- ...ware-manual-efi-acpi-q35.x86_64-latest.args | 6 +++--- .../firmware-manual-efi-acpi-q35.xml | 6 +++--- .../firmware-manual-efi-features.xml | 6 +++--- .../firmware-manual-efi-no-path.xml| 6 +++--- ...nual-efi-noacpi-aarch64.aarch64-latest.args | 6 +++--- .../firmware-manual-efi-noacpi-aarch64.xml | 6 +++--- .../firmware-manual-efi-noacpi-q35.xml | 6 +++--- ...re-manual-efi-nvram-file.x86_64-latest.args | 18 +- .../firmware-manual-efi-nvram-file.xml | 8 ...-efi-nvram-network-iscsi.x86_64-latest.args | 16 ...firmware-manual-efi-nvram-network-iscsi.xml | 6 +++--- ...al-efi-nvram-network-nbd.x86_64-latest.args | 16 .../firmware-manual-efi-nvram-network-nbd.xml | 6 +++--- .../firmware-manual-efi-nvram-stateless.xml| 8 ...are-manual-efi-nvram-template-stateless.xml | 6 +++--- ...anual-efi-nvram-template.x86_64-latest.args | 18 +- .../firmware-manual-efi-nvram-template.xml | 6 +++--- ...e-manual-efi-rw-implicit.x86_64-latest.args | 18 +- .../firmware-manual-efi-rw-implicit.xml| 8 .../firmware-manual-efi-rw.x86_64-latest.args | 18 +- .../firmware-manual-efi-rw.xml | 8 ...rmware-manual-efi-secure.x86_64-latest.args | 18 +- .../firmware-manual-efi-secure.xml | 8 ...are-manual-efi-stateless.x86_64-latest.args | 16 .../firmware-manual-efi-stateless.xml | 6 +++--- .../firmware-manual-efi.x86_64-latest.args | 18 +- tests/qemuxml2argvdata/firmware-manual-efi.xml | 8 .../firmware-manual-noefi-acpi-aarch64.xml | 4 ++-- ...re-manual-noefi-acpi-q35.x86_64-latest.args | 4 ++-- .../firmware-manual-noefi-acpi-q35.xml | 4 ++-- ...al-noefi-noacpi-aarch64.aarch64-latest.args | 4 ++-- .../firmware-manual-noefi-noacpi-aarch64.xml | 4 ++-- ...-manual-noefi-noacpi-q35.x86_64-latest.args | 4 ++-- .../firmware-manual-noefi-noacpi-q35.xml | 4 ++-- ...mware-auto-bios-stateless.x86_64-latest.xml | 6 +++--- .../firmware-auto-bios.x86_64-latest.xml | 6 +++--- ...irmware-auto-efi-aarch64.aarch64-latest.xml | 4 ++-- ...re-auto-efi-enrolled
[libvirt PATCH 30/33] tests: Add more firmware tests
These cover various scenarios related to firmware formats,
specifically ensuring that all the ways in which the user can
ask for a non-default format to be used work correctly.
Signed-off-by: Andrea Bolognani
---
...efi-format-loader-qcow2.x86_64-latest.args | 37 +++
.../firmware-auto-efi-format-loader-qcow2.xml | 18 +
...-efi-format-loader-raw.aarch64-latest.args | 35 ++
.../firmware-auto-efi-format-loader-raw.xml | 18 +
...auto-efi-format-mismatch.x86_64-latest.err | 1 +
.../firmware-auto-efi-format-mismatch.xml | 19 ++
...nvram-qcow2-network-nbd.x86_64-latest.args | 35 ++
...uto-efi-format-nvram-qcow2-network-nbd.xml | 22 +++
...format-nvram-qcow2-path.x86_64-latest.args | 37 +++
...mware-auto-efi-format-nvram-qcow2-path.xml | 18 +
...-efi-format-nvram-qcow2.x86_64-latest.args | 37 +++
.../firmware-auto-efi-format-nvram-qcow2.xml | 18 +
tests/qemuxml2argvtest.c | 7
...-efi-format-loader-qcow2.x86_64-latest.xml | 36 ++
...o-efi-format-loader-raw.aarch64-latest.xml | 31
...-nvram-qcow2-network-nbd.x86_64-latest.xml | 36 ++
...-format-nvram-qcow2-path.x86_64-latest.xml | 36 ++
...o-efi-format-nvram-qcow2.x86_64-latest.xml | 36 ++
tests/qemuxml2xmltest.c | 6 +++
19 files changed, 483 insertions(+)
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-loader-qcow2.x86_64-latest.args
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-loader-qcow2.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-loader-raw.aarch64-latest.args
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-loader-raw.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-mismatch.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-format-mismatch.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-nvram-qcow2-network-nbd.x86_64-latest.args
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-nvram-qcow2-network-nbd.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-nvram-qcow2-path.x86_64-latest.args
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-nvram-qcow2-path.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-nvram-qcow2.x86_64-latest.args
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-format-nvram-qcow2.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-format-loader-qcow2.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-format-loader-raw.aarch64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-format-nvram-qcow2-network-nbd.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-format-nvram-qcow2-path.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-format-nvram-qcow2.x86_64-latest.xml
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-format-loader-qcow2.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-format-loader-qcow2.x86_64-latest.args
new file mode 100644
index 00..cbd837d2bd
--- /dev/null
+++
b/tests/qemuxml2argvdata/firmware-auto-efi-format-loader-qcow2.x86_64-latest.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-guest/master-key.aes"}'
\
+-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.qcow2","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"qcow2","file":"libvirt-pflash0-storage"}'
\
+-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_VARS.qcow2","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"qcow2","file":"libvirt-pflash1-storage"}'
\
+-machine
pc-q35-4.0,usb=off,dump-guest-core=off,memory-backend=
[libvirt PATCH 27/33] qemu: Propagate firmware format
Take the information from the descriptor and store it in the
domain definition. Various things, such as the arguments passed
to -blockdev and the path generated for the NVRAM file, will
then be based on it.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_firmware.c | 27 +--
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f0b9e08d85..a19a3577e1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11478,7 +11478,7 @@ qemuDomainInitializePflashStorageSource(virDomainObj
*vm,
pflash0 = virStorageSourceNew();
pflash0->type = VIR_STORAGE_TYPE_FILE;
-pflash0->format = VIR_STORAGE_FILE_RAW;
+pflash0->format = def->os.loader->format;
pflash0->path = g_strdup(def->os.loader->path);
pflash0->readonly = false;
virTristateBoolToBool(def->os.loader->readonly, >readonly);
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index a0167f860c..01efb60e69 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -990,9 +990,11 @@
qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(virDomainLoader type)
*/
static void
qemuFirmwareEnsureNVRAM(virDomainDef *def,
-const virQEMUDriverConfig *cfg)
+const virQEMUDriverConfig *cfg,
+virStorageFileFormat format)
{
virDomainLoaderDef *loader = def->os.loader;
+const char *ext = NULL;
if (!loader)
return;
@@ -1007,9 +1009,14 @@ qemuFirmwareEnsureNVRAM(virDomainDef *def,
loader->nvram = virStorageSourceNew();
loader->nvram->type = VIR_STORAGE_TYPE_FILE;
-loader->nvram->format = VIR_STORAGE_FILE_RAW;
+loader->nvram->format = format;
-loader->nvram->path = g_strdup_printf("%s/%s_VARS.fd", cfg->nvramDir,
def->name);
+if (format == VIR_STORAGE_FILE_RAW)
+ext = ".fd";
+
+loader->nvram->path = g_strdup_printf("%s/%s_VARS%s",
+ cfg->nvramDir, def->name,
+ ext ? ext : "");
}
@@ -1233,22 +1240,30 @@ qemuFirmwareEnableFeaturesModern(virQEMUDriverConfig
*cfg,
const qemuFirmwareMappingKernel *kernel = >mapping.data.kernel;
const qemuFirmwareMappingMemory *memory = >mapping.data.memory;
virDomainLoaderDef *loader = NULL;
+virStorageFileFormat format;
size_t i;
switch (fw->mapping.device) {
case QEMU_FIRMWARE_DEVICE_FLASH:
+if ((format =
virStorageFileFormatTypeFromString(flash->executable.format)) < 0)
+return -1;
+
if (!def->os.loader)
def->os.loader = virDomainLoaderDefNew();
loader = def->os.loader;
loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
loader->readonly = VIR_TRISTATE_BOOL_YES;
+loader->format = format;
VIR_FREE(loader->path);
loader->path = g_strdup(flash->executable.filename);
if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
-qemuFirmwareEnsureNVRAM(def, cfg);
+if ((format =
virStorageFileFormatTypeFromString(flash->nvram_template.format)) < 0)
+return -1;
+
+qemuFirmwareEnsureNVRAM(def, cfg, format);
/* If the NVRAM is not a local path then we can't create or
* reset it, so in that case filling in the nvramTemplate
@@ -1457,7 +1472,7 @@ qemuFirmwareFillDomainLegacy(virQEMUDriver *driver,
loader->readonly = VIR_TRISTATE_BOOL_YES;
loader->nvramTemplate = g_strdup(cfg->firmwares[i]->nvram);
-qemuFirmwareEnsureNVRAM(def, cfg);
+qemuFirmwareEnsureNVRAM(def, cfg, VIR_STORAGE_FILE_RAW);
VIR_DEBUG("decided on firmware '%s' template '%s'",
loader->path, NULLSTR(loader->nvramTemplate));
@@ -1625,7 +1640,7 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
* generate a path to the domain-specific NVRAM file, but
* otherwise we're good to go */
if (loader->nvramTemplate) {
-qemuFirmwareEnsureNVRAM(def, cfg);
+qemuFirmwareEnsureNVRAM(def, cfg, loader->format);
return 0;
}
}
--
2.39.1
[libvirt PATCH 19/33] conf: Export virDomainDefOSValidate()
We're going to need it elsewhere very soon.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_validate.c | 4 ++--
src/conf/domain_validate.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 5559a71e14..db13e56107 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1586,14 +1586,14 @@ virDomainDefMemtuneValidate(const virDomainDef *def)
}
-static int
+int
virDomainDefOSValidate(const virDomainDef *def,
virDomainXMLOption *xmlopt)
{
virDomainLoaderDef *loader = def->os.loader;
if (def->os.firmware) {
-if (!(xmlopt->config.features & VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT))
{
+if (xmlopt && !(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;
diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h
index 07b99195e3..fc441cef5b 100644
--- a/src/conf/domain_validate.h
+++ b/src/conf/domain_validate.h
@@ -44,3 +44,6 @@ int virDomainDiskDefValidateSource(const virStorageSource
*src);
int virDomainDiskDefValidateStartupPolicy(const virDomainDiskDef *disk);
int virDomainDiskDefSourceLUNValidate(const virStorageSource *src);
+
+int virDomainDefOSValidate(const virDomainDef *def,
+ virDomainXMLOption *xmlopt);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e550e7139d..5a356ccd48 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -793,6 +793,7 @@ virDomainDefPostParse;
# conf/domain_validate.h
virDomainActualNetDefValidate;
+virDomainDefOSValidate;
virDomainDefValidate;
virDomainDeviceValidateAliasForHotplug;
virDomainDiskDefSourceLUNValidate;
--
2.39.1
[libvirt PATCH 14/33] qemu: Only fill nvramTemplate for local sources
It doesn't make sense for non-local sources, since we can't
create or reset the corresponding NVRAM file.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 13bac9490a..175a4db21d 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1194,15 +1194,20 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
return -1;
}
-VIR_FREE(loader->nvramTemplate);
-loader->nvramTemplate = g_strdup(flash->nvram_template.filename);
-
if (!loader->nvram) {
loader->nvram = virStorageSourceNew();
loader->nvram->type = VIR_STORAGE_TYPE_FILE;
loader->nvram->format = VIR_STORAGE_FILE_RAW;
qemuDomainNVRAMPathFormat(cfg, def, >nvram->path);
}
+
+/* If the NVRAM is not a local path then we can't create or
+ * reset it, so in that case filling in the nvramTemplate
+ * field would be misleading */
+VIR_FREE(loader->nvramTemplate);
+if (loader->nvram &&
virStorageSourceIsLocalStorage(loader->nvram)) {
+loader->nvramTemplate =
g_strdup(flash->nvram_template.filename);
+}
}
VIR_DEBUG("decided on firmware '%s' template '%s' NVRAM '%s'",
--
2.39.1
[libvirt PATCH 15/33] qemu: Clear os.firmwareFeatures after autoselection
We already clear os.firmware, so it doesn't make sense to keep the list of features around. Moreover, our validation routines will reject an XML that contains a list of firmware features but disables firmware autoselection, so not clearing these means that the live XML for a domain that uses feature-based autoselection can't be fed back into libvirt. Signed-off-by: Andrea Bolognani --- src/qemu/qemu_firmware.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 175a4db21d..572172bc75 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -1454,6 +1454,7 @@ qemuFirmwareFillDomain(virQEMUDriver *driver, goto cleanup; def->os.firmware = VIR_DOMAIN_OS_DEF_FIRMWARE_NONE; +VIR_FREE(def->os.firmwareFeatures); ret = 0; cleanup: -- 2.39.1
[libvirt PATCH 28/33] conf: Accept QCOW2 firmware format
All of the drivers will reject this value.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c| 6 --
src/conf/schemas/domaincommon.rng | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 528426511e..d1c347dafc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16769,7 +16769,8 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
, VIR_STORAGE_FILE_RAW) < 0) {
return -1;
}
-if (format != VIR_STORAGE_FILE_RAW) {
+if (format != VIR_STORAGE_FILE_RAW &&
+format != VIR_STORAGE_FILE_QCOW2) {
virReportError(VIR_ERR_XML_ERROR,
_("Unsupported nvram format '%s'"),
virStorageFileFormatTypeToString(format));
@@ -16857,7 +16858,8 @@ virDomainLoaderDefParseXMLLoader(virDomainLoaderDef
*loader,
, VIR_STORAGE_FILE_RAW) < 0) {
return -1;
}
-if (format != VIR_STORAGE_FILE_RAW) {
+if (format != VIR_STORAGE_FILE_RAW &&
+format != VIR_STORAGE_FILE_QCOW2) {
virReportError(VIR_ERR_XML_ERROR,
_("Unsupported loader format '%s'"),
virStorageFileFormatTypeToString(format));
diff --git a/src/conf/schemas/domaincommon.rng
b/src/conf/schemas/domaincommon.rng
index 150543e076..f3d2a786cc 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -7548,6 +7548,7 @@
raw
+qcow2
--
2.39.1
[libvirt PATCH 16/33] qemu: Don't pick firmware that requires SMM when smm=off
At the moment, if SMM is explicitly disabled in the domain XML
but a firmware descriptor that requires SMM to be enabled has
the highest priority and otherwise matches the requirements,
we pick that firmware only to error out later, when the domain
is started.
A better approach is to take into account the fact that SMM is
disabled while performing autoselection, and ignore all
descriptors that advertise the requires-smm feature.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 33 +++--
...rmware-auto-efi-smm-off.x86_64-latest.args | 37 +++
...irmware-auto-efi-smm-off.x86_64-latest.err | 1 -
tests/qemuxml2argvtest.c | 2 +-
4 files changed, 52 insertions(+), 21 deletions(-)
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.args
delete mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.err
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 572172bc75..c24bca6183 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1102,11 +1102,18 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
}
}
-if (loader && loader->secure == VIR_TRISTATE_BOOL_YES &&
-!requiresSMM) {
-VIR_DEBUG("Domain restricts pflash programming to SMM, "
- "but firmware '%s' doesn't support SMM", path);
-return false;
+if (requiresSMM) {
+if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_OFF) {
+VIR_DEBUG("Domain explicitly disables SMM, "
+ "but firmware '%s' requires it to be enabled", path);
+return false;
+}
+} else {
+if (loader && loader->secure == VIR_TRISTATE_BOOL_YES) {
+VIR_DEBUG("Domain restricts pflash programming to SMM, "
+ "but firmware '%s' doesn't support SMM", path);
+return false;
+}
}
if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_FLASH) {
@@ -1244,21 +1251,9 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
for (i = 0; i < fw->nfeatures; i++) {
switch (fw->features[i]) {
case QEMU_FIRMWARE_FEATURE_REQUIRES_SMM:
-switch (def->features[VIR_DOMAIN_FEATURE_SMM]) {
-case VIR_TRISTATE_SWITCH_OFF:
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("domain has SMM turned off "
- "but chosen firmware requires it"));
-return -1;
-case VIR_TRISTATE_SWITCH_ABSENT:
-VIR_DEBUG("Enabling SMM feature");
-def->features[VIR_DOMAIN_FEATURE_SMM] = VIR_TRISTATE_SWITCH_ON;
-break;
+VIR_DEBUG("Enabling SMM feature");
+def->features[VIR_DOMAIN_FEATURE_SMM] = VIR_TRISTATE_SWITCH_ON;
-case VIR_TRISTATE_SWITCH_ON:
-case VIR_TRISTATE_SWITCH_LAST:
-break;
-}
VIR_DEBUG("Enabling secure loader");
def->os.loader->secure = VIR_TRISTATE_BOOL_YES;
break;
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.args
new file mode 100644
index 00..692795194f
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-guest/master-key.aes"}'
\
+-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.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/guest_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver&q
[libvirt PATCH 25/33] drivers: Reject unsupported firmware formats
This ensures that, as we add support for more formats at the
domain XML level, we don't accidentally cause drivers to
misbehave or users to get confused.
All existing drivers support the raw format, and supporting
additional formats will require explicit opt-in on the
driver's part.
Signed-off-by: Andrea Bolognani
---
src/bhyve/bhyve_firmware.c | 7 +++
src/libxl/libxl_conf.c | 7 +++
src/qemu/qemu_firmware.c | 16
3 files changed, 30 insertions(+)
diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c
index cb1b94b4d5..ff131efa41 100644
--- a/src/bhyve/bhyve_firmware.c
+++ b/src/bhyve/bhyve_firmware.c
@@ -80,6 +80,13 @@ bhyveFirmwareFillDomain(bhyveConn *driver,
if (!def->os.loader)
def->os.loader = virDomainLoaderDefNew();
+if (def->os.loader->format != VIR_STORAGE_FILE_RAW) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported loader format '%s'"),
+
virStorageFileFormatTypeToString(def->os.loader->format));
+return -1;
+}
+
def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
def->os.loader->readonly = VIR_TRISTATE_BOOL_YES;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 97c183ebf3..485015ef63 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -656,6 +656,13 @@ libxlMakeDomBuildInfo(virDomainDef *def,
b_info->u.hvm.bios = LIBXL_BIOS_TYPE_OVMF;
}
+if (def->os.loader && def->os.loader->format != VIR_STORAGE_FILE_RAW) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported loader format '%s'"),
+
virStorageFileFormatTypeToString(def->os.loader->format));
+return -1;
+}
+
if (def->emulator) {
if (!virFileExists(def->emulator)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 4d34062ebf..be6d8d4519 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1545,6 +1545,7 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
virDomainLoaderDef *loader = def->os.loader;
+virStorageSource *nvram = loader ? loader->nvram : NULL;
bool autoSelection = (def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE);
int ret;
@@ -1559,6 +1560,21 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
if (virDomainDefOSValidate(def, NULL) < 0)
return -1;
+if (loader &&
+loader->format != VIR_STORAGE_FILE_RAW) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported loader format '%s'"),
+ virStorageFileFormatTypeToString(loader->format));
+return -1;
+}
+if (nvram &&
+nvram->format != VIR_STORAGE_FILE_RAW) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported nvram format '%s'"),
+ virStorageFileFormatTypeToString(nvram->format));
+return -1;
+}
+
/* If firmware autoselection is disabled and the loader is a ROM
* instead of a PFLASH device, then we're using BIOS and we don't
* need any information at all */
--
2.39.1
[libvirt PATCH 32/33] news: Document changes to firmware autoselection
Signed-off-by: Andrea Bolognani --- NEWS.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 4625a838f8..577e1502b5 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -41,6 +41,16 @@ v9.1.0 (unreleased) allowing installation of a modular daemon configuration without the traditional monolithic libvirtd. + * qemu: Make firmware selection persistent + +Up until now, firmware autoselection has been performed at domain startup +time: as a result, changes to the JSON firmware descriptors present on the +system could have translated to a different firmware being chosen for +subsequent startups of the same domain, potentially rendering it unbootable +or lowering the security guarantees. Firmware selection now happens once, +when the domain is defined, and its results are stored in the domain XML +to be reused, unchanged, for all subsequent boots. + * **Bug fixes** * QEMU: iTCO watchdog made operational -- 2.39.1
[libvirt PATCH 33/33] news: Document support for QCOW2 format firmware
Signed-off-by: Andrea Bolognani --- NEWS.rst | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 577e1502b5..d9b923dabb 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -33,6 +33,12 @@ v9.1.0 (unreleased) Support crypto device(virtio crypto only), also add support for QEMU with backend ``builtin`` and ``lkcf``. + * qemu: Add support for QCOW2 formatted firmware + +This type of firmware can be picked up either automatically, if the +corresponding JSON descriptor has the highest priority, or manually by +using in the domain XML. + * **Improvements** * RPM packaging changes -- 2.39.1
[libvirt PATCH 22/33] qemu: Introduce qemuFirmwareEnsureNVRAM()
This helper replaces qemuDomainNVRAMPathFormat() and also
incorporates some common operations that all callers of that
helper needed.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 49 ++--
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index cf854dfe19..ac1ae1e923 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -979,12 +979,32 @@
qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(virDomainLoader type)
}
+/**
+ * qemuFirmwareEnsureNVRAM:
+ * @def: domain definition
+ * @cfg: QEMU driver configuration
+ *
+ * Make sure that a source for the NVRAM file exists, possibly by
+ * creating it. This might involve automatically generating the
+ * corresponding path.
+ */
static void
-qemuDomainNVRAMPathFormat(virQEMUDriverConfig *cfg,
- virDomainDef *def,
- char **path)
+qemuFirmwareEnsureNVRAM(virDomainDef *def,
+const virQEMUDriverConfig *cfg)
{
-*path = g_strdup_printf("%s/%s_VARS.fd", cfg->nvramDir, def->name);
+virDomainLoaderDef *loader = def->os.loader;
+
+if (!loader)
+return;
+
+if (loader->nvram)
+return;
+
+loader->nvram = virStorageSourceNew();
+loader->nvram->type = VIR_STORAGE_TYPE_FILE;
+loader->nvram->format = VIR_STORAGE_FILE_RAW;
+
+loader->nvram->path = g_strdup_printf("%s/%s_VARS.fd", cfg->nvramDir,
def->name);
}
@@ -1209,12 +1229,7 @@ qemuFirmwareEnableFeaturesModern(virQEMUDriverConfig
*cfg,
loader->path = g_strdup(flash->executable.filename);
if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
-if (!loader->nvram) {
-loader->nvram = virStorageSourceNew();
-loader->nvram->type = VIR_STORAGE_TYPE_FILE;
-loader->nvram->format = VIR_STORAGE_FILE_RAW;
-qemuDomainNVRAMPathFormat(cfg, def, >nvram->path);
-}
+qemuFirmwareEnsureNVRAM(def, cfg);
/* If the NVRAM is not a local path then we can't create or
* reset it, so in that case filling in the nvramTemplate
@@ -1417,12 +1432,7 @@ qemuFirmwareFillDomainLegacy(virQEMUDriver *driver,
loader->readonly = VIR_TRISTATE_BOOL_YES;
loader->nvramTemplate = g_strdup(cfg->firmwares[i]->nvram);
-if (!loader->nvram) {
-loader->nvram = virStorageSourceNew();
-loader->nvram->type = VIR_STORAGE_TYPE_FILE;
-loader->nvram->format = VIR_STORAGE_FILE_RAW;
-qemuDomainNVRAMPathFormat(cfg, def, >nvram->path);
-}
+qemuFirmwareEnsureNVRAM(def, cfg);
VIR_DEBUG("decided on firmware '%s' template '%s'",
loader->path, NULLSTR(loader->nvramTemplate));
@@ -1574,12 +1584,7 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
* generate a path to the domain-specific NVRAM file, but
* otherwise we're good to go */
if (loader->nvramTemplate) {
-if (!loader->nvram) {
-loader->nvram = virStorageSourceNew();
-loader->nvram->type = VIR_STORAGE_TYPE_FILE;
-loader->nvram->format = VIR_STORAGE_FILE_RAW;
-qemuDomainNVRAMPathFormat(cfg, def, >nvram->path);
-}
+qemuFirmwareEnsureNVRAM(def, cfg);
return 0;
}
}
--
2.39.1
[libvirt PATCH 23/33] conf: Change handling for empty NVRAM path
Right now, this results in loader->nvram being NULL, which is
reasonable: loader->nvramTemplate is stored separately, so if
the element doesn't contain a path there is really no
useful information inside it.
However, this is about to change, so we will find ourselves
needing to hold on to loader->nvram even when no path is
present. Change the firmware handling code so that such a
scenario is dealt with appropriately.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 9 +++--
src/qemu/qemu_firmware.c | 7 ++-
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8117cff83e..30a3261dab 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16766,16 +16766,13 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
return -1;
if (!typePresent) {
-g_autofree char *path = NULL;
-
-if (!(path = virXMLNodeContentString(nvramNode)))
+if (!(src->path = virXMLNodeContentString(nvramNode)))
return -1;
-if (STREQ(path, ""))
-return 0;
+if (STREQ(src->path, ""))
+VIR_FREE(src->path);
src->type = VIR_STORAGE_TYPE_FILE;
-src->path = g_steal_pointer();
} else {
if (!nvramSourceNode)
return -1;
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index ac1ae1e923..4d34062ebf 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -997,9 +997,14 @@ qemuFirmwareEnsureNVRAM(virDomainDef *def,
if (!loader)
return;
-if (loader->nvram)
+/* If the source already exists and is fully specified, including
+ * the path, leave it alone */
+if (loader->nvram && loader->nvram->path)
return;
+if (loader->nvram)
+virObjectUnref(loader->nvram);
+
loader->nvram = virStorageSourceNew();
loader->nvram->type = VIR_STORAGE_TYPE_FILE;
loader->nvram->format = VIR_STORAGE_FILE_RAW;
--
2.39.1
[libvirt PATCH 31/33] docs: Document firmware format attribute
Signed-off-by: Andrea Bolognani --- docs/formatdomain.rst | 9 + 1 file changed, 9 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 8407bab1ba..6122f30b76 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -257,6 +257,11 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. any config changes on shutdown. The ``stateless`` flag (:since:`Since 8.6.0`) can be used to control this behaviour, when set to ``yes`` NVRAM will never be created. + + When firmware autoselection is enabled, the ``format`` attribute can be + used to tell libvirt to only consider firmware builds that are in a + specific format. Supported values are ``raw`` and ``qcow2``. + :since:`Since 9.1.0 (QEMU only)` ``nvram`` Some UEFI firmwares may want to use a non-volatile memory to store some variables. In the host, this is represented as a file and the absolute path @@ -276,6 +281,10 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. **Note:** ``network`` backed NVRAM the variables are not instantiated from the ``template`` and it's user's responsibility to provide a valid NVRAM image. + This element supports a ``format`` attribute, which has the same semantics + as the attribute of the same name for the element. + :since:`Since 9.1.0 (QEMU only)` + It is not valid to provide this element if the loader is marked as stateless. -- 2.39.1
[libvirt PATCH 29/33] qemu: Add support for QCOW2 format firmware
https://bugzilla.redhat.com/show_bug.cgi?id=2161965
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 14 ++
.../firmware-auto-efi-aarch64.aarch64-latest.args | 8
.../virtio-iommu-aarch64.aarch64-latest.args | 8
.../firmware-auto-efi-aarch64.aarch64-latest.xml | 4 ++--
.../virtio-iommu-aarch64.aarch64-latest.xml| 4 ++--
5 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 01efb60e69..9de4166772 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1013,6 +1013,8 @@ qemuFirmwareEnsureNVRAM(virDomainDef *def,
if (format == VIR_STORAGE_FILE_RAW)
ext = ".fd";
+if (format == VIR_STORAGE_FILE_QCOW2)
+ext = ".qcow2";
loader->nvram->path = g_strdup_printf("%s/%s_VARS%s",
cfg->nvramDir, def->name,
@@ -1173,7 +1175,8 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
}
}
-if (STRNEQ(flash->executable.format, "raw")) {
+if (STRNEQ(flash->executable.format, "raw") &&
+STRNEQ(flash->executable.format, "qcow2")) {
VIR_DEBUG("Discarding loader with unsupported flash format '%s'",
flash->executable.format);
return false;
@@ -1186,7 +1189,8 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
return false;
}
if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
-if (STRNEQ(flash->nvram_template.format, "raw")) {
+if (STRNEQ(flash->nvram_template.format, "raw") &&
+STRNEQ(flash->nvram_template.format, "qcow2")) {
VIR_DEBUG("Discarding loader with unsupported nvram template
format '%s'",
flash->nvram_template.format);
return false;
@@ -1596,14 +1600,16 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
return -1;
if (loader &&
-loader->format != VIR_STORAGE_FILE_RAW) {
+loader->format != VIR_STORAGE_FILE_RAW &&
+loader->format != VIR_STORAGE_FILE_QCOW2) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported loader format '%s'"),
virStorageFileFormatTypeToString(loader->format));
return -1;
}
if (nvram &&
-nvram->format != VIR_STORAGE_FILE_RAW) {
+nvram->format != VIR_STORAGE_FILE_RAW &&
+nvram->format != VIR_STORAGE_FILE_QCOW2) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported nvram format '%s'"),
virStorageFileFormatTypeToString(nvram->format));
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-aarch64.aarch64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-aarch64.aarch64-latest.args
index 05b27e84e8..8ebd1b8a26 100644
--- a/tests/qemuxml2argvdata/firmware-auto-efi-aarch64.aarch64-latest.args
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-aarch64.aarch64-latest.args
@@ -10,10 +10,10 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \
-name guest=guest,debug-threads=on \
-S \
-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-guest/master-key.aes"}'
\
--blockdev
'{"driver":"file","filename":"/usr/share/AAVMF/AAVMF_CODE.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/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"}'
\
+-blockdev
'{"driver":"file","filename":"/usr/share/AAVMF/AAVMF_CODE.qcow2","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"qcow2","file":"libvirt-pflash0-storage&q
[libvirt PATCH 24/33] conf: Parse firmware format
The default is raw, which corresponds to the historical
behavior and is also the only accepted value.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c| 72 +--
src/conf/domain_conf.h| 1 +
src/conf/schemas/domaincommon.rng | 14 ++
3 files changed, 84 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 30a3261dab..528426511e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3717,7 +3717,12 @@ virDomainPanicDefFree(virDomainPanicDef *panic)
virDomainLoaderDef *
virDomainLoaderDefNew(void)
{
-return g_new0(virDomainLoaderDef, 1);
+virDomainLoaderDef *def = NULL;
+
+def = g_new0(virDomainLoaderDef, 1);
+def->format = VIR_STORAGE_FILE_RAW;
+
+return def;
}
void
@@ -16751,6 +16756,7 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
unsigned int flags)
{
g_autoptr(virStorageSource) src = virStorageSourceNew();
+unsigned int format = 0;
int typePresent;
if (!nvramNode)
@@ -16758,7 +16764,18 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
loader->nvramTemplate = virXMLPropString(nvramNode, "template");
-src->format = VIR_STORAGE_FILE_RAW;
+if (virXMLPropEnumDefault(nvramNode, "format",
+ virStorageFileFormatTypeFromString,
VIR_XML_PROP_NONE,
+ , VIR_STORAGE_FILE_RAW) < 0) {
+return -1;
+}
+if (format != VIR_STORAGE_FILE_RAW) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("Unsupported nvram format '%s'"),
+ virStorageFileFormatTypeToString(format));
+return -1;
+}
+src->format = format;
if ((typePresent = virXMLPropEnum(nvramNode, "type",
virStorageTypeFromString,
VIR_XML_PROP_NONE,
@@ -16792,8 +16809,26 @@ static int
virDomainLoaderDefParseXMLLoader(virDomainLoaderDef *loader,
xmlNodePtr loaderNode)
{
-if (!loaderNode)
+unsigned int format = 0;
+
+if (!loaderNode) {
+/* If there is no element but the element
+ * was present, copy the format from the latter to the
+ * former.
+ *
+ * This ensures that a configuration such as
+ *
+ *
+ *
+ *
+ *
+ * behaves as expected, that is, results in a firmware build
+ * with format 'foo' being selected */
+if (loader->nvram)
+loader->format = loader->nvram->format;
+
return 0;
+}
if (virXMLPropTristateBool(loaderNode, "readonly", VIR_XML_PROP_NONE,
>readonly) < 0)
@@ -16817,6 +16852,19 @@ virDomainLoaderDefParseXMLLoader(virDomainLoaderDef
*loader,
>stateless) < 0)
return -1;
+if (virXMLPropEnumDefault(loaderNode, "format",
+ virStorageFileFormatTypeFromString,
VIR_XML_PROP_NONE,
+ , VIR_STORAGE_FILE_RAW) < 0) {
+return -1;
+}
+if (format != VIR_STORAGE_FILE_RAW) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("Unsupported loader format '%s'"),
+ virStorageFileFormatTypeToString(format));
+return -1;
+}
+loader->format = format;
+
return 0;
}
@@ -16839,6 +16887,14 @@ virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
loaderNode) < 0)
return -1;
+if (loader->nvram && loader->format != loader->nvram->format) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("Format mismatch: loader.format='%s'
nvram.format='%s'"),
+ virStorageFileFormatTypeToString(loader->format),
+
virStorageFileFormatTypeToString(loader->nvram->format));
+return -1;
+}
+
return 0;
}
@@ -26156,6 +26212,11 @@ virDomainLoaderDefFormatNvram(virBuffer *buf,
false, flags, false, false, xmlopt)
< 0)
return -1;
}
+
+if (src->format != VIR_STORAGE_FILE_RAW) {
+virBufferEscapeString(, " format='%s'",
+
virStorageFileFormatTypeToString(src->format));
+}
}
virXMLFormatElementInternal(buf, "nvram", , childBuf, false,
childNewline);
@@ -26190,6 +26251,11 @@ virDomainLoaderDefFormat(virBuffer *buf,
virTristateBoolTypeToString(loader->stateless));
}
+if (loader->format != VIR_STORAGE_FILE_RAW) {
+virBufferEscapeString(, " format='%s'",
[libvirt PATCH 26/33] qemu: Filter firmwares based on format
If the user has requested a specific firmware format, then
all firmware builds that are not in that format should be
ignored while looking for matches.
The legacy hardcoded firmware list predates firmware
descriptors and their "format" field, so we can safely
assume that all builds listed in there are in raw format.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 20
1 file changed, 20 insertions(+)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index be6d8d4519..a0167f860c 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1171,12 +1171,26 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
flash->executable.format);
return false;
}
+if (loader &&
+STRNEQ(flash->executable.format,
virStorageFileFormatTypeToString(loader->format))) {
+VIR_DEBUG("Discarding loader with mismatching flash format '%s' !=
'%s'",
+ flash->executable.format,
+ virStorageFileFormatTypeToString(loader->format));
+return false;
+}
if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
if (STRNEQ(flash->nvram_template.format, "raw")) {
VIR_DEBUG("Discarding loader with unsupported nvram template
format '%s'",
flash->nvram_template.format);
return false;
}
+if (loader && loader->nvram &&
+STRNEQ(flash->nvram_template.format,
virStorageFileFormatTypeToString(loader->nvram->format))) {
+VIR_DEBUG("Discarding loader with mismatching nvram template
format '%s' != '%s'",
+ flash->nvram_template.format,
+
virStorageFileFormatTypeToString(loader->nvram->format));
+return false;
+}
}
}
@@ -1424,6 +1438,12 @@ qemuFirmwareFillDomainLegacy(virQEMUDriver *driver,
return 0;
}
+if (loader->format != VIR_STORAGE_FILE_RAW) {
+VIR_DEBUG("Ignoring legacy entries for loader with flash format '%s'",
+ virStorageFileFormatTypeToString(loader->format));
+return 1;
+}
+
for (i = 0; i < cfg->nfirmwares; i++) {
virFirmware *fw = cfg->firmwares[i];
--
2.39.1
[libvirt PATCH 21/33] qemu: Move qemuDomainNVRAMPathFormat() to qemu_firmware
There are no other callers remaining.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_domain.c | 9 -
src/qemu/qemu_domain.h | 5 -
src/qemu/qemu_firmware.c | 9 +
3 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fad46fec1b..f0b9e08d85 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11353,15 +11353,6 @@ qemuDomainDiskIsMissingLocalOptional(virDomainDiskDef
*disk)
}
-void
-qemuDomainNVRAMPathFormat(virQEMUDriverConfig *cfg,
- virDomainDef *def,
- char **path)
-{
-*path = g_strdup_printf("%s/%s_VARS.fd", cfg->nvramDir, def->name);
-}
-
-
virDomainEventSuspendedDetailType
qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason)
{
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 1053d1d4cb..987398486a 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1049,11 +1049,6 @@
qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason);
bool
qemuDomainDiskIsMissingLocalOptional(virDomainDiskDef *disk);
-void
-qemuDomainNVRAMPathFormat(virQEMUDriverConfig *cfg,
- virDomainDef *def,
- char **path);
-
virDomainEventSuspendedDetailType
qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason);
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 4a6adedfcb..cf854dfe19 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -979,6 +979,15 @@
qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(virDomainLoader type)
}
+static void
+qemuDomainNVRAMPathFormat(virQEMUDriverConfig *cfg,
+ virDomainDef *def,
+ char **path)
+{
+*path = g_strdup_printf("%s/%s_VARS.fd", cfg->nvramDir, def->name);
+}
+
+
#define VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY (1 << 2)
--
2.39.1
[libvirt PATCH 20/33] qemu: Move firmware selection from startup to postparse
Currently, firmware selection is performed as part of the
domain startup process. This mostly works fine, but there's a
significant downside to this approach: since the process is
affected by factors outside of libvirt's control, specifically
the contents of the various JSON firmware descriptors and
their names, it's pretty much impossible to guarantee that the
outcome is always going to be the same. It would only take an
edk2 update, or a change made by the local admin, to render a
domain unbootable or downgrade its boot security.
To avoid this, move firmware selection to the postparse phase.
This way it will only be performed once, when the domain is
first defined; subsequent boots will not need to go through
the process again, as all the paths that were picked during
firmware selection are recorded in the domain XML.
Care is taken to ensure that existing domains are handled
correctly, even if their firmware configuration can't be
successfully resolved. Failure to complete the firmware
selection process is only considered fatal when defining a
new domain; in all other cases the error will be reported
during startup, as is already the case today.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_domain.c| 40 ++-
src/qemu/qemu_driver.c| 2 -
src/qemu/qemu_firmware.c | 268 ++
src/qemu/qemu_firmware.h | 3 +-
src/qemu/qemu_process.c | 33 ++-
.../aarch64-virt-graphics.aarch64-latest.xml | 2 +-
.../aarch64-virt-headless.aarch64-latest.xml | 2 +-
...ware-auto-bios-stateless.x86_64-latest.xml | 4 +-
.../firmware-auto-bios.x86_64-latest.xml | 3 +-
...rmware-auto-efi-aarch64.aarch64-latest.xml | 4 +-
...e-auto-efi-enrolled-keys.x86_64-latest.xml | 9 +-
...e-auto-efi-loader-secure.x86_64-latest.xml | 6 +-
...uto-efi-no-enrolled-keys.x86_64-latest.xml | 7 +-
...ware-auto-efi-no-secboot.x86_64-latest.xml | 7 +-
...ware-auto-efi-nvram-file.x86_64-latest.xml | 5 +-
...-efi-nvram-network-iscsi.x86_64-latest.xml | 3 +-
...to-efi-nvram-network-nbd.x86_64-latest.xml | 3 +-
.../firmware-auto-efi-nvram.x86_64-latest.xml | 6 +-
...irmware-auto-efi-secboot.x86_64-latest.xml | 8 +-
...irmware-auto-efi-smm-off.x86_64-latest.xml | 4 +-
...mware-auto-efi-stateless.x86_64-latest.xml | 4 +-
.../firmware-auto-efi.x86_64-latest.xml | 5 +-
...manual-efi-acpi-aarch64.aarch64-latest.xml | 2 +-
...ware-manual-efi-acpi-q35.x86_64-latest.xml | 2 +-
...nual-efi-noacpi-aarch64.aarch64-latest.xml | 2 +-
...re-manual-efi-nvram-file.x86_64-latest.xml | 2 +-
...rmware-manual-efi-secure.x86_64-latest.xml | 2 +-
.../firmware-manual-efi.x86_64-latest.xml | 2 +-
.../virtio-iommu-aarch64.aarch64-latest.xml | 4 +-
29 files changed, 310 insertions(+), 134 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index da1a2413a5..fad46fec1b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -27,6 +27,7 @@
#include "qemu_cgroup.h"
#include "qemu_command.h"
#include "qemu_capabilities.h"
+#include "qemu_firmware.h"
#include "qemu_hostdev.h"
#include "qemu_migration_params.h"
#include "qemu_security.h"
@@ -4427,21 +4428,42 @@ qemuDomainRecheckInternalPaths(virDomainDef *def,
static int
qemuDomainDefBootPostParse(virDomainDef *def,
- virQEMUDriverConfig *cfg)
+ virQEMUDriver *driver,
+ unsigned int parseFlags)
{
+bool abiUpdate = !!(parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);
+
if (def->os.bootloader || def->os.bootloaderArgs) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("bootloader is not supported by QEMU"));
return -1;
}
-if (virDomainDefHasOldStyleROUEFI(def) &&
-!def->os.loader->nvram &&
-def->os.loader->stateless != VIR_TRISTATE_BOOL_YES) {
-def->os.loader->nvram = virStorageSourceNew();
-def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE;
-def->os.loader->nvram->format = VIR_STORAGE_FILE_RAW;
-qemuDomainNVRAMPathFormat(cfg, def, >os.loader->nvram->path);
+/* Firmware selection can fail for a number of reasons, but the
+ * most likely one is that the requested configuration contains
+ * mistakes or includes constraints that are impossible to
+ * satisfy on the current system.
+ *
+ * If that happens, we have to react differently based on the
+ * situation: if we're defining a new domain or updating its ABI,
+ * we should let the user know immediately so that they can
+ * change the requested configuration, hopefully into one that we
+ * can work with; if we're loading the con
[libvirt PATCH 18/33] tests: Add descriptors for QCOW2 format firmware builds
Now that we ignore all firmwares that are not in raw format
while performing autoselection, we can have descriptors for
firmware builds in QCOW2 format without breaking anything.
Note that the descriptors are arranged so that they have the
highest priority on aarch64, but the lowest one on x86_64.
This matches the expectation that QCOW2 will quickly be
adopted as the default on aarch64, where its use produces
significant benefits in terms of memory usage, while x86_64
will likely stick with raw for the foreseeable future.
Signed-off-by: Andrea Bolognani
---
.../share/qemu/firmware/65-ovmf-qcow2.json| 35 ++
.../share/qemu/firmware/66-aavmf-qcow2.json | 36 +++
tests/qemufirmwaretest.c | 11 --
3 files changed, 80 insertions(+), 2 deletions(-)
create mode 100644
tests/qemufirmwaredata/usr/share/qemu/firmware/65-ovmf-qcow2.json
create mode 100644
tests/qemufirmwaredata/usr/share/qemu/firmware/66-aavmf-qcow2.json
diff --git a/tests/qemufirmwaredata/usr/share/qemu/firmware/65-ovmf-qcow2.json
b/tests/qemufirmwaredata/usr/share/qemu/firmware/65-ovmf-qcow2.json
new file mode 100644
index 00..3a45cf70f2
--- /dev/null
+++ b/tests/qemufirmwaredata/usr/share/qemu/firmware/65-ovmf-qcow2.json
@@ -0,0 +1,35 @@
+{
+"description": "UEFI firmware for x86_64 virtual machines (QCOW2 format)",
+"interface-types": [
+"uefi"
+],
+"mapping": {
+"device": "flash",
+"mode": "split",
+"executable": {
+"filename": "/usr/share/OVMF/OVMF_CODE.qcow2",
+"format": "qcow2"
+},
+"nvram-template": {
+"filename": "/usr/share/OVMF/OVMF_VARS.qcow2",
+"format": "qcow2"
+}
+},
+"targets": [
+{
+"architecture": "x86_64",
+"machines": [
+"pc-i440fx-*",
+"pc-q35-*"
+]
+}
+],
+"features": [
+"acpi-s3",
+"amd-sev",
+"verbose-dynamic"
+],
+"tags": [
+
+]
+}
diff --git a/tests/qemufirmwaredata/usr/share/qemu/firmware/66-aavmf-qcow2.json
b/tests/qemufirmwaredata/usr/share/qemu/firmware/66-aavmf-qcow2.json
new file mode 100644
index 00..9d80971ee4
--- /dev/null
+++ b/tests/qemufirmwaredata/usr/share/qemu/firmware/66-aavmf-qcow2.json
@@ -0,0 +1,36 @@
+{
+"description": "UEFI firmware for ARM64 virtual machines (QCOW2 format)",
+"interface-types": [
+"uefi"
+],
+"mapping": {
+"device": "flash",
+"mode": "split",
+"executable": {
+"filename": "/usr/share/AAVMF/AAVMF_CODE.qcow2",
+"format": "qcow2"
+},
+"nvram-template": {
+"filename": "/usr/share/AAVMF/AAVMF_VARS.qcow2",
+"format": "qcow2"
+}
+},
+"targets": [
+{
+"architecture": "aarch64",
+"machines": [
+"virt-*"
+]
+}
+],
+"features": [
+
+],
+"tags": [
+"-a AARCH64",
+"-p ArmVirtPkg/ArmVirtQemu.dsc",
+"-t GCC48",
+"-b DEBUG",
+"-D DEBUG_PRINT_ERROR_LEVEL=0x8000"
+]
+}
diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c
index fc3416b2ae..6817c93d9a 100644
--- a/tests/qemufirmwaretest.c
+++ b/tests/qemufirmwaretest.c
@@ -73,6 +73,8 @@ testFWPrecedence(const void *opaque G_GNUC_UNUSED)
PREFIX "/share/qemu/firmware/50-ovmf-sb-keys.json",
PREFIX "/share/qemu/firmware/55-ovmf-sb-combined.json",
PREFIX "/share/qemu/firmware/61-ovmf.json",
+PREFIX "/share/qemu/firmware/65-ovmf-qcow2.json",
+PREFIX "/share/qemu/firmware/66-aavmf-qcow2.json",
PREFIX "/share/qemu/firmware/70-aavmf.json",
NULL
};
@@ -234,6 +236,8 @@ mymain(void)
DO_PARSE_TEST("usr/share/qemu/firmware/55-ovmf-sb-combined.json");
DO_PARSE_TEST("usr/share/qemu/firmware/60-ovmf-sb.json");
DO_PARSE_TEST("usr/share/qemu/firmware/61-ovmf.json");
+DO_PARSE_TEST("usr/share/qemu/firmware/65-ovmf-qcow2.json");
+DO_PARSE_TEST("usr/share/qemu/firmware/66-aavmf-qcow2.json");
DO_PARSE_TEST("usr/share/qemu/firmware/70-aavmf.json");
if (virTe
[libvirt PATCH 11/33] conf: Introduce virDomainLoaderDefParseXMLLoader()
We already handle the element in a separate helper,
which is cleaner than having all the logic in the top-level
virDomainLoaderDefParseXML() function.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_conf.c | 36
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a5578324b9..74c7889e01 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16786,19 +16786,9 @@ virDomainLoaderDefParseXMLNvram(virDomainLoaderDef
*loader,
static int
-virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
- xmlNodePtr loaderNode,
- xmlNodePtr nvramNode,
- xmlNodePtr nvramSourceNode,
- xmlXPathContextPtr ctxt,
- virDomainXMLOption *xmlopt,
- unsigned int flags)
+virDomainLoaderDefParseXMLLoader(virDomainLoaderDef *loader,
+ xmlNodePtr loaderNode)
{
-if (virDomainLoaderDefParseXMLNvram(loader,
-nvramNode, nvramSourceNode,
-ctxt, xmlopt, flags) < 0)
-return -1;
-
if (!loaderNode)
return 0;
@@ -16828,6 +16818,28 @@ virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
}
+static int
+virDomainLoaderDefParseXML(virDomainLoaderDef *loader,
+ xmlNodePtr loaderNode,
+ xmlNodePtr nvramNode,
+ xmlNodePtr nvramSourceNode,
+ xmlXPathContextPtr ctxt,
+ virDomainXMLOption *xmlopt,
+ unsigned int flags)
+{
+if (virDomainLoaderDefParseXMLNvram(loader,
+nvramNode, nvramSourceNode,
+ctxt, xmlopt, flags) < 0)
+return -1;
+
+if (virDomainLoaderDefParseXMLLoader(loader,
+ loaderNode) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
virDomainSchedulerParseCommonAttrs(xmlNodePtr node,
virProcessSchedPolicy *policy,
--
2.39.1
[libvirt PATCH 17/33] qemu: Don't pick firmware with unsupported format
Right now, if the descriptor with the highest priority happens
to describe a firmware in a format other than raw, no domain
that uses autoselection will be able to start.
A better approach is to filter out descriptors that advertise
unsupported formats during autoselection.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index c24bca6183..72036bd82b 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1130,6 +1130,19 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
return false;
}
}
+
+if (STRNEQ(flash->executable.format, "raw")) {
+VIR_DEBUG("Discarding loader with unsupported flash format '%s'",
+ flash->executable.format);
+return false;
+}
+if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
+if (STRNEQ(flash->nvram_template.format, "raw")) {
+VIR_DEBUG("Discarding loader with unsupported nvram template
format '%s'",
+ flash->nvram_template.format);
+return false;
+}
+}
}
if (def->sec) {
@@ -1183,24 +1196,10 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
loader->readonly = VIR_TRISTATE_BOOL_YES;
-if (STRNEQ(flash->executable.format, "raw")) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
- _("unsupported flash format '%s'"),
- flash->executable.format);
-return -1;
-}
-
VIR_FREE(loader->path);
loader->path = g_strdup(flash->executable.filename);
if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
-if (STRNEQ(flash->nvram_template.format, "raw")) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
- _("unsupported nvram template format '%s'"),
- flash->nvram_template.format);
-return -1;
-}
-
if (!loader->nvram) {
loader->nvram = virStorageSourceNew();
loader->nvram->type = VIR_STORAGE_TYPE_FILE;
--
2.39.1
[libvirt PATCH 09/33] qemu: Introduce qemuDomainDefMachinePostParse()
Move all the machine type related parts of
qemuDomainDefPostParse() to a separate helper.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_domain.c | 45 ++
1 file changed, 28 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e9bc0f375d..9db5370055 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4425,6 +4425,31 @@ qemuDomainRecheckInternalPaths(virDomainDef *def,
}
+static int
+qemuDomainDefMachinePostParse(virDomainDef *def,
+ virQEMUCaps *qemuCaps)
+{
+if (!def->os.machine) {
+const char *machine = virQEMUCapsGetPreferredMachine(qemuCaps,
+ def->virtType);
+if (!machine) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("could not get preferred machine for %s type=%s"),
+ def->emulator,
+ virDomainVirtTypeToString(def->virtType));
+return -1;
+}
+
+def->os.machine = g_strdup(machine);
+}
+
+if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
qemuDomainDefVcpusPostParse(virDomainDef *def)
{
@@ -4767,26 +4792,15 @@ qemuDomainDefPostParse(virDomainDef *def,
if (!qemuCaps)
return 1;
+if (qemuDomainDefMachinePostParse(def, qemuCaps) < 0)
+return -1;
+
if (def->os.bootloader || def->os.bootloaderArgs) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("bootloader is not supported by QEMU"));
return -1;
}
-if (!def->os.machine) {
-const char *machine = virQEMUCapsGetPreferredMachine(qemuCaps,
- def->virtType);
-if (!machine) {
-virReportError(VIR_ERR_INVALID_ARG,
- _("could not get preferred machine for %s type=%s"),
- def->emulator,
- virDomainVirtTypeToString(def->virtType));
-return -1;
-}
-
-def->os.machine = g_strdup(machine);
-}
-
if (virDomainDefHasOldStyleROUEFI(def) &&
!def->os.loader->nvram &&
def->os.loader->stateless != VIR_TRISTATE_BOOL_YES) {
@@ -4799,9 +4813,6 @@ qemuDomainDefPostParse(virDomainDef *def,
if (qemuDomainDefAddDefaultDevices(driver, def, qemuCaps) < 0)
return -1;
-if (qemuCanonicalizeMachine(def, qemuCaps) < 0)
-return -1;
-
if (qemuDomainDefSetDefaultCPU(def, driver->hostarch, qemuCaps) < 0)
return -1;
--
2.39.1
[libvirt PATCH 13/33] qemu: Add convenience local variables
This makes the code more compact and less awkward.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_firmware.c | 58 +---
1 file changed, 31 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 98b42bc6fb..13bac9490a 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -986,6 +986,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
const qemuFirmware *fw,
const char *path)
{
+const virDomainLoaderDef *loader = def->os.loader;
size_t i;
qemuFirmwareOSInterface want;
bool supportsS3 = false;
@@ -1000,17 +1001,16 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.firmware);
-if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE &&
-def->os.loader) {
-want =
qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(def->os.loader->type);
+if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE && loader) {
+want = qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(loader->type);
if (fw->mapping.device != QEMU_FIRMWARE_DEVICE_FLASH ||
-STRNEQ(def->os.loader->path,
fw->mapping.data.flash.executable.filename)) {
+STRNEQ(loader->path, fw->mapping.data.flash.executable.filename)) {
VIR_DEBUG("Not matching FW interface %s or loader "
"path '%s' for user provided path '%s'",
qemuFirmwareDeviceTypeToString(fw->mapping.device),
fw->mapping.data.flash.executable.filename,
- def->os.loader->path);
+ loader->path);
return false;
}
}
@@ -1102,8 +1102,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
}
}
-if (def->os.loader &&
-def->os.loader->secure == VIR_TRISTATE_BOOL_YES &&
+if (loader && loader->secure == VIR_TRISTATE_BOOL_YES &&
!requiresSMM) {
VIR_DEBUG("Domain restricts pflash programming to SMM, "
"but firmware '%s' doesn't support SMM", path);
@@ -,13 +1110,15 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
}
if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_FLASH) {
-if (def->os.loader && def->os.loader->stateless ==
VIR_TRISTATE_BOOL_YES) {
-if (fw->mapping.data.flash.mode !=
QEMU_FIRMWARE_FLASH_MODE_STATELESS) {
+const qemuFirmwareMappingFlash *flash = >mapping.data.flash;
+
+if (loader && loader->stateless == VIR_TRISTATE_BOOL_YES) {
+if (flash->mode != QEMU_FIRMWARE_FLASH_MODE_STATELESS) {
VIR_DEBUG("Discarding loader without stateless flash");
return false;
}
} else {
-if (fw->mapping.data.flash.mode != QEMU_FIRMWARE_FLASH_MODE_SPLIT)
{
+if (flash->mode != QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
VIR_DEBUG("Discarding loader without split flash");
return false;
}
@@ -1163,15 +1164,17 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
const qemuFirmwareMappingFlash *flash = >mapping.data.flash;
const qemuFirmwareMappingKernel *kernel = >mapping.data.kernel;
const qemuFirmwareMappingMemory *memory = >mapping.data.memory;
+virDomainLoaderDef *loader = NULL;
size_t i;
switch (fw->mapping.device) {
case QEMU_FIRMWARE_DEVICE_FLASH:
if (!def->os.loader)
def->os.loader = virDomainLoaderDefNew();
+loader = def->os.loader;
-def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
-def->os.loader->readonly = VIR_TRISTATE_BOOL_YES;
+loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
+loader->readonly = VIR_TRISTATE_BOOL_YES;
if (STRNEQ(flash->executable.format, "raw")) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -1180,8 +1183,8 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
return -1;
}
-VIR_FREE(def->os.loader->path);
-def->os.loader->path = g_strdup(flash->executable.filename);
+VIR_FREE(loader->path);
+loader->path = g_strdup(flash->executable.filename);
if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
if (STRNEQ(flash->nvram_template.format, "raw")) {
@@ -1191,21 +1194,21 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver,
return -1;
}
-VIR_FREE(def->os.loader->nvramTemplate);
-def->os.loader->nvramTemplate =
g_strdup(flash->nvra
[libvirt PATCH 10/33] qemu: Introduce qemuDomainDefBootPostParse()
Move all the boot related parts of qemuDomainDefPostParse()
to a separate helper.
Signed-off-by: Andrea Bolognani
---
src/qemu/qemu_domain.c | 37 -
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9db5370055..da1a2413a5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4425,6 +4425,29 @@ qemuDomainRecheckInternalPaths(virDomainDef *def,
}
+static int
+qemuDomainDefBootPostParse(virDomainDef *def,
+ virQEMUDriverConfig *cfg)
+{
+if (def->os.bootloader || def->os.bootloaderArgs) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("bootloader is not supported by QEMU"));
+return -1;
+}
+
+if (virDomainDefHasOldStyleROUEFI(def) &&
+!def->os.loader->nvram &&
+def->os.loader->stateless != VIR_TRISTATE_BOOL_YES) {
+def->os.loader->nvram = virStorageSourceNew();
+def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE;
+def->os.loader->nvram->format = VIR_STORAGE_FILE_RAW;
+qemuDomainNVRAMPathFormat(cfg, def, >os.loader->nvram->path);
+}
+
+return 0;
+}
+
+
static int
qemuDomainDefMachinePostParse(virDomainDef *def,
virQEMUCaps *qemuCaps)
@@ -4795,20 +4818,8 @@ qemuDomainDefPostParse(virDomainDef *def,
if (qemuDomainDefMachinePostParse(def, qemuCaps) < 0)
return -1;
-if (def->os.bootloader || def->os.bootloaderArgs) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("bootloader is not supported by QEMU"));
+if (qemuDomainDefBootPostParse(def, cfg) < 0)
return -1;
-}
-
-if (virDomainDefHasOldStyleROUEFI(def) &&
-!def->os.loader->nvram &&
-def->os.loader->stateless != VIR_TRISTATE_BOOL_YES) {
-def->os.loader->nvram = virStorageSourceNew();
-def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE;
-def->os.loader->nvram->format = VIR_STORAGE_FILE_RAW;
-qemuDomainNVRAMPathFormat(cfg, def, >os.loader->nvram->path);
-}
if (qemuDomainDefAddDefaultDevices(driver, def, qemuCaps) < 0)
return -1;
--
2.39.1
[libvirt PATCH 12/33] conf: introduce virDomainLoaderDefNew()
For now we just allocate the object, so the only advantage is
that invocations are shorter and look a bit nicer.
Later on, its introduction will pay off by letting us change
things in a single spot instead of all over the library.
Signed-off-by: Andrea Bolognani
---
src/bhyve/bhyve_firmware.c | 2 +-
src/conf/domain_conf.c | 8 +++-
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/libxl/libxl_conf.c | 2 +-
src/libxl/xen_xl.c | 4 ++--
src/libxl/xen_xm.c | 2 +-
src/qemu/qemu_firmware.c | 4 ++--
8 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c
index b0f3026c76..cb1b94b4d5 100644
--- a/src/bhyve/bhyve_firmware.c
+++ b/src/bhyve/bhyve_firmware.c
@@ -78,7 +78,7 @@ bhyveFirmwareFillDomain(bhyveConn *driver,
}
if (!def->os.loader)
-def->os.loader = g_new0(virDomainLoaderDef, 1);
+def->os.loader = virDomainLoaderDefNew();
def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
def->os.loader->readonly = VIR_TRISTATE_BOOL_YES;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 74c7889e01..8117cff83e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3714,6 +3714,12 @@ virDomainPanicDefFree(virDomainPanicDef *panic)
g_free(panic);
}
+virDomainLoaderDef *
+virDomainLoaderDefNew(void)
+{
+return g_new0(virDomainLoaderDef, 1);
+}
+
void
virDomainLoaderDefFree(virDomainLoaderDef *loader)
{
@@ -17236,7 +17242,7 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
if (!loaderNode && !nvramNode)
return 0;
-def->os.loader = g_new0(virDomainLoaderDef, 1);
+def->os.loader = virDomainLoaderDefNew();
if (virDomainLoaderDefParseXML(def->os.loader,
loaderNode, nvramNode, nvramSourceNode,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 62f80d653d..96121220ea 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2317,6 +2317,7 @@ struct _virDomainLoaderDef {
char *nvramTemplate; /* user override of path to master nvram */
};
+virDomainLoaderDef *virDomainLoaderDefNew(void);
void virDomainLoaderDefFree(virDomainLoaderDef *loader);
typedef enum {
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 97c3d86217..e550e7139d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -507,6 +507,7 @@ virDomainLeaseRemoveAt;
virDomainLifecycleActionTypeFromString;
virDomainLifecycleActionTypeToString;
virDomainLoaderDefFree;
+virDomainLoaderDefNew;
virDomainLoaderTypeFromString;
virDomainLoaderTypeToString;
virDomainLockFailureTypeFromString;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index cee9f827f7..97c183ebf3 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -643,7 +643,7 @@ libxlMakeDomBuildInfo(virDomainDef *def,
*/
if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) {
if (def->os.loader == NULL)
-def->os.loader = g_new0(virDomainLoaderDef, 1);
+def->os.loader = virDomainLoaderDefNew();
if (def->os.loader->path == NULL)
def->os.loader->path = g_strdup(cfg->firmwares[0]->name);
if (def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_NONE)
diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index 6919325623..cfd56b3b05 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -111,7 +111,7 @@ xenParseXLOS(virConf *conf, virDomainDef *def, virCaps
*caps)
return -1;
if (bios && STREQ(bios, "ovmf")) {
-def->os.loader = g_new0(virDomainLoaderDef, 1);
+def->os.loader = virDomainLoaderDefNew();
def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
def->os.loader->readonly = VIR_TRISTATE_BOOL_YES;
@@ -120,7 +120,7 @@ xenParseXLOS(virConf *conf, virDomainDef *def, virCaps
*caps)
for (i = 0; i < caps->nguests; i++) {
if (caps->guests[i]->ostype == VIR_DOMAIN_OSTYPE_HVM &&
caps->guests[i]->arch.id == def->os.arch) {
-def->os.loader = g_new0(virDomainLoaderDef, 1);
+def->os.loader = virDomainLoaderDefNew();
def->os.loader->path =
g_strdup(caps->guests[i]->arch.defaultInfo.loader);
}
}
diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c
index 081d323c2a..334de071ba 100644
--- a/src/libxl/xen_xm.c
+++ b/src/libxl/xen_xm.c
@@ -42,7 +42,7 @@ xenParseXMOS(virConf *conf, virDomainDef *def)
if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
g_autofree char *boot = NULL;
-def->os.loader = g_new0(virDomainLoaderDef, 1);
+def->os.loader =
[libvirt PATCH 03/33] tests: Rename firmware-manual-efi-rw* tests
These test cases deal with EFI, not BIOS.
Signed-off-by: Andrea Bolognani
---
...rgs => firmware-manual-efi-rw-implicit.x86_64-latest.args} | 0
...os-rw-implicit.xml => firmware-manual-efi-rw-implicit.xml} | 0
...-latest.args => firmware-manual-efi-rw.x86_64-latest.args} | 0
...firmware-manual-bios-rw.xml => firmware-manual-efi-rw.xml} | 0
tests/qemuxml2argvtest.c | 4 ++--
5 files changed, 2 insertions(+), 2 deletions(-)
rename
tests/qemuxml2argvdata/{firmware-manual-bios-rw-implicit.x86_64-latest.args =>
firmware-manual-efi-rw-implicit.x86_64-latest.args} (100%)
rename tests/qemuxml2argvdata/{firmware-manual-bios-rw-implicit.xml =>
firmware-manual-efi-rw-implicit.xml} (100%)
rename tests/qemuxml2argvdata/{firmware-manual-bios-rw.x86_64-latest.args =>
firmware-manual-efi-rw.x86_64-latest.args} (100%)
rename tests/qemuxml2argvdata/{firmware-manual-bios-rw.xml =>
firmware-manual-efi-rw.xml} (100%)
diff --git
a/tests/qemuxml2argvdata/firmware-manual-bios-rw-implicit.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-manual-efi-rw-implicit.x86_64-latest.args
similarity index 100%
rename from
tests/qemuxml2argvdata/firmware-manual-bios-rw-implicit.x86_64-latest.args
rename to
tests/qemuxml2argvdata/firmware-manual-efi-rw-implicit.x86_64-latest.args
diff --git a/tests/qemuxml2argvdata/firmware-manual-bios-rw-implicit.xml
b/tests/qemuxml2argvdata/firmware-manual-efi-rw-implicit.xml
similarity index 100%
rename from tests/qemuxml2argvdata/firmware-manual-bios-rw-implicit.xml
rename to tests/qemuxml2argvdata/firmware-manual-efi-rw-implicit.xml
diff --git a/tests/qemuxml2argvdata/firmware-manual-bios-rw.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-manual-efi-rw.x86_64-latest.args
similarity index 100%
rename from tests/qemuxml2argvdata/firmware-manual-bios-rw.x86_64-latest.args
rename to tests/qemuxml2argvdata/firmware-manual-efi-rw.x86_64-latest.args
diff --git a/tests/qemuxml2argvdata/firmware-manual-bios-rw.xml
b/tests/qemuxml2argvdata/firmware-manual-efi-rw.xml
similarity index 100%
rename from tests/qemuxml2argvdata/firmware-manual-bios-rw.xml
rename to tests/qemuxml2argvdata/firmware-manual-efi-rw.xml
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ae0afb6f1c..f8ea0b6d10 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1129,8 +1129,8 @@ mymain(void)
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_CAPS_LATEST("firmware-manual-efi-rw");
+DO_TEST_CAPS_LATEST("firmware-manual-efi-rw-implicit");
DO_TEST("firmware-manual-efi-secure",
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
QEMU_CAPS_DEVICE_PCI_BRIDGE,
--
2.39.1
[libvirt PATCH 08/33] tests: Add more firmware tests
These cover scenarios such as using the new, more verbose
format of the element to point to a local path, mixing
firmware autoselection with non-local NVRAM files, and
explicitly disabling SMM when using firmware autoselection.
Signed-off-by: Andrea Bolognani
---
...are-auto-efi-nvram-file.x86_64-latest.args | 35 +
.../firmware-auto-efi-nvram-file.xml | 20 ++
...efi-nvram-network-iscsi.x86_64-latest.args | 36 ++
.../firmware-auto-efi-nvram-network-iscsi.xml | 25
...o-efi-nvram-network-nbd.x86_64-latest.args | 35 +
.../firmware-auto-efi-nvram-network-nbd.xml | 22 +++
...irmware-auto-efi-smm-off.x86_64-latest.err | 1 +
.../firmware-auto-efi-smm-off.xml | 18 +
tests/qemuxml2argvtest.c | 4 ++
...ware-auto-efi-nvram-file.x86_64-latest.xml | 33
...-efi-nvram-network-iscsi.x86_64-latest.xml | 38 +++
...to-efi-nvram-network-nbd.x86_64-latest.xml | 35 +
...irmware-auto-efi-smm-off.x86_64-latest.xml | 35 +
tests/qemuxml2xmltest.c | 4 ++
14 files changed, 341 insertions(+)
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-nvram-file.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-nvram-file.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-nvram-network-iscsi.x86_64-latest.args
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-nvram-network-iscsi.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-nvram-network-nbd.x86_64-latest.args
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-nvram-network-nbd.xml
create mode 100644
tests/qemuxml2argvdata/firmware-auto-efi-smm-off.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-smm-off.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-nvram-file.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-nvram-network-iscsi.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-nvram-network-nbd.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/firmware-auto-efi-smm-off.x86_64-latest.xml
diff --git
a/tests/qemuxml2argvdata/firmware-auto-efi-nvram-file.x86_64-latest.args
b/tests/qemuxml2argvdata/firmware-auto-efi-nvram-file.x86_64-latest.args
new file mode 100644
index 00..ec950b8792
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-nvram-file.x86_64-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-guest \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-guest/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-guest/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-guest/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=guest,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-guest/master-key.aes"}'
\
+-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.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":"/path/to/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
pc-i440fx-4.0,usb=off,dump-guest-core=off,memory-backend=pc.ram,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format
\
+-accel tcg \
+-cpu qemu64 \
+-m 1024 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \
+-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-nvram-file.xml
b/tests/qemuxml2argvdata/firmware-auto-efi-nvram-file.xml
new file mode 100644
index 00..83
[libvirt PATCH 07/33] tests: Enable qemuxml2xml for more firmware tests
Some of the test cases had only been added to the xml2argv test program and not to the xml2xml one. Signed-off-by: Andrea Bolognani --- ...mware-auto-efi-stateless.x86_64-latest.xml | 35 ++ ...manual-efi-acpi-aarch64.aarch64-latest.xml | 31 ...ware-manual-efi-acpi-q35.x86_64-latest.xml | 36 ++ ...nual-efi-noacpi-aarch64.aarch64-latest.xml | 30 +++ ...anual-efi-nvram-template.x86_64-latest.xml | 32 ...e-manual-efi-rw-implicit.x86_64-latest.xml | 31 .../firmware-manual-efi-rw.x86_64-latest.xml | 31 ...rmware-manual-efi-secure.x86_64-latest.xml | 37 +++ ...are-manual-efi-stateless.x86_64-latest.xml | 31 ...re-manual-noefi-acpi-q35.x86_64-latest.xml | 34 + ...al-noefi-noacpi-aarch64.aarch64-latest.xml | 28 ++ ...-manual-noefi-noacpi-q35.x86_64-latest.xml | 31 tests/qemuxml2xmltest.c | 13 +++ 13 files changed, 400 insertions(+) create mode 100644 tests/qemuxml2xmloutdata/firmware-auto-efi-stateless.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-acpi-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-acpi-q35.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-noacpi-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-nvram-template.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-rw-implicit.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-rw.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-secure.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-efi-stateless.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-noefi-acpi-q35.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-noefi-noacpi-aarch64.aarch64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-manual-noefi-noacpi-q35.x86_64-latest.xml diff --git a/tests/qemuxml2xmloutdata/firmware-auto-efi-stateless.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-auto-efi-stateless.x86_64-latest.xml new file mode 100644 index 00..143756dbff --- /dev/null +++ b/tests/qemuxml2xmloutdata/firmware-auto-efi-stateless.x86_64-latest.xml @@ -0,0 +1,35 @@ + + guest + 63840878-0deb-4095-97e6-fc444d9bc9fa + 1048576 + 1048576 + 1 + +hvm + + + + + + + +qemu64 + + + destroy + restart + destroy + +/usr/bin/qemu-system-x86_64 + + + + + + + + + + + + diff --git a/tests/qemuxml2xmloutdata/firmware-manual-efi-acpi-aarch64.aarch64-latest.xml b/tests/qemuxml2xmloutdata/firmware-manual-efi-acpi-aarch64.aarch64-latest.xml new file mode 100644 index 00..24e4de6fc6 --- /dev/null +++ b/tests/qemuxml2xmloutdata/firmware-manual-efi-acpi-aarch64.aarch64-latest.xml @@ -0,0 +1,31 @@ + + guest + 63840878-0deb-4095-97e6-fc444d9bc9fa + 1048576 + 1048576 + 1 + +hvm +/usr/share/AAVMF/AAVMF_CODE.fd +/path/to/guest_VARS.fd + + + + + + + +cortex-a15 + + + destroy + restart + destroy + +/usr/bin/qemu-system-aarch64 + + + + + + diff --git a/tests/qemuxml2xmloutdata/firmware-manual-efi-acpi-q35.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-manual-efi-acpi-q35.x86_64-latest.xml new file mode 100644 index 00..2a36c46737 --- /dev/null +++ b/tests/qemuxml2xmloutdata/firmware-manual-efi-acpi-q35.x86_64-latest.xml @@ -0,0 +1,36 @@ + + guest + 63840878-0deb-4095-97e6-fc444d9bc9fa + 1048576 + 1048576 + 1 + +hvm +/usr/share/OVMF/OVMF_CODE.fd +/path/to/guest_VARS.fd + + + + + + +qemu64 + + + destroy + restart + destroy + +/usr/bin/qemu-system-x86_64 + + + + + + + + + + + + diff --git a/tests/qemuxml2xmloutdata/firmware-manual-efi-noacpi-aarch64.aarch64-latest.xml b/tests/qemuxml2xmloutdata/firmware-manual-efi-noacpi-aarch64.aarch64-latest.xml new file mode 100644 index 00..414c1d6611 --- /dev/null +++ b/tests/qemuxml2xmloutdata/firmware-manual-efi-noacpi-aarch64.aarch64-latest.xml @@ -0,0 +1,30 @@ + + guest + 63840878-0deb-4095-97e6-fc444d9bc9fa + 1048576 + 1048576 + 1 + +hvm +/usr/share/AAVMF/AAVMF_CODE.fd +/path/to/guest_VARS.fd + + + + + + +cortex-a15 + + + destroy + restart + destroy + +/usr/bin/qemu-system-aarch64 + + + + + + diff --git a/tests/qemuxml2xmloutdata/firmware-manual-efi-nvram-template.x86_64-latest.xml b/tests/qemuxml2xmloutdata/firmware-manual-efi-nvram-template.x86_64-latest.xml new file mode 100644 index 00..3b79af418a
[libvirt PATCH 05/33] tests: Move firmware tests to CAPS_LATEST
This is already the case for the vast majority, but a few are
using explicit capabilities lists.
Signed-off-by: Andrea Bolognani
---
...nual-bios-not-stateless.x86_64-latest.err} | 0
...-manual-bios-stateless.x86_64-latest.args} | 7 ++--
...> firmware-manual-bios.x86_64-latest.args} | 7 ++--
...nual-efi-acpi-aarch64.aarch64-latest.args} | 7 ++--
...re-manual-efi-acpi-q35.x86_64-latest.args} | 7 ++--
...ware-manual-efi-no-path.x86_64-latest.err} | 0
...al-efi-noacpi-aarch64.aarch64-latest.args} | 7 ++--
...e-manual-efi-noacpi-q35.x86_64-latest.err} | 0
...ware-manual-efi-secure.x86_64-latest.args} | 7 ++--
...=> firmware-manual-efi.x86_64-latest.args} | 7 ++--
...ual-noefi-acpi-aarch64.aarch64-latest.err} | 0
...-manual-noefi-acpi-q35.x86_64-latest.args} | 7 ++--
...-noefi-noacpi-aarch64.aarch64-latest.args} | 7 ++--
...anual-noefi-noacpi-q35.x86_64-latest.args} | 7 ++--
tests/qemuxml2argvtest.c | 36 ---
...e-manual-bios-stateless.x86_64-latest.xml} | 3 ++
...=> firmware-manual-bios.x86_64-latest.xml} | 3 ++
... => firmware-manual-efi.x86_64-latest.xml} | 3 ++
tests/qemuxml2xmltest.c | 6 ++--
19 files changed, 76 insertions(+), 45 deletions(-)
rename tests/qemuxml2argvdata/{firmware-manual-bios-not-stateless.err =>
firmware-manual-bios-not-stateless.x86_64-latest.err} (100%)
rename tests/qemuxml2argvdata/{firmware-manual-bios-stateless.args =>
firmware-manual-bios-stateless.x86_64-latest.args} (68%)
rename tests/qemuxml2argvdata/{firmware-manual-bios.args =>
firmware-manual-bios.x86_64-latest.args} (68%)
rename tests/qemuxml2argvdata/{firmware-manual-efi-acpi-aarch64.args =>
firmware-manual-efi-acpi-aarch64.aarch64-latest.args} (73%)
rename tests/qemuxml2argvdata/{firmware-manual-efi-acpi-q35.args =>
firmware-manual-efi-acpi-q35.x86_64-latest.args} (75%)
rename tests/qemuxml2argvdata/{firmware-manual-efi-no-path.err =>
firmware-manual-efi-no-path.x86_64-latest.err} (100%)
rename tests/qemuxml2argvdata/{firmware-manual-efi-noacpi-aarch64.args =>
firmware-manual-efi-noacpi-aarch64.aarch64-latest.args} (73%)
rename tests/qemuxml2argvdata/{firmware-manual-efi-noacpi-q35.err =>
firmware-manual-efi-noacpi-q35.x86_64-latest.err} (100%)
rename tests/qemuxml2argvdata/{firmware-manual-efi-secure.args =>
firmware-manual-efi-secure.x86_64-latest.args} (76%)
rename tests/qemuxml2argvdata/{firmware-manual-efi.args =>
firmware-manual-efi.x86_64-latest.args} (75%)
rename tests/qemuxml2argvdata/{firmware-manual-noefi-acpi-aarch64.err =>
firmware-manual-noefi-acpi-aarch64.aarch64-latest.err} (100%)
rename tests/qemuxml2argvdata/{firmware-manual-noefi-acpi-q35.args =>
firmware-manual-noefi-acpi-q35.x86_64-latest.args} (68%)
rename tests/qemuxml2argvdata/{firmware-manual-noefi-noacpi-aarch64.args =>
firmware-manual-noefi-noacpi-aarch64.aarch64-latest.args} (65%)
rename tests/qemuxml2argvdata/{firmware-manual-noefi-noacpi-q35.args =>
firmware-manual-noefi-noacpi-q35.x86_64-latest.args} (68%)
rename tests/qemuxml2xmloutdata/{firmware-manual-bios-stateless.xml =>
firmware-manual-bios-stateless.x86_64-latest.xml} (89%)
rename tests/qemuxml2xmloutdata/{firmware-manual-bios.xml =>
firmware-manual-bios.x86_64-latest.xml} (89%)
rename tests/qemuxml2xmloutdata/{firmware-manual-efi.xml =>
firmware-manual-efi.x86_64-latest.xml} (90%)
diff --git a/tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.err
b/tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.x86_64-latest.err
similarity index 100%
rename from tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.err
rename to
tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.x86_64-latest.err
diff --git a/tests/qemuxml2argvdata/firmware-manual-bios-stateless.args
b/tests/qemuxml2argvdata/firmware-manual-bios-stateless.x86_64-latest.args
similarity index 68%
rename from tests/qemuxml2argvdata/firmware-manual-bios-stateless.args
rename to
tests/qemuxml2argvdata/firmware-manual-bios-stateless.x86_64-latest.args
index 3aa4fa90fa..20953097ef 100644
--- a/tests/qemuxml2argvdata/firmware-manual-bios-stateless.args
+++ b/tests/qemuxml2argvdata/firmware-manual-bios-stateless.x86_64-latest.args
@@ -9,11 +9,13 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-test-bios/.config \
/usr/bin/qemu-system-x86_64 \
-name guest=test-bios,debug-threads=on \
-S \
--object
secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-test-bios/master-key.aes
\
--machine pc,usb=off,dump-guest-core=off \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-test-bios/master-key.aes"}'
\
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \
-accel tcg \
+-cpu qemu64 \
-bios /usr/share/seabios/bios.bin \
-m 1024 \
+-object '{"qom-type":"memory-backend-ram"
[libvirt PATCH 02/33] tests: Set nvramDir in qemuxml2xmltest
We already do this in qemuxml2argvtest.
Right now setting this doesn't change anything, but it will
become relevant later.
Signed-off-by: Andrea Bolognani
---
tests/qemuxml2xmltest.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index ae9716062d..907762190b 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -163,6 +163,9 @@ mymain(void)
cfg = virQEMUDriverGetConfig();
driver.privileged = true;
+VIR_FREE(cfg->nvramDir);
+cfg->nvramDir = g_strdup("/var/lib/libvirt/qemu/nvram");
+
if (!(conn = virGetConnect()))
goto cleanup;
--
2.39.1
[libvirt PATCH 04/33] tests: Use x86_64 for all x86 firmware tests
Most test cases are on 64-bit architectures already, but there are a couple of exceptions. Right now this works, but it will no longer fly after some upcoming changes. Prepare for those by switching away from 32-bit architectures. Signed-off-by: Andrea Bolognani --- tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.xml | 4 ++-- tests/qemuxml2argvdata/firmware-manual-bios-stateless.args| 2 +- tests/qemuxml2argvdata/firmware-manual-bios-stateless.xml | 4 ++-- tests/qemuxml2argvdata/firmware-manual-bios.args | 2 +- tests/qemuxml2argvdata/firmware-manual-bios.xml | 4 ++-- tests/qemuxml2xmloutdata/firmware-manual-bios-stateless.xml | 4 ++-- tests/qemuxml2xmloutdata/firmware-manual-bios.xml | 4 ++-- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.xml b/tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.xml index b60878ca0b..6e5f87354a 100644 --- a/tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.xml +++ b/tests/qemuxml2argvdata/firmware-manual-bios-not-stateless.xml @@ -4,11 +4,11 @@ 1048576 1 -hvm +hvm /usr/share/seabios/bios.bin - /usr/bin/qemu-system-i386 + /usr/bin/qemu-system-x86_64 diff --git a/tests/qemuxml2argvdata/firmware-manual-bios-stateless.args b/tests/qemuxml2argvdata/firmware-manual-bios-stateless.args index 4f2ac0c682..3aa4fa90fa 100644 --- a/tests/qemuxml2argvdata/firmware-manual-bios-stateless.args +++ b/tests/qemuxml2argvdata/firmware-manual-bios-stateless.args @@ -6,7 +6,7 @@ LOGNAME=test \ XDG_DATA_HOME=/tmp/lib/domain--1-test-bios/.local/share \ XDG_CACHE_HOME=/tmp/lib/domain--1-test-bios/.cache \ XDG_CONFIG_HOME=/tmp/lib/domain--1-test-bios/.config \ -/usr/bin/qemu-system-i386 \ +/usr/bin/qemu-system-x86_64 \ -name guest=test-bios,debug-threads=on \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-test-bios/master-key.aes \ diff --git a/tests/qemuxml2argvdata/firmware-manual-bios-stateless.xml b/tests/qemuxml2argvdata/firmware-manual-bios-stateless.xml index 9d6f4e4c83..fa644af8b9 100644 --- a/tests/qemuxml2argvdata/firmware-manual-bios-stateless.xml +++ b/tests/qemuxml2argvdata/firmware-manual-bios-stateless.xml @@ -4,11 +4,11 @@ 1048576 1 -hvm +hvm /usr/share/seabios/bios.bin - /usr/bin/qemu-system-i386 + /usr/bin/qemu-system-x86_64 diff --git a/tests/qemuxml2argvdata/firmware-manual-bios.args b/tests/qemuxml2argvdata/firmware-manual-bios.args index 4f2ac0c682..3aa4fa90fa 100644 --- a/tests/qemuxml2argvdata/firmware-manual-bios.args +++ b/tests/qemuxml2argvdata/firmware-manual-bios.args @@ -6,7 +6,7 @@ LOGNAME=test \ XDG_DATA_HOME=/tmp/lib/domain--1-test-bios/.local/share \ XDG_CACHE_HOME=/tmp/lib/domain--1-test-bios/.cache \ XDG_CONFIG_HOME=/tmp/lib/domain--1-test-bios/.config \ -/usr/bin/qemu-system-i386 \ +/usr/bin/qemu-system-x86_64 \ -name guest=test-bios,debug-threads=on \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-test-bios/master-key.aes \ diff --git a/tests/qemuxml2argvdata/firmware-manual-bios.xml b/tests/qemuxml2argvdata/firmware-manual-bios.xml index 3e1946029c..f5c33bc306 100644 --- a/tests/qemuxml2argvdata/firmware-manual-bios.xml +++ b/tests/qemuxml2argvdata/firmware-manual-bios.xml @@ -4,11 +4,11 @@ 1048576 1 -hvm +hvm /usr/share/seabios/bios.bin - /usr/bin/qemu-system-i386 + /usr/bin/qemu-system-x86_64 diff --git a/tests/qemuxml2xmloutdata/firmware-manual-bios-stateless.xml b/tests/qemuxml2xmloutdata/firmware-manual-bios-stateless.xml index de5ecb96dc..2fb7b49a79 100644 --- a/tests/qemuxml2xmloutdata/firmware-manual-bios-stateless.xml +++ b/tests/qemuxml2xmloutdata/firmware-manual-bios-stateless.xml @@ -5,7 +5,7 @@ 1048576 1 -hvm +hvm /usr/share/seabios/bios.bin @@ -14,7 +14,7 @@ restart destroy -/usr/bin/qemu-system-i386 +/usr/bin/qemu-system-x86_64 diff --git a/tests/qemuxml2xmloutdata/firmware-manual-bios.xml b/tests/qemuxml2xmloutdata/firmware-manual-bios.xml index 75bb6038ca..47432ac525 100644 --- a/tests/qemuxml2xmloutdata/firmware-manual-bios.xml +++ b/tests/qemuxml2xmloutdata/firmware-manual-bios.xml @@ -5,7 +5,7 @@ 1048576 1 -hvm +hvm /usr/share/seabios/bios.bin @@ -14,7 +14,7 @@ restart destroy -/usr/bin/qemu-system-i386 +/usr/bin/qemu-system-x86_64 -- 2.39.1
[libvirt PATCH 01/33] docs: Fix documentation for loader.stateless attribute
It works exactly the other way around. Signed-off-by: Andrea Bolognani --- docs/formatdomain.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 36c6d87907..8407bab1ba 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -255,7 +255,7 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. 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 (:since:`Since 8.6.0`) - can be used to control this behaviour, when set to ``no`` NVRAM will never + can be used to control this behaviour, when set to ``yes`` NVRAM will never be created. ``nvram`` Some UEFI firmwares may want to use a non-volatile memory to store some -- 2.39.1
[libvirt PATCH 00/33] qemu: Move firmware selection to postparse and add support for QCOW2 firmware
Motivation for these changes can be found in the commit message
for patch 20 ("qemu: Move firmware selection from startup to
postparse") as well as [RHBZ#2161965].
Patches 01-17 are preparatory fixes/improvements/cleanups.
Patches 19-20 move firmware selection from startup to postparse,
and patches 21-22 clean up a bit after that change. Patch 20 in
particular is significantly larger than I would have liked, but
I haven't been able to come up with a way to split it while
still preserving bisectability and making things clearer instead
of complicating them. If anyone has ideas in this regard, please
let me know!
Patches 23-27 add support for choosing a firmware format, but
are effectively no-op because formats other than raw are still
rejected at this point.
Patches 28-30 add support for QCOW2 format firmware in the QEMU
driver.
Patches 31-33 document the changes.
[RHBZ#2161965] https://bugzilla.redhat.com/show_bug.cgi?id=2161965
Andrea Bolognani (33):
docs: Fix documentation for loader.stateless attribute
tests: Set nvramDir in qemuxml2xmltest
tests: Rename firmware-manual-efi-rw* tests
tests: Use x86_64 for all x86 firmware tests
tests: Move firmware tests to CAPS_LATEST
tests: Unify input files for firmware tests
tests: Enable qemuxml2xml for more firmware tests
tests: Add more firmware tests
qemu: Introduce qemuDomainDefMachinePostParse()
qemu: Introduce qemuDomainDefBootPostParse()
conf: Introduce virDomainLoaderDefParseXMLLoader()
conf: introduce virDomainLoaderDefNew()
qemu: Add convenience local variables
qemu: Only fill nvramTemplate for local sources
qemu: Clear os.firmwareFeatures after autoselection
qemu: Don't pick firmware that requires SMM when smm=off
qemu: Don't pick firmware with unsupported format
tests: Add descriptors for QCOW2 format firmware builds
conf: Export virDomainDefOSValidate()
qemu: Move firmware selection from startup to postparse
qemu: Move qemuDomainNVRAMPathFormat() to qemu_firmware
qemu: Introduce qemuFirmwareEnsureNVRAM()
conf: Change handling for empty NVRAM path
conf: Parse firmware format
drivers: Reject unsupported firmware formats
qemu: Filter firmwares based on format
qemu: Propagate firmware format
conf: Accept QCOW2 firmware format
qemu: Add support for QCOW2 format firmware
tests: Add more firmware tests
docs: Document firmware format attribute
news: Document changes to firmware autoselection
news: Document support for QCOW2 format firmware
NEWS.rst | 16 +
docs/formatdomain.rst | 11 +-
src/bhyve/bhyve_firmware.c| 9 +-
src/conf/domain_conf.c| 123 -
src/conf/domain_conf.h| 2 +
src/conf/domain_validate.c| 4 +-
src/conf/domain_validate.h| 3 +
src/conf/schemas/domaincommon.rng | 15 +
src/libvirt_private.syms | 2 +
src/libxl/libxl_conf.c| 9 +-
src/libxl/xen_xl.c| 4 +-
src/libxl/xen_xm.c| 2 +-
src/qemu/qemu_domain.c| 113 +++--
src/qemu/qemu_domain.h| 5 -
src/qemu/qemu_driver.c| 2 -
src/qemu/qemu_firmware.c | 460 +-
src/qemu/qemu_firmware.h | 3 +-
src/qemu/qemu_process.c | 33 +-
.../share/qemu/firmware/65-ovmf-qcow2.json| 35 ++
.../share/qemu/firmware/66-aavmf-qcow2.json | 36 ++
tests/qemufirmwaretest.c | 11 +-
.../firmware-auto-bios-not-stateless.xml | 4 +-
.../firmware-auto-bios-nvram.xml | 6 +-
...are-auto-bios-stateless.x86_64-latest.args | 16 +-
.../firmware-auto-bios-stateless.xml | 4 +-
.../firmware-auto-bios.x86_64-latest.args | 16 +-
tests/qemuxml2argvdata/firmware-auto-bios.xml | 4 +-
...mware-auto-efi-aarch64.aarch64-latest.args | 22 +-
.../firmware-auto-efi-aarch64.xml | 4 +-
...ware-auto-efi-enrolled-keys-no-secboot.xml | 4 +-
...-auto-efi-enrolled-keys.x86_64-latest.args | 18 +-
.../firmware-auto-efi-enrolled-keys.xml | 4 +-
...fi-format-loader-qcow2.x86_64-latest.args} | 24 +-
...firmware-auto-efi-format-loader-qcow2.xml} | 6 +-
...efi-format-loader-raw.aarch64-latest.args} | 5 +-
...> firmware-auto-efi-format-loader-raw.xml} | 5 +-
...auto-efi-format-mismatch.x86_64-latest.err | 1 +
... => firmware-auto-efi-format-mismatch.xml} | 7 +-
...vram-qcow2-network-nbd.x86_64-latest.args} | 22 +-
...to-efi-format-nvram-qcow2-network-nbd.xml} | 11 +-
...ormat-nvram-qcow2-path.x86_64-latest.args} | 24 +-
...ware-auto-efi-format-nvram-qcow2-path.xml} | 6 +-
...efi-format-nvram-qcow2.x86_64-latest.args} | 24 +-
... firmware-auto-efi-format-nvram-qcow2.xml} | 6
Re: [libvirt PATCH] gitlab: Ask users not to attach screenshots to bug reports
On Fri, Feb 10, 2023 at 11:11:03AM +, Daniel P. Berrangé wrote: > On Fri, Feb 10, 2023 at 02:27:46AM -0800, Andrea Bolognani wrote: > > On Fri, Feb 10, 2023 at 09:50:25AM +, Daniel P. Berrangé wrote: > > > IMHO if someone provides a bug report with a screenshot and there is not > > > sufficient accompanying information, just nicely ask them to explain the > > > problem in a little more detail. We need todo that with plenty of bugs > > > which don't have screenshots either, and screenshots can be helpful in > > > some cases. > > > > Sure, but there's a difference between asking for more details, which > > can always be done as a follow-up, and asking users to provide > > information as plain text instead of screenshots. In the latter case, > > once the screenshot has been attached, that's it, there's no going > > back. > > Why can't we ask users to add the XML as text afterwards ? I've often > done just that when people have attached images, if it was useful to > me to have it in text format. That feels worse, from the user's point of view, than being asked to do that ahead of time, because it introduces friction and delays. Anyway, clearly people are not into this patch and I don't care enough about it to fiercely advocate for its inclusion, so let's all grab our favorite SNACK and move on :) -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH] gitlab: Ask users not to attach screenshots to bug reports
On Fri, Feb 10, 2023 at 09:50:25AM +, Daniel P. Berrangé wrote: > On Fri, Feb 10, 2023 at 10:34:11AM +0100, Andrea Bolognani wrote: > > + > > I think this is pretty user hostile. When filing bugs, the more rules a > project imposes on the bug report, the less likely I am to go ahead with > filing it, especially when the rules are worded in a very direct manner > that makes it feel like you'll be flamed for getting it wrong. For the person reporting the bug, taking a screenshot and attaching it or copying and pasting the corresponding text shouldn't make any difference in terms of effort. If anything, the latter is probably going to be less work. Yeah, the wording I've proposed can perhaps be seen as hostile. Any suggestions on how to improve it, while still making it pop out so that users aren't likely to miss it? > IMHO if someone provides a bug report with a screenshot and there is not > sufficient accompanying information, just nicely ask them to explain the > problem in a little more detail. We need todo that with plenty of bugs > which don't have screenshots either, and screenshots can be helpful in > some cases. Sure, but there's a difference between asking for more details, which can always be done as a follow-up, and asking users to provide information as plain text instead of screenshots. In the latter case, once the screenshot has been attached, that's it, there's no going back. -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] gitlab: Ask users not to attach screenshots to bug reports
Hopefully including this request in ALL CAPITAL LETTERS in the issue template will cut down on the number of screenshots that end up cluttering bug reports by at least a tiny bit. Signed-off-by: Andrea Bolognani --- .gitlab/issue_templates/bug.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab/issue_templates/bug.md b/.gitlab/issue_templates/bug.md index f220671a06..b923be0ca0 100644 --- a/.gitlab/issue_templates/bug.md +++ b/.gitlab/issue_templates/bug.md @@ -17,3 +17,4 @@ ## Additional information + -- 2.39.1
Re: [PATCH 3/6] tests: add test cases for device pvpanic-pci
On Thu, Feb 09, 2023 at 07:47:45AM -0800, Andrea Bolognani wrote: > On Wed, Feb 08, 2023 at 12:49:02PM +0100, Kristina Hanicova wrote: > > + > > You can use > > > > here and in the other input XMLs for slightly smaller output files. You can similarly drop the disk, as well as the fdc and ide controllers, from the x86_64 input file. In general, the smaller the input and output files are, the more obvious it becomes what scenario the test is supposed to cover and the easier it is to focus on just that. Note that none of this is a requirement, just my own (fairly strong) preference :) So if you don't agree that spending a bit of time minimizing the test files is a worthwhile investment, feel free to ignore these comments. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 4/6] qemu: assign PCI address to device pvpanic-pci
On Thu, Feb 09, 2023 at 06:26:14PM +0100, Eric Auger wrote:
> On 2/9/23 17:33, Andrea Bolognani wrote:
> > On Wed, Feb 08, 2023 at 12:49:03PM +0100, Kristina Hanicova wrote:
> >> +++ b/src/qemu/qemu_domain_address.c
> >> @@ -1062,10 +1062,24 @@
> >> qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
> >> }
> >> break;
> >>
> >> +case VIR_DOMAIN_DEVICE_PANIC:
> >> +switch ((virDomainPanicModel) dev->data.panic->model) {
> >> +case VIR_DOMAIN_PANIC_MODEL_PVPANIC:
> >> +return pciFlags;
> >
> > So, this is correct, because pvpanic-pci is indeed a conventional PCI
> > device (as opposed to a PCI Express device).
> >
> > However, when used with a PCIe-based machine such as aarch64/virt,
> > these flags result in the device being plugged into a
> > pcie-pci-bridge, which in turn is plugged into a pcie-root-port,
> > itself plugged into pcie.0.
> >
> > While this seems to work fine, it's also kind of a waste of PCI
> > controllers. For a "system" device such as pvpanic-pci, I think
> > plugging directly into pcie.0 might be more appropriate. This is
> > particularly true of x86/q35, where with the current implementation
> > switching from ISA pvpanic to pvpanic-pci would result in adding two
> > extra PCI controllers.
> >
> > So I think this would be a good fit for the
> > VIR_PCI_CONNECT_INTEGRATED flag that I've added a while ago for use
> > with virtio-iommu. While this would technically result in libvirt
> > being more restrictive than QEMU in what PCI addresses it accepts for
> > pvpanic-pci, I don't think this would limit users in practice, and in
> > the default case (libvirt automatically picking the address) the
> > resulting configuration would be preferable.
> >
> > We can always decide to relax this restriction later down the line,
> > if it turns out to really be a limiting factor.
> >
> > Eric, what do you think about this idea?
>
> I do agree. If we can avoid putting that device downstream to a
> pcie-to-pci bridge that's much better. Putting it onto pcie.0 looks more
> appropriate to me indeed.
Thanks for the input! Let's go with this approach then, unless
someone raises concerns with it.
> > (There's one caveat: VIR_PCI_CONNECT_INTEGRATED doesn't seem to work
> > with pciFlags at the moment O:-) It works fine with pcieFlags and
> > virtioFlags. I'll try to figure out why that's the case.)
Fixed by this patch:
https://listman.redhat.com/archives/libvir-list/2023-February/237688.html
--
Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] conf: Allow conventional PCI devices to be marked as integrated
Integrated PCI devices can be either PCIe (virtio-iommu) or
conventional PCI (pvpanic-pci). Right now libvirt will refuse
to assign an address on pcie.0 for the latter, but that's an
undesirable limitation that we can easily remove.
Signed-off-by: Andrea Bolognani
---
src/conf/domain_addr.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 76f9c12ca6..b6534f502c 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -306,8 +306,11 @@ virDomainPCIAddressFlagsCompatible(virPCIDeviceAddress
*addr,
if (addr->bus == 0) {
/* pcie-root doesn't usually allow endpoint devices to be
* plugged directly into it, but for integrated devices
- * that's exactly what we want */
-busFlags |= VIR_PCI_CONNECT_AUTOASSIGN;
+ * that's exactly what we want. It also refuses conventional
+ * PCI devices by default, but in the case of integrated
+ * devices both types are fine */
+busFlags |= VIR_PCI_CONNECT_TYPE_PCI_DEVICE |
+VIR_PCI_CONNECT_AUTOASSIGN;
} else {
if (reportError) {
virReportError(errType,
--
2.39.1
Re: [PATCH 6/6] docs: document panic device 'pvpanic-pci'
On Thu, Feb 09, 2023 at 06:32:12PM +0100, Peter Krempa wrote: > On Thu, Feb 09, 2023 at 08:41:01 -0800, Andrea Bolognani wrote: > > On Wed, Feb 08, 2023 at 01:09:05PM +0100, Peter Krempa wrote: > > > On Wed, Feb 08, 2023 at 12:49:05 +0100, Kristina Hanicova wrote: > > > > +++ b/docs/formatdomain.rst > > > > @@ -7940,6 +7940,7 @@ Example: usage of panic configuration > > > > - 'hyperv' - for Hyper-V crash CPU feature. :since:`Since 1.3.0, > > > > QEMU and > > > >KVM only` > > > > - 's390' - default for S390 guests. :since:`Since 1.3.5` > > > > + - 'pvpanic' - for PCI pvpanic devicen :since:`Since 9.1.0, QEMU > > > > and KVM only` > > > > > > s/devicen/device/ > > > > > > Also is KVM really required? I'd expect that it will also work with TCG > > > VMs. > > > > I think so as well. But to be honest I'm unclear on what "QEMU and > > KVM only", as used extensively throughout the document, is intended > > to mean. Does it mean "only when using the QEMU driver and its KVM > > domain type", or rather "only when using the QEMU or KVM domain > > type"? The latter sounds more likely to me, and it would be accurate > > for the pvpanic-pci device. > > Generaly in our docs "QEMU only" means that it works only with the qemu > driver/hypervisor, thus I don't think the interpretation that type='qemu'> or kvm will work here. A few counterexamples: > Watchdog devices > A virtual hardware watchdog device can be added to the guest via > the watchdog element. Since 0.7.3, QEMU and KVM only > Memory balloon device > A virtual memory balloon device is added to all Xen and KVM/QEMU > guests. It will be seen as memballoon element. It will be > automatically added when appropriate, so there is no need to > explicitly add this element in the guest XML unless a specific > PCI slot needs to be assigned. Since 0.8.3, Xen, QEMU and KVM > only > Firmware > The firmware attribute allows management applications to > automatically fill and elements and possibly > enable some features required by selected firmware. [...] Since > 5.2.0 (QEMU and KVM only) > ROM > The optional file attribute contains an absolute path to a binary > file to be presented to the guest as the device's ROM BIOS. This > can be useful, for example, to provide a PXE boot ROM for a > virtual function of an sr-iov capable ethernet device (which has > no boot ROMs for the VFs). Since 0.9.10 (QEMU and KVM only) All of the above work perfectly fine with TCG, at least as far as I know. There are a few instances of "(QEMU/KVM only)" in the document, and even a couple of "(KVM only)". tl;dr We're awfully inconsistent about this. I'm okay with dropping the "and KVM" part from this patch. I'd also be very much okay with someone[1] going through the document and changing it to use a single, unambiguous way to indicate whether a feature works with TCG or is restricted with KVM. [1] No, I'm not volunteering to be that someone :) -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 0/6] add support for pvpanic-pci device
On Wed, Feb 08, 2023 at 12:48:59PM +0100, Kristina Hanicova wrote: > qemu: introduce QEMU_CAPS_DEVICE_PANIC_PCI > conf: add panic model 'pvpanic' > tests: add test cases for device pvpanic-pci > qemu: assign PCI address to device pvpanic-pci > tests: add case for pvpanic-pci without address > docs: document panic device 'pvpanic-pci' Stray observations: * an entry in the release notes would certainly be warranted for this nice user-visible improvement; * I noticed that panic devices don't currently have aliases associated with them: this looks like an oversight rather than a conscious design decision, and it would be great if you would consider addressing that in a follow-up series :) -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 6/6] docs: document panic device 'pvpanic-pci'
On Wed, Feb 08, 2023 at 01:09:05PM +0100, Peter Krempa wrote: > On Wed, Feb 08, 2023 at 12:49:05 +0100, Kristina Hanicova wrote: > > +++ b/docs/formatdomain.rst > > @@ -7940,6 +7940,7 @@ Example: usage of panic configuration > > - 'hyperv' - for Hyper-V crash CPU feature. :since:`Since 1.3.0, QEMU > > and > >KVM only` > > - 's390' - default for S390 guests. :since:`Since 1.3.5` > > + - 'pvpanic' - for PCI pvpanic devicen :since:`Since 9.1.0, QEMU and > > KVM only` > > s/devicen/device/ > > Also is KVM really required? I'd expect that it will also work with TCG > VMs. I think so as well. But to be honest I'm unclear on what "QEMU and KVM only", as used extensively throughout the document, is intended to mean. Does it mean "only when using the QEMU driver and its KVM domain type", or rather "only when using the QEMU or KVM domain type"? The latter sounds more likely to me, and it would be accurate for the pvpanic-pci device. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 5/6] tests: add case for pvpanic-pci without address
On Wed, Feb 08, 2023 at 12:49:04PM +0100, Kristina Hanicova wrote:
> +++ b/tests/qemuxml2xmltest.c
> @@ -910,6 +910,7 @@ mymain(void)
>
> DO_TEST_CAPS_LATEST("pvpanic-pci-x86_64");
> DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-aarch64", "aarch64");
> +DO_TEST_CAPS_ARCH_LATEST("pvpanic-pci-no-address-aarch64", "aarch64");
You have forgotten the corresponding qemuxml2argvtest change. The
output file is there, so I know you have made the change locally ;)
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 4/6] qemu: assign PCI address to device pvpanic-pci
On Wed, Feb 08, 2023 at 12:49:03PM +0100, Kristina Hanicova wrote:
> +++ b/src/qemu/qemu_domain_address.c
> @@ -1062,10 +1062,24 @@
> qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev,
> }
> break;
>
> +case VIR_DOMAIN_DEVICE_PANIC:
> +switch ((virDomainPanicModel) dev->data.panic->model) {
> +case VIR_DOMAIN_PANIC_MODEL_PVPANIC:
> +return pciFlags;
So, this is correct, because pvpanic-pci is indeed a conventional PCI
device (as opposed to a PCI Express device).
However, when used with a PCIe-based machine such as aarch64/virt,
these flags result in the device being plugged into a
pcie-pci-bridge, which in turn is plugged into a pcie-root-port,
itself plugged into pcie.0.
While this seems to work fine, it's also kind of a waste of PCI
controllers. For a "system" device such as pvpanic-pci, I think
plugging directly into pcie.0 might be more appropriate. This is
particularly true of x86/q35, where with the current implementation
switching from ISA pvpanic to pvpanic-pci would result in adding two
extra PCI controllers.
So I think this would be a good fit for the
VIR_PCI_CONNECT_INTEGRATED flag that I've added a while ago for use
with virtio-iommu. While this would technically result in libvirt
being more restrictive than QEMU in what PCI addresses it accepts for
pvpanic-pci, I don't think this would limit users in practice, and in
the default case (libvirt automatically picking the address) the
resulting configuration would be preferable.
We can always decide to relax this restriction later down the line,
if it turns out to really be a limiting factor.
Eric, what do you think about this idea?
(There's one caveat: VIR_PCI_CONNECT_INTEGRATED doesn't seem to work
with pciFlags at the moment O:-) It works fine with pcieFlags and
virtioFlags. I'll try to figure out why that's the case.)
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 3/6] tests: add test cases for device pvpanic-pci
On Wed, Feb 08, 2023 at 12:49:02PM +0100, Kristina Hanicova wrote: > +++ b/tests/qemuxml2argvdata/pvpanic-pci-aarch64.xml > @@ -0,0 +1,20 @@ > + > + guest > + 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 > + 1048576 > + 1 > + > +hvm > + > + > + > + > + > +/usr/bin/qemu-system-aarch64 > + > + You can use here and in the other input XMLs for slightly smaller output files. > + > + function='0x0'/> > + This explicit address could be on pcie.0, e.g. to prevent the pcie-pci-bridge and pci-bridge controllers from being added and, again, produce slightly smaller output files. More discussion about device placement in the upcoming reply to a later patch :) -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 2/6] conf: add panic model 'pvpanic'
On Wed, Feb 08, 2023 at 01:05:59PM +0100, Peter Krempa wrote:
> On Wed, Feb 08, 2023 at 12:49:01 +0100, Kristina Hanicova wrote:
> > +++ b/src/qemu/qemu_command.c
> > @@ -9557,6 +9557,25 @@ qemuBuildPanicCommandLine(virCommand *cmd,
> > break;
> > }
> >
> > +case VIR_DOMAIN_PANIC_MODEL_PVPANIC: {
> > +g_autoptr(virJSONValue) props = NULL;
> > +
> > +if (virJSONValueObjectAdd(,
> > + "s:driver", "pvpanic-pci",
> > + NULL) < 0)
> > +return -1;
> > +
> > +if (def->panics[i]->info.type ==
> > VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
>
> This check doesn't make much sense ...
I imagine it was lifted from the ISA variant, handled just above,
where it's necessary because not specifying an address is somehow
considered a valid configuration.
I agree with you that it's not needed for pvpanic-pci.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 1/1] apparmor: Allow umount(/dev)
On Wed, Jan 18, 2023 at 08:59:23AM -0700, Jim Fehlig wrote: > On 1/18/23 03:45, Andrea Bolognani wrote: > > Jim, it looks like you came up with exactly the same solution as > > me, despite concerns about the size of the resulting hammer. Any > > other ideas, or should we just go ahead and merge this as-is? > > My apparmor skills are too weak to select a smaller tool, so I'd say merge > as-is. It wasn't clear to me if/why the umount of /dev was actually needed, > but Michal did an excellent job of describing why it is. Okay, pushed now. Does this warrant creating a maintenance branch / release? 9.0.0 is basically unusable out of the box on AppArmor hosts... On the other hand, package maintainers for Debian/Ubuntu and openSUSE are aware of the issue and know exactly which commit they need to backport. Are there other distros out there using AppArmor? -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH 1/1] apparmor: Allow umount(/dev)
On Wed, Jan 18, 2023 at 11:00:33AM +0100, Michal Prívozník wrote: > On 1/18/23 10:43, Andrea Bolognani wrote: > > Commit 379c0ce4bfed introduced a call to umount(/dev) performed > > inside the namespace that we run QEMU in. > > > > As a result of this, on machines using AppArmor, VM startup now > > fails with > > > > internal error: Process exited prior to exec: libvirt: > > QEMU Driver error: failed to umount devfs on /dev: Permission denied > > > > The corresponding denial is > > > > AVC apparmor="DENIED" operation="umount" profile="libvirtd" > > name="/dev/" pid=70036 comm="rpc-libvirtd" > > > > Extend the AppArmor configuration for virtqemud and libvirtd so > > that this operation is allowed. > > > > Signed-off-by: Andrea Bolognani > > --- > > src/security/apparmor/usr.sbin.libvirtd.in | 1 + > > src/security/apparmor/usr.sbin.virtqemud.in | 1 + > > 2 files changed, 2 insertions(+) > > Reviewed-by: Michal Privoznik > > For more background on why umount is needed see my reply to Jim's > question from earlier: > > https://listman.redhat.com/archives/libvir-list/2023-January/237149.html Welp, missed that one O:-) Jim, it looks like you came up with exactly the same solution as me, despite concerns about the size of the resulting hammer. Any other ideas, or should we just go ahead and merge this as-is? -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH V9 00/14] spec: Decompose the daemon subpackage
On Tue, Jan 17, 2023 at 06:58:39PM +, Daniel P. Berrangé wrote: > On Tue, Jan 17, 2023 at 11:50:10AM -0700, Jim Fehlig wrote: > > All patches in this series contain a R-B from Andrea or Daniel. Andrea, can > > this be pushed now or did you want a final blessing from Daniel? > > I have no further comments. So can you please give your R-b to the few patches that are missing it? -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 0/1] apparmor: Allow umount(/dev)
CC'ing AppArmor experts to get their input :) This is a farily big hammer, but unfortunately I don't think it's possible to tell AppArmor "let the driver use umount, but only if it's running inside a namespace". Andrea Bolognani (1): apparmor: Allow umount(/dev) src/security/apparmor/usr.sbin.libvirtd.in | 1 + src/security/apparmor/usr.sbin.virtqemud.in | 1 + 2 files changed, 2 insertions(+) -- 2.39.0
