[libvirt PATCH 0/3] Misc fixes / improvements
Just a few things I dealt with as side effect of other work I'm doing wrt AMD SEV. Daniel P. Berrangé (3): docs: split example for <os> schema docs: use virYesNo definition in more schemas qemu: split handling of distinct firmware enum conversions docs/formatdomain.rst | 32 +++++++++++++++++++++++++++++--- docs/schemas/capability.rng | 5 +---- docs/schemas/domainbackup.rng | 5 +---- docs/schemas/domaincaps.rng | 5 +---- docs/schemas/domaincommon.rng | 17 ++++------------- src/conf/domain_conf.h | 4 ++-- src/qemu/qemu_firmware.c | 21 +++++++++++++++++++-- 7 files changed, 57 insertions(+), 32 deletions(-) -- 2.33.1
The docs illustration for the <os> schema contains a mixture of incompatible configuration options. This is rather confusing and misleading to users. Splitting the illustration into four separate examples clarifies the situation. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/formatdomain.rst | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index d4f30bb8af..3dee28d52a 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -110,12 +110,19 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. :: + <!-- Xen with fullvirt loader --> ... - <os firmware='efi'> + <os> <type>hvm</type> - <loader readonly='yes' secure='no' type='rom'>/usr/lib/xen/boot/hvmloader</loader> - <nvram template='/usr/share/OVMF/OVMF_VARS.fd'>/var/lib/libvirt/nvram/guest_VARS.fd</nvram> + <loader>/usr/lib/xen/boot/hvmloader</loader> <boot dev='hd'/> + </os> + ... + + <!-- QEMU with default firmware, serial console and SMBIOS --> + ... + <os> + <type>hvm</type> <boot dev='cdrom'/> <bootmenu enable='yes' timeout='3000'/> <smbios mode='sysinfo'/> @@ -123,6 +130,25 @@ harddisk, cdrom, network) determining where to obtain/find the boot image. </os> ... + <!-- QEMU with UEFI manual firmware and secure boot --> + ... + <os> + <type>hvm</type> + <loader readonly='yes' secure='yes' type='pflash'>/usr/share/OVMF/OVMF_CODE.fd</loader> + <nvram template='/usr/share/OVMF/OVMF_VARS.fd'>/var/lib/libvirt/nvram/guest_VARS.fd</nvram> + <boot dev='hd'/> + </os> + ... + + <!-- QEMU with automatic UEFI firmware and secure boot --> + ... + <os firmware='efi'> + <type>hvm</type> + <loader secure='yes'/> + <boot dev='hd'/> + </os> + ... + ``firmware`` The ``firmware`` attribute allows management applications to automatically fill ``<loader/>`` and ``<nvram/>`` elements and possibly enable some -- 2.33.1
A few places are still using an expend yes/no choice instead of the common virYesNo definition. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/schemas/capability.rng | 5 +---- docs/schemas/domainbackup.rng | 5 +---- docs/schemas/domaincaps.rng | 5 +---- docs/schemas/domaincommon.rng | 17 ++++------------- 4 files changed, 7 insertions(+), 25 deletions(-) diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index 7747a01558..c7e72c6f39 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -409,10 +409,7 @@ </optional> <optional> <attribute name="deprecated"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virYesNo"/> </attribute> </optional> <text/> diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng index 05cc28ab00..1ac9da62c1 100644 --- a/docs/schemas/domainbackup.rng +++ b/docs/schemas/domainbackup.rng @@ -60,10 +60,7 @@ <element name="server"> <optional> <attribute name="tls"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virYesNo"/> </attribute> </optional> <choice> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index b40ee0f35a..9cbc2467ab 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -156,10 +156,7 @@ </attribute> <optional> <attribute name="deprecated"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virYesNo"/> </attribute> </optional> <text/> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7fa5c2b8b5..169b8d8dee 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -300,18 +300,12 @@ <element name="loader"> <optional> <attribute name="readonly"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virYesNo"/> </attribute> </optional> <optional> <attribute name="secure"> - <choice> - <value>yes</value> - <value>no</value> - </choice> + <ref name="virYesNo"/> </attribute> </optional> <optional> @@ -5331,11 +5325,8 @@ <ref name="tpm-backend-emulator-active-pcr-banks"/> <optional> <attribute name="persistent_state"> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> + <ref name="virYesNo"/> + </attribute> </optional> </group> </choice> -- 2.33.1
The qemuFirmwareOSInterfaceTypeFromOsDefFirmware method was added to convert from virDomainOsDefFirmware to the qemuFirmwareOSInterface enum. It was later also used to convert from virDomainLoader to qemuFirmwareOSInterface in: commit 8e1804f9f66f13ca1412d22bf1a957b6d55a2365 Author: Michal Prívozník <mprivozn@redhat.com> Date: Tue Dec 17 17:45:50 2019 +0100 qemu_firmware: Try to autofill for old style UEFI specification This caused compile errors with clang due to passing a mis-matched enum type. These were later silenced by stripping the enum types: commit 8fcee47807d29008632a7ad918cbe93ac0a20597 Author: Michal Prívozník <mprivozn@redhat.com> Date: Wed Jan 8 09:42:47 2020 +0100 qemu_firmware: Accept int in qemuFirmwareOSInterfaceTypeFromOsDefFirmware() This is still rather confusing to humans reading the code. It is clearer to just define a separate helper method for the virDomainLoader type conversion. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.h | 4 ++-- src/qemu/qemu_firmware.c | 21 +++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 144ba4dd12..8a7e9a1668 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2292,8 +2292,8 @@ struct _virDomainOSEnv { typedef enum { VIR_DOMAIN_OS_DEF_FIRMWARE_NONE = 0, - VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS = VIR_DOMAIN_LOADER_TYPE_ROM, - VIR_DOMAIN_OS_DEF_FIRMWARE_EFI = VIR_DOMAIN_LOADER_TYPE_PFLASH, + VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS, + VIR_DOMAIN_OS_DEF_FIRMWARE_EFI, VIR_DOMAIN_OS_DEF_FIRMWARE_LAST } virDomainOsDefFirmware; diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 529ab8d68e..84c80eaacb 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -899,7 +899,7 @@ qemuFirmwareMatchesMachineArch(const qemuFirmware *fw, static qemuFirmwareOSInterface -qemuFirmwareOSInterfaceTypeFromOsDefFirmware(int fw) +qemuFirmwareOSInterfaceTypeFromOsDefFirmware(virDomainOsDefFirmware fw) { switch (fw) { case VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS: @@ -915,6 +915,23 @@ qemuFirmwareOSInterfaceTypeFromOsDefFirmware(int fw) } +static qemuFirmwareOSInterface +qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(virDomainLoader type) +{ + switch (type) { + case VIR_DOMAIN_LOADER_TYPE_ROM: + return QEMU_FIRMWARE_OS_INTERFACE_BIOS; + case VIR_DOMAIN_LOADER_TYPE_PFLASH: + return QEMU_FIRMWARE_OS_INTERFACE_UEFI; + case VIR_DOMAIN_LOADER_TYPE_NONE: + case VIR_DOMAIN_LOADER_TYPE_LAST: + break; + } + + return QEMU_FIRMWARE_OS_INTERFACE_NONE; +} + + #define VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY (1 << 2) @@ -939,7 +956,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, if (want == QEMU_FIRMWARE_OS_INTERFACE_NONE && def->os.loader) { - want = qemuFirmwareOSInterfaceTypeFromOsDefFirmware(def->os.loader->type); + want = qemuFirmwareOSInterfaceTypeFromOsDefLoaderType(def->os.loader->type); if (fw->mapping.device != QEMU_FIRMWARE_DEVICE_FLASH || STRNEQ(def->os.loader->path, fw->mapping.data.flash.executable.filename)) { -- 2.33.1
On 1/11/22 18:00, Daniel P. Berrangé wrote:
Just a few things I dealt with as side effect of other work I'm doing wrt AMD SEV.
Daniel P. Berrangé (3): docs: split example for <os> schema docs: use virYesNo definition in more schemas qemu: split handling of distinct firmware enum conversions
docs/formatdomain.rst | 32 +++++++++++++++++++++++++++++--- docs/schemas/capability.rng | 5 +---- docs/schemas/domainbackup.rng | 5 +---- docs/schemas/domaincaps.rng | 5 +---- docs/schemas/domaincommon.rng | 17 ++++------------- src/conf/domain_conf.h | 4 ++-- src/qemu/qemu_firmware.c | 21 +++++++++++++++++++-- 7 files changed, 57 insertions(+), 32 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Daniel P. Berrangé -
Michal Prívozník