Generally speaking, when firmware autoselection is in use we
don't want any information to be provided manually. There are
two exceptions:
* we still want the path to the NVRAM file to be customizable;
* using <loader secure='yes'/> was how you would ask for a
firmware that implements the Secure Boot feature in the
original approach to firmware autoselection, so we want to
keep that working.
Anything else should result in a descriptive error.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/327
Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
---
src/conf/domain_validate.c | 48 +++++++++++++++++++
...firmware-auto-bios-nvram.x86_64-latest.err | 1 +
.../firmware-auto-bios-nvram.xml | 18 +++++++
...auto-efi-loader-insecure.x86_64-latest.err | 1 +
.../firmware-auto-efi-loader-insecure.xml | 18 +++++++
...are-auto-efi-loader-path.x86_64-latest.err | 1 +
.../firmware-auto-efi-loader-path.xml | 18 +++++++
tests/qemuxml2argvtest.c | 3 ++
8 files changed, 108 insertions(+)
create mode 100644 tests/qemuxml2argvdata/firmware-auto-bios-nvram.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-auto-bios-nvram.xml
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.xml
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err
create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-path.xml
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 1f6c32a816..87fdb677d1 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1606,6 +1606,54 @@ virDomainDefOSValidate(const virDomainDef *def,
_("firmware auto selection not implemented for this driver"));
return -1;
}
+
+ if (!loader)
+ return 0;
+
+ if (loader->readonly) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("loader attribute 'readonly' cannot be specified "
+ "when firmware autoselection is enabled"));
+ return -1;
+ }
+ if (loader->type) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("loader attribute 'type' cannot be specified "
+ "when firmware autoselection is enabled"));
+ return -1;
+ }
+ if (loader->path) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("loader path cannot be specified "
+ "when firmware autoselection is enabled"));
+ return -1;
+ }
+ if (loader->nvramTemplate) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("nvram attribute 'template' cannot be specified "
+ "when firmware autoselection is enabled"));
+ return -1;
+ }
+
+ /* We need to accept 'yes' here because the initial implementation
+ * of firmware autoselection used it as a way to request a firmware
+ * with Secure Boot support, so the error message is technically
+ * incorrect; however, we want to discourage people from using this
+ * attribute at all, so it's fine to be a bit more aggressive than
+ * it would be strictly required :) */
+ if (loader->secure == VIR_TRISTATE_BOOL_NO) {
+ virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("loader attribute 'secure' cannot be specified "
+ "when firmware autoselection is enabled"));
+ return -1;
+ }
+
+ if (loader->nvram && def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) {
+ virReportError(VIR_ERR_XML_DETAIL,
+ _("firmware type '%s' does not support nvram"),
+ virDomainOsDefFirmwareTypeToString(def->os.firmware));
+ return -1;
+ }
} else {
if (!loader)
return 0;
diff --git a/tests/qemuxml2argvdata/firmware-auto-bios-nvram.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-bios-nvram.x86_64-latest.err
new file mode 100644
index 0000000000..772beb49e2
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-bios-nvram.x86_64-latest.err
@@ -0,0 +1 @@
+firmware type 'bios' does not support nvram
diff --git a/tests/qemuxml2argvdata/firmware-auto-bios-nvram.xml b/tests/qemuxml2argvdata/firmware-auto-bios-nvram.xml
new file mode 100644
index 0000000000..6dad1e1f7f
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-bios-nvram.xml
@@ -0,0 +1,18 @@
+<domain type='kvm'>
+ <name>fedora</name>
+ <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+ <memory unit='KiB'>8192</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os firmware='bios'>
+ <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+ <nvram>/path/to/fedora_VARS.fd</nvram>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err
new file mode 100644
index 0000000000..564f0e6918
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-latest.err
@@ -0,0 +1 @@
+loader attribute 'secure' cannot be specified when firmware autoselection is enabled
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.xml b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.xml
new file mode 100644
index 0000000000..33bd7b0ac1
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.xml
@@ -0,0 +1,18 @@
+<domain type='kvm'>
+ <name>fedora</name>
+ <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+ <memory unit='KiB'>8192</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os firmware='efi'>
+ <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+ <loader secure='no'/>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err
new file mode 100644
index 0000000000..e551fafd03
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err
@@ -0,0 +1 @@
+loader attribute 'type' cannot be specified when firmware autoselection is enabled
diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.xml b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.xml
new file mode 100644
index 0000000000..a40f5e730c
--- /dev/null
+++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.xml
@@ -0,0 +1,18 @@
+<domain type='kvm'>
+ <name>fedora</name>
+ <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+ <memory unit='KiB'>8192</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os firmware='efi'>
+ <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
+ <loader>/path/to/OVMF_CODE.fd</loader>
+ </os>
+ <features>
+ <acpi/>
+ </features>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='usb' model='none'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b01ad8d4e9..473e00ffa7 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1217,9 +1217,12 @@ mymain(void)
DO_TEST_NOCAPS("firmware-manual-noefi-noacpi-q35");
DO_TEST_CAPS_LATEST("firmware-auto-bios");
+ DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-bios-nvram");
DO_TEST_CAPS_LATEST("firmware-auto-efi");
DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram");
DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-secure");
+ DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-loader-insecure");
+ DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-loader-path");
DO_TEST_CAPS_LATEST("firmware-auto-efi-secboot");
DO_TEST_CAPS_LATEST("firmware-auto-efi-no-secboot");
DO_TEST_CAPS_LATEST("firmware-auto-efi-enrolled-keys");
--
2.35.3
