[PATCH 0/2] libxl: Support custom EFI firmware path

libxl added support for specifying custom firmware paths long ago. This series adds support to the libxl driver and the XML to xl.cfg config converter. Jim Fehlig (2): libxl: Support specifying a custom firmware path libxl: Add support for custom firmware path in config converter src/libxl/libxl_conf.c | 14 ++-- src/libxl/xen_xl.c | 19 +++-- tests/libxlxml2domconfigdata/efi-hvm.json | 91 +++++++++++++++++++++++ tests/libxlxml2domconfigdata/efi-hvm.xml | 36 +++++++++ tests/libxlxml2domconfigtest.c | 1 + tests/xlconfigdata/test-fullvirt-ovmf.cfg | 1 + tests/xlconfigdata/test-fullvirt-ovmf.xml | 2 +- 7 files changed, 150 insertions(+), 14 deletions(-) create mode 100644 tests/libxlxml2domconfigdata/efi-hvm.json create mode 100644 tests/libxlxml2domconfigdata/efi-hvm.xml -- 2.39.1

libxl added support for specifying custom firmware paths long ago. The functionality exists in all Xen version supported by libvirt. This patch adds support for user-specified efi firmware paths in the libxl driver. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 14 ++-- tests/libxlxml2domconfigdata/efi-hvm.json | 91 +++++++++++++++++++++++ tests/libxlxml2domconfigdata/efi-hvm.xml | 36 +++++++++ tests/libxlxml2domconfigtest.c | 1 + 4 files changed, 134 insertions(+), 8 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index cee9f827f7..f856f12075 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -632,14 +632,10 @@ libxlMakeDomBuildInfo(virDomainDef *def, b_info->ramdisk = g_strdup(def->os.initrd); /* - * Currently libxl only allows specifying the type of BIOS. - * If automatic firmware selection is enabled or the loader - * type is PFLASH, we assume OVMF and set libxl_bios_type - * to LIBXL_BIOS_TYPE_OVMF. The path to the OVMF firmware is - * configured when building Xen using '--with-system-ovmf='. If - * not specified, LIBXL_FIRMWARE_DIR/ovmf.bin is used. In the - * future, Xen will support a user-specified firmware path. See - * https://lists.xenproject.org/archives/html/xen-devel/2016-03/msg01628.html + * libxl allows specifying the type of firmware and an optional path. + * If the path is not explicitly specified, a default path for the given + * firmware type is used. For EFI, it's LIBXL_FIRMWARE_DIR/ovmf.bin. + * Currently libxl does not support specifying nvram for EFI firmwares. */ if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) { if (def->os.loader == NULL) @@ -651,9 +647,11 @@ libxlMakeDomBuildInfo(virDomainDef *def, if (def->os.loader->readonly == VIR_TRISTATE_BOOL_ABSENT) def->os.loader->readonly = VIR_TRISTATE_BOOL_YES; b_info->u.hvm.bios = LIBXL_BIOS_TYPE_OVMF; + b_info->u.hvm.system_firmware = g_strdup(def->os.loader->path); def->os.firmware = VIR_DOMAIN_OS_DEF_FIRMWARE_NONE; } else if (virDomainDefHasOldStyleUEFI(def)) { b_info->u.hvm.bios = LIBXL_BIOS_TYPE_OVMF; + b_info->u.hvm.system_firmware = g_strdup(def->os.loader->path); } if (def->emulator) { diff --git a/tests/libxlxml2domconfigdata/efi-hvm.json b/tests/libxlxml2domconfigdata/efi-hvm.json new file mode 100644 index 0000000000..32b5a49c3f --- /dev/null +++ b/tests/libxlxml2domconfigdata/efi-hvm.json @@ -0,0 +1,91 @@ +{ + "c_info": { + "type": "hvm", + "name": "test-hvm", + "uuid": "2147d599-9cc6-c0dc-92ab-4064b5446e9b" + }, + "b_info": { + "max_vcpus": 4, + "avail_vcpus": [ + 0, + 1, + 2, + 3 + ], + "max_memkb": 1048576, + "target_memkb": 1048576, + "video_memkb": 8192, + "shadow_memkb": 1234, + "device_model_version": "qemu_xen", + "device_model": "/bin/true", + "sched_params": { + + }, + "apic": "True", + "acpi": "True", + "type.hvm": { + "bios": "ovmf", + "pae": "True", + "system_firmware": "/usr/share/qemu/ovmf-x86_64-xen.bin", + "vga": { + "kind": "cirrus" + }, + "vnc": { + "enable": "True", + "listen": "0.0.0.0", + "findunused": "False" + }, + "sdl": { + "enable": "False" + }, + "spice": { + + }, + "boot": "c", + "rdm": { + + } + }, + "arch_arm": { + + } + }, + "disks": [ + { + "pdev_path": "/var/lib/xen/images/test-hvm.img", + "vdev": "hda", + "backend": "qdisk", + "format": "raw", + "removable": 1, + "readwrite": 1 + } + ], + "nics": [ + { + "devid": 0, + "mac": "00:16:3e:66:12:b4", + "bridge": "br0", + "script": "/etc/xen/scripts/vif-bridge", + "nictype": "vif_ioemu" + } + ], + "vfbs": [ + { + "devid": -1, + "vnc": { + "enable": "True", + "listen": "0.0.0.0", + "findunused": "False" + }, + "sdl": { + "enable": "False" + } + } + ], + "vkbs": [ + { + "devid": -1 + } + ], + "on_reboot": "restart" +} diff --git a/tests/libxlxml2domconfigdata/efi-hvm.xml b/tests/libxlxml2domconfigdata/efi-hvm.xml new file mode 100644 index 0000000000..4c94476b59 --- /dev/null +++ b/tests/libxlxml2domconfigdata/efi-hvm.xml @@ -0,0 +1,36 @@ +<domain type='xen'> + <name>test-hvm</name> + <description>None</description> + <uuid>2147d599-9cc6-c0dc-92ab-4064b5446e9b</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu>4</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <clock offset='utc'/> + <os> + <type>hvm</type> + <loader readonly='yes' type='pflash'>/usr/share/qemu/ovmf-x86_64-xen.bin</loader> + <boot dev='hd'/> + </os> + <features> + <apic/> + <acpi/> + <pae/> + </features> + <devices> + <emulator>/bin/true</emulator> + <disk type='file' device='disk'> + <driver name='qemu'/> + <source file='/var/lib/xen/images/test-hvm.img'/> + <target dev='hda'/> + </disk> + <interface type='bridge'> + <source bridge='br0'/> + <mac address='00:16:3e:66:12:b4'/> + <script path='/etc/xen/scripts/vif-bridge'/> + </interface> + <graphics type='vnc' port='-1' autoport='yes' listen='0.0.0.0'/> + </devices> +</domain> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index a7080e0fa2..6418341564 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -183,6 +183,7 @@ mymain(void) DO_TEST("basic-pv"); DO_TEST("basic-hvm"); + DO_TEST("efi-hvm"); # ifdef WITH_XEN_PVH DO_TEST("basic-pvh"); # endif -- 2.39.1

Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/xen_xl.c | 19 ++++++++++++++----- tests/xlconfigdata/test-fullvirt-ovmf.cfg | 1 + tests/xlconfigdata/test-fullvirt-ovmf.xml | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 6919325623..a7aa0f90d2 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -104,18 +104,23 @@ xenParseXLOS(virConf *conf, virDomainDef *def, virCaps *caps) if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { g_autofree char *bios = NULL; + g_autofree char *bios_path = NULL; g_autofree char *boot = NULL; int val = 0; if (xenConfigGetString(conf, "bios", &bios, NULL) < 0) return -1; + if (xenConfigGetString(conf, "bios_path_override", &bios_path, NULL) < 0) + return -1; if (bios && STREQ(bios, "ovmf")) { def->os.loader = g_new0(virDomainLoaderDef, 1); def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; def->os.loader->readonly = VIR_TRISTATE_BOOL_YES; - - def->os.loader->path = g_strdup(LIBXL_FIRMWARE_DIR "/ovmf.bin"); + if (bios_path) + def->os.loader->path = g_strdup(bios_path); + else + def->os.loader->path = g_strdup(LIBXL_FIRMWARE_DIR "/ovmf.bin"); } else { for (i = 0; i < caps->nguests; i++) { if (caps->guests[i]->ostype == VIR_DOMAIN_OSTYPE_HVM && @@ -1119,9 +1124,13 @@ xenFormatXLOS(virConf *conf, virDomainDef *def) if (xenConfigSetString(conf, "builder", "hvm") < 0) return -1; - if (virDomainDefHasOldStyleUEFI(def) && - xenConfigSetString(conf, "bios", "ovmf") < 0) - return -1; + if (virDomainDefHasOldStyleUEFI(def)) { + if (xenConfigSetString(conf, "bios", "ovmf") < 0) + return -1; + if (def->os.loader->path && + (xenConfigSetString(conf, "bios_path_override", def->os.loader->path) < 0)) + return -1; + } if (def->os.slic_table && xenConfigSetString(conf, "acpi_firmware", def->os.slic_table) < 0) diff --git a/tests/xlconfigdata/test-fullvirt-ovmf.cfg b/tests/xlconfigdata/test-fullvirt-ovmf.cfg index 4d31a81108..77b5073005 100644 --- a/tests/xlconfigdata/test-fullvirt-ovmf.cfg +++ b/tests/xlconfigdata/test-fullvirt-ovmf.cfg @@ -22,5 +22,6 @@ parallel = "none" serial = "none" builder = "hvm" bios = "ovmf" +bios_path_override = "/usr/share/qemu/ovmf-x86_64-xen.bin" boot = "d" disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=qcow2,vdev=hdb,access=rw,backendtype=qdisk,target=/var/lib/libvirt/images/XenGuest2-home", "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" ] diff --git a/tests/xlconfigdata/test-fullvirt-ovmf.xml b/tests/xlconfigdata/test-fullvirt-ovmf.xml index 8994b3ea36..da3954dba9 100644 --- a/tests/xlconfigdata/test-fullvirt-ovmf.xml +++ b/tests/xlconfigdata/test-fullvirt-ovmf.xml @@ -6,7 +6,7 @@ <vcpu placement='static'>1</vcpu> <os> <type arch='x86_64' machine='xenfv'>hvm</type> - <loader readonly='yes' type='pflash'>/LIBXL_FIRMWARE_DIR/ovmf.bin</loader> + <loader readonly='yes' type='pflash'>/usr/share/qemu/ovmf-x86_64-xen.bin</loader> <boot dev='cdrom'/> </os> <features> -- 2.39.1

On 2/13/23 22:45, Jim Fehlig wrote:
libxl added support for specifying custom firmware paths long ago. This series adds support to the libxl driver and the XML to xl.cfg config converter.
Jim Fehlig (2): libxl: Support specifying a custom firmware path libxl: Add support for custom firmware path in config converter
src/libxl/libxl_conf.c | 14 ++-- src/libxl/xen_xl.c | 19 +++-- tests/libxlxml2domconfigdata/efi-hvm.json | 91 +++++++++++++++++++++++ tests/libxlxml2domconfigdata/efi-hvm.xml | 36 +++++++++ tests/libxlxml2domconfigtest.c | 1 + tests/xlconfigdata/test-fullvirt-ovmf.cfg | 1 + tests/xlconfigdata/test-fullvirt-ovmf.xml | 2 +- 7 files changed, 150 insertions(+), 14 deletions(-) create mode 100644 tests/libxlxml2domconfigdata/efi-hvm.json create mode 100644 tests/libxlxml2domconfigdata/efi-hvm.xml
Ooops, this has slipped through the cracks even though I had it marked for review. Sorry. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> BTW: what's your view on bumping minimal XEN version from 4.9 to at least 4.10 (or even something newer)? That way we could drop some code (at least). Michal

On 3/10/23 06:58, Michal Prívozník wrote:
On 2/13/23 22:45, Jim Fehlig wrote:
libxl added support for specifying custom firmware paths long ago. This series adds support to the libxl driver and the XML to xl.cfg config converter.
Jim Fehlig (2): libxl: Support specifying a custom firmware path libxl: Add support for custom firmware path in config converter
src/libxl/libxl_conf.c | 14 ++-- src/libxl/xen_xl.c | 19 +++-- tests/libxlxml2domconfigdata/efi-hvm.json | 91 +++++++++++++++++++++++ tests/libxlxml2domconfigdata/efi-hvm.xml | 36 +++++++++ tests/libxlxml2domconfigtest.c | 1 + tests/xlconfigdata/test-fullvirt-ovmf.cfg | 1 + tests/xlconfigdata/test-fullvirt-ovmf.xml | 2 +- 7 files changed, 150 insertions(+), 14 deletions(-) create mode 100644 tests/libxlxml2domconfigdata/efi-hvm.json create mode 100644 tests/libxlxml2domconfigdata/efi-hvm.xml
Ooops, this has slipped through the cracks even though I had it marked for review. Sorry.
No worries. Thanks a lot for taking the time to review!
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
BTW: what's your view on bumping minimal XEN version from 4.9 to at least 4.10 (or even something newer)? That way we could drop some code (at least).
We could drop a fair bit of code by bumping the minimum version to 4.13. That would allow us to unconditionally set LIBXL_API_VERSION to 0x041300 and drop support for the old 0x040500. We got stuck there since Xen didn't advertise support for 0x040700 and 0x040800 until commit c3999835df2, which was included in 4.13 - opps! According to repology Debian 11: 4.14.x Fedora 37: 4.16.x openSUSE Leap15.4/SLE15 SP4: 4.16.x Ubuntu 22.10: 4.16.x Maybe bumping the minimum version to 4.13 is a viable proposition? Regards, Jim

On Fri, Mar 10, 2023 at 10:33:05AM -0700, Jim Fehlig wrote:
On 3/10/23 06:58, Michal Prívozník wrote:
BTW: what's your view on bumping minimal XEN version from 4.9 to at least 4.10 (or even something newer)? That way we could drop some code (at least).
We could drop a fair bit of code by bumping the minimum version to 4.13. That would allow us to unconditionally set LIBXL_API_VERSION to 0x041300 and drop support for the old 0x040500. We got stuck there since Xen didn't advertise support for 0x040700 and 0x040800 until commit c3999835df2, which was included in 4.13 - opps! According to repology
Debian 11: 4.14.x Fedora 37: 4.16.x openSUSE Leap15.4/SLE15 SP4: 4.16.x Ubuntu 22.10: 4.16.x
Maybe bumping the minimum version to 4.13 is a viable proposition?
Ubuntu 20.04 comes with Xen 4.11, so unfortunately we can't do that for another year :( -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
Jim Fehlig
-
Michal Prívozník