po/POTFILES.in | 1 + src/bhyve/bhyve_domain.c | 5 + src/bhyve/bhyve_firmware.c | 91 +++++++++++++++++++ src/bhyve/bhyve_firmware.h | 30 ++++++ src/bhyve/bhyve_process.c | 15 +++ src/bhyve/bhyve_process.h | 5 + src/bhyve/meson.build | 1 + .../bhyvexml2argv-firmware-efi.args | 11 +++ .../bhyvexml2argv-firmware-efi.ldargs | 1 + .../bhyvexml2argv-firmware-efi.xml | 22 +++++ tests/bhyvexml2argvtest.c | 83 ++++++++++++++--- 11 files changed, 254 insertions(+), 11 deletions(-) create mode 100644 src/bhyve/bhyve_firmware.c create mode 100644 src/bhyve/bhyve_firmware.h create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml
Implement "<os firmware='efi'>" support for bhyve driver.
As there are not really lot of options, try to find
"BHYVE_UEFI.fd" firmware which is installed by the
sysutils/uefi-edk2-bhyve FreeBSD port.
If not found, just use the first found firmware
in the firmwares directory (which is configurable via
config file).
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
Not extremely happy about the LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE knob,
but not sure how to test this otherwise.
po/POTFILES.in | 1 +
src/bhyve/bhyve_domain.c | 5 +
src/bhyve/bhyve_firmware.c | 91 +++++++++++++++++++
src/bhyve/bhyve_firmware.h | 30 ++++++
src/bhyve/bhyve_process.c | 15 +++
src/bhyve/bhyve_process.h | 5 +
src/bhyve/meson.build | 1 +
.../bhyvexml2argv-firmware-efi.args | 11 +++
.../bhyvexml2argv-firmware-efi.ldargs | 1 +
.../bhyvexml2argv-firmware-efi.xml | 22 +++++
tests/bhyvexml2argvtest.c | 83 ++++++++++++++---
11 files changed, 254 insertions(+), 11 deletions(-)
create mode 100644 src/bhyve/bhyve_firmware.c
create mode 100644 src/bhyve/bhyve_firmware.h
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs
create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 80c5f145be..413783ee35 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -14,6 +14,7 @@
@SRCDIR@src/bhyve/bhyve_command.c
@SRCDIR@src/bhyve/bhyve_domain.c
@SRCDIR@src/bhyve/bhyve_driver.c
+@SRCDIR@src/bhyve/bhyve_firmware.c
@SRCDIR@src/bhyve/bhyve_monitor.c
@SRCDIR@src/bhyve/bhyve_parse_command.c
@SRCDIR@src/bhyve/bhyve_process.c
diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
index 8fbc554a0a..209e4d3905 100644
--- a/src/bhyve/bhyve_domain.c
+++ b/src/bhyve/bhyve_domain.c
@@ -64,6 +64,9 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def)
if (def->os.bootloader == NULL && def->os.loader)
return true;
+ if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI)
+ return true;
+
if (def->nserials || def->nconsoles)
return true;
@@ -230,6 +233,8 @@ virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = {
.domainPostParseCallback = bhyveDomainDefPostParse,
.assignAddressesCallback = bhyveDomainDefAssignAddresses,
.deviceValidateCallback = bhyveDomainDeviceDefValidate,
+
+ .features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT,
};
static void
diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c
new file mode 100644
index 0000000000..ccc3a5ffc8
--- /dev/null
+++ b/src/bhyve/bhyve_firmware.c
@@ -0,0 +1,91 @@
+/*
+ * bhyve_firmware.c: bhyve firmware management
+ *
+ * Copyright (C) 2021 Roman Bogorodskiy
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <config.h>
+#include <dirent.h>
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virfile.h"
+#include "bhyve_conf.h"
+#include "bhyve_firmware.h"
+
+#define VIR_FROM_THIS VIR_FROM_BHYVE
+
+VIR_LOG_INIT("bhyve.bhyve_firmware");
+
+
+#define BHYVE_DEFAULT_FIRMWARE "BHYVE_UEFI.fd"
+
+int
+bhyveFirmwareFillDomain(bhyveConnPtr driver,
+ virDomainDefPtr def,
+ unsigned int flags)
+{
+ g_autoptr(DIR) dir = NULL;
+ virBhyveDriverConfigPtr cfg = virBhyveDriverGetConfig(driver);
+ const char *firmware_dir_cfg = cfg->firmwareDir;
+ const char *firmware_dir_env = NULL, *firmware_dir = NULL;
+ struct dirent *entry;
+ char *matching_firmware = NULL;
+ char *first_found = NULL;
+
+ virCheckFlags(0, -1);
+
+ if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
+ return 0;
+
+ if (virDirOpenIfExists(&dir, firmware_dir_cfg) > 0) {
+ while ((virDirRead(dir, &entry, firmware_dir)) > 0) {
+ if (STREQ(entry->d_name, BHYVE_DEFAULT_FIRMWARE)) {
+ matching_firmware = g_strdup(entry->d_name);
+ break;
+ }
+ if (!first_found)
+ first_found = g_strdup(entry->d_name);
+ }
+ }
+
+ if (!matching_firmware) {
+ if (!first_found) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("no firmwares found in %s"),
+ firmware_dir_cfg);
+ return -1;
+ } else {
+ matching_firmware = first_found;
+ }
+ }
+
+ if (!def->os.loader)
+ def->os.loader = g_new0(virDomainLoaderDef, 1);
+
+ def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
+ def->os.loader->readonly = VIR_TRISTATE_BOOL_YES;
+
+ VIR_FREE(def->os.loader->path);
+
+ firmware_dir_env = g_getenv("LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE");
+ firmware_dir = firmware_dir_env ? firmware_dir_env : firmware_dir_cfg;
+ def->os.loader->path = g_build_filename(firmware_dir, matching_firmware, NULL);
+
+ return 0;
+}
diff --git a/src/bhyve/bhyve_firmware.h b/src/bhyve/bhyve_firmware.h
new file mode 100644
index 0000000000..ae4bc98676
--- /dev/null
+++ b/src/bhyve/bhyve_firmware.h
@@ -0,0 +1,30 @@
+/*
+ * bhyve_firmware.h: bhyve firmware management
+ *
+ * Copyright (C) 2021 Roman Bogorodskiy
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#pragma once
+
+#include "domain_conf.h"
+#include "bhyve_utils.h"
+
+int
+bhyveFirmwareFillDomain(bhyveConnPtr driver,
+ virDomainDefPtr def,
+ unsigned int flags);
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 060018bc70..0cfe69688c 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -33,6 +33,7 @@
#include "bhyve_device.h"
#include "bhyve_driver.h"
#include "bhyve_command.h"
+#include "bhyve_firmware.h"
#include "bhyve_monitor.h"
#include "bhyve_process.h"
#include "datatypes.h"
@@ -251,6 +252,17 @@ virBhyveProcessStartImpl(bhyveConnPtr driver,
return ret;
}
+int
+bhyveProcessPrepareDomain(bhyveConnPtr driver,
+ virDomainObjPtr vm,
+ unsigned int flags)
+{
+ if (bhyveFirmwareFillDomain(driver, vm->def, flags) < 0)
+ return -1;
+
+ return 0;
+}
+
int
virBhyveProcessStart(virConnectPtr conn,
virDomainObjPtr vm,
@@ -268,6 +280,9 @@ virBhyveProcessStart(virConnectPtr conn,
conn, bhyveProcessAutoDestroy) < 0)
return -1;
+ if (bhyveProcessPrepareDomain(driver, vm, flags) < 0)
+ return -1;
+
return virBhyveProcessStartImpl(driver, vm, reason);
}
diff --git a/src/bhyve/bhyve_process.h b/src/bhyve/bhyve_process.h
index d7b4e0bd4e..133863c1e0 100644
--- a/src/bhyve/bhyve_process.h
+++ b/src/bhyve/bhyve_process.h
@@ -23,6 +23,11 @@
#include "bhyve_utils.h"
+int
+bhyveProcessPrepareDomain(bhyveConnPtr driver,
+ virDomainObjPtr vm,
+ unsigned int flags);
+
int virBhyveProcessStart(virConnectPtr conn,
virDomainObjPtr vm,
virDomainRunningReason reason,
diff --git a/src/bhyve/meson.build b/src/bhyve/meson.build
index 2b65eecf5e..b3551477b7 100644
--- a/src/bhyve/meson.build
+++ b/src/bhyve/meson.build
@@ -2,6 +2,7 @@ bhyve_sources = files(
'bhyve_capabilities.c',
'bhyve_command.c',
'bhyve_conf.c',
+ 'bhyve_firmware.c',
'bhyve_parse_command.c',
'bhyve_device.c',
'bhyve_domain.c',
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args b/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args
new file mode 100644
index 0000000000..40bc84ef27
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args
@@ -0,0 +1,11 @@
+/usr/sbin/bhyve \
+-c 1 \
+-m 214 \
+-u \
+-H \
+-P \
+-s 0:0,hostbridge \
+-l bootrom,test_firmware_dir/BHYVE_UEFI.fd \
+-s 1:0,lpc \
+-s 2:0,ahci,hd:/tmp/freebsd.img \
+-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs
new file mode 100644
index 0000000000..421376db9e
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs
@@ -0,0 +1 @@
+dummy
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml
new file mode 100644
index 0000000000..302326cb26
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml
@@ -0,0 +1,22 @@
+<domain type='bhyve'>
+ <name>bhyve</name>
+ <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
+ <memory>219136</memory>
+ <vcpu>1</vcpu>
+ <os firmware='efi'>
+ <type>hvm</type>
+ </os>
+ <devices>
+ <disk type='file'>
+ <driver name='file' type='raw'/>
+ <source file='/tmp/freebsd.img'/>
+ <target dev='hda' bus='sata'/>
+ <address type='drive' controller='0' bus='0' target='2' unit='0'/>
+ </disk>
+ <interface type='bridge'>
+ <model type='virtio'/>
+ <source bridge="virbr0"/>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+ </interface>
+ </devices>
+</domain>
diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index 197334f9c4..13b8e34c2b 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -7,17 +7,20 @@
# include "datatypes.h"
# include "bhyve/bhyve_capabilities.h"
+# include "bhyve/bhyve_conf.h"
# include "bhyve/bhyve_domain.h"
# include "bhyve/bhyve_utils.h"
# include "bhyve/bhyve_command.h"
+# include "bhyve/bhyve_process.h"
# define VIR_FROM_THIS VIR_FROM_BHYVE
static bhyveConn driver;
typedef enum {
- FLAG_EXPECT_FAILURE = 1 << 0,
- FLAG_EXPECT_PARSE_ERROR = 1 << 1,
+ FLAG_EXPECT_FAILURE = 1 << 0,
+ FLAG_EXPECT_PARSE_ERROR = 1 << 1,
+ FLAG_EXPECT_PREPARE_ERROR = 1 << 2,
} virBhyveXMLToArgvTestFlags;
static int testCompareXMLToArgvFiles(const char *xml,
@@ -29,7 +32,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
g_autofree char *actualargv = NULL;
g_autofree char *actualld = NULL;
g_autofree char *actualdm = NULL;
- g_autoptr(virDomainDef) vmdef = NULL;
+ g_autoptr(virDomainObj) vm = NULL;
g_autoptr(virCommand) cmd = NULL;
g_autoptr(virCommand) ldcmd = NULL;
g_autoptr(virConnect) conn = NULL;
@@ -38,7 +41,10 @@ static int testCompareXMLToArgvFiles(const char *xml,
if (!(conn = virGetConnect()))
goto out;
- if (!(vmdef = virDomainDefParseFile(xml, driver.xmlopt,
+ if (!(vm = virDomainObjNew(driver.xmlopt)))
+ return -1;
+
+ if (!(vm->def = virDomainDefParseFile(xml, driver.xmlopt,
NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE))) {
if (flags & FLAG_EXPECT_PARSE_ERROR) {
ret = 0;
@@ -54,11 +60,20 @@ static int testCompareXMLToArgvFiles(const char *xml,
conn->privateData = &driver;
- cmd = virBhyveProcessBuildBhyveCmd(&driver, vmdef, false);
- if (vmdef->os.loader)
+ if (bhyveProcessPrepareDomain(&driver, vm, 0) < 0) {
+ if (flags & FLAG_EXPECT_PREPARE_ERROR) {
+ ret = 0;
+ VIR_TEST_DEBUG("Got expected error: %s",
+ virGetLastErrorMessage());
+ }
+ goto out;
+ }
+
+ cmd = virBhyveProcessBuildBhyveCmd(&driver, vm->def, false);
+ if (vm->def->os.loader)
ldcmd = virCommandNew("dummy");
else
- ldcmd = virBhyveProcessBuildLoadCmd(&driver, vmdef, "<device.map>",
+ ldcmd = virBhyveProcessBuildLoadCmd(&driver, vm->def, "<device.map>",
&actualdm);
if ((cmd == NULL) || (ldcmd == NULL)) {
@@ -94,10 +109,10 @@ static int testCompareXMLToArgvFiles(const char *xml,
ret = 0;
out:
- if (vmdef &&
- vmdef->ngraphics == 1 &&
- vmdef->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC)
- virPortAllocatorRelease(vmdef->graphics[0]->data.vnc.port);
+ if (vm && vm->def &&
+ vm->def->ngraphics == 1 &&
+ vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC)
+ virPortAllocatorRelease(vm->def->graphics[0]->data.vnc.port);
return ret;
}
@@ -128,10 +143,16 @@ testCompareXMLToArgvHelper(const void *data)
return testCompareXMLToArgvFiles(xml, args, ldargs, dmargs, info->flags);
}
+# define FAKEFIRMWAREDIRTEMPLATE abs_builddir "/bhyvefakefirmwaredir-XXXXXX"
+
static int
mymain(void)
{
int ret = 0;
+ g_autofree char *fakefirmwaredir = NULL;
+ g_autofree char *emptyfirmwaredir = NULL;
+ const char *firmwares[] = {"BHYVE_UEFI.fd", "BHYVE_UEFI_CSM.fd", "refind_x64.efi", NULL};
+ size_t i;
if ((driver.caps = virBhyveCapsBuild()) == NULL)
return EXIT_FAILURE;
@@ -142,6 +163,35 @@ mymain(void)
if (!(driver.remotePorts = virPortAllocatorRangeNew("display", 5900, 65535)))
return EXIT_FAILURE;
+ if (!(driver.config = virBhyveDriverConfigNew()))
+ return EXIT_FAILURE;
+
+ fakefirmwaredir = g_strdup(FAKEFIRMWAREDIRTEMPLATE);
+
+ if (!g_mkdtemp(fakefirmwaredir)) {
+ fprintf(stderr, "Cannot create fakefirmwaredir");
+ abort();
+ }
+ driver.config->firmwareDir = fakefirmwaredir;
+
+ emptyfirmwaredir = g_strdup(FAKEFIRMWAREDIRTEMPLATE);
+
+ if (!g_mkdtemp(emptyfirmwaredir)) {
+ fprintf(stderr, "Cannot create emptyfirmwaredir");
+ abort();
+ }
+
+ i = 0;
+ while (firmwares[i]) {
+ g_autofree char *firmware_path = g_strdup_printf("%s/%s", fakefirmwaredir, firmwares[i++]);
+
+ if (virFileTouch(firmware_path, 0600) < 0) {
+ fprintf(stderr, "Cannot create firmware file");
+ abort();
+ }
+ }
+
+ g_setenv("LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE", "test_firmware_dir", TRUE);
# define DO_TEST_FULL(name, flags) \
do { \
@@ -162,6 +212,9 @@ mymain(void)
# define DO_TEST_PARSE_ERROR(name) \
DO_TEST_FULL(name, FLAG_EXPECT_PARSE_ERROR)
+# define DO_TEST_PREPARE_ERROR(name) \
+ DO_TEST_FULL(name, FLAG_EXPECT_PREPARE_ERROR)
+
driver.grubcaps = BHYVE_GRUB_CAP_CONSDEV;
driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_AHCI32SLOT | \
BHYVE_CAP_NET_E1000 | BHYVE_CAP_LPC_BOOTROM | \
@@ -209,6 +262,10 @@ mymain(void)
DO_TEST("sound");
DO_TEST("isa-controller");
DO_TEST_FAILURE("isa-multiple-controllers");
+ DO_TEST("firmware-efi");
+ g_unsetenv("LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE");
+ driver.config->firmwareDir = emptyfirmwaredir;
+ DO_TEST_PREPARE_ERROR("firmware-efi");
DO_TEST("fs-9p");
DO_TEST("fs-9p-readonly");
DO_TEST_FAILURE("fs-9p-unsupported-type");
@@ -264,9 +321,13 @@ mymain(void)
driver.bhyvecaps &= ~BHYVE_CAP_VNC_PASSWORD;
DO_TEST_FAILURE("vnc-password");
+ virFileDeleteTree(fakefirmwaredir);
+ virFileDeleteTree(emptyfirmwaredir);
+
virObjectUnref(driver.caps);
virObjectUnref(driver.xmlopt);
virPortAllocatorRangeFree(driver.remotePorts);
+ virObjectUnref(driver.config);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.30.0
On 2/27/21 5:34 AM, Roman Bogorodskiy wrote:
> Implement "<os firmware='efi'>" support for bhyve driver.
> As there are not really lot of options, try to find
> "BHYVE_UEFI.fd" firmware which is installed by the
> sysutils/uefi-edk2-bhyve FreeBSD port.
>
> If not found, just use the first found firmware
> in the firmwares directory (which is configurable via
> config file).
>
> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> ---
> Not extremely happy about the LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE knob,
> but not sure how to test this otherwise.
>
> po/POTFILES.in | 1 +
> src/bhyve/bhyve_domain.c | 5 +
> src/bhyve/bhyve_firmware.c | 91 +++++++++++++++++++
> src/bhyve/bhyve_firmware.h | 30 ++++++
> src/bhyve/bhyve_process.c | 15 +++
> src/bhyve/bhyve_process.h | 5 +
> src/bhyve/meson.build | 1 +
> .../bhyvexml2argv-firmware-efi.args | 11 +++
> .../bhyvexml2argv-firmware-efi.ldargs | 1 +
> .../bhyvexml2argv-firmware-efi.xml | 22 +++++
> tests/bhyvexml2argvtest.c | 83 ++++++++++++++---
> 11 files changed, 254 insertions(+), 11 deletions(-)
> create mode 100644 src/bhyve/bhyve_firmware.c
> create mode 100644 src/bhyve/bhyve_firmware.h
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs
> create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 80c5f145be..413783ee35 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -14,6 +14,7 @@
> @SRCDIR@src/bhyve/bhyve_command.c
> @SRCDIR@src/bhyve/bhyve_domain.c
> @SRCDIR@src/bhyve/bhyve_driver.c
> +@SRCDIR@src/bhyve/bhyve_firmware.c
> @SRCDIR@src/bhyve/bhyve_monitor.c
> @SRCDIR@src/bhyve/bhyve_parse_command.c
> @SRCDIR@src/bhyve/bhyve_process.c
> diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
> index 8fbc554a0a..209e4d3905 100644
> --- a/src/bhyve/bhyve_domain.c
> +++ b/src/bhyve/bhyve_domain.c
> @@ -64,6 +64,9 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def)
> if (def->os.bootloader == NULL && def->os.loader)
> return true;
>
> + if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI)
> + return true;
> +
> if (def->nserials || def->nconsoles)
> return true;
>
> @@ -230,6 +233,8 @@ virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = {
> .domainPostParseCallback = bhyveDomainDefPostParse,
> .assignAddressesCallback = bhyveDomainDefAssignAddresses,
> .deviceValidateCallback = bhyveDomainDeviceDefValidate,
> +
> + .features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT,
> };
>
> static void
> diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c
> new file mode 100644
> index 0000000000..ccc3a5ffc8
> --- /dev/null
> +++ b/src/bhyve/bhyve_firmware.c
> @@ -0,0 +1,91 @@
> +/*
> + * bhyve_firmware.c: bhyve firmware management
> + *
> + * Copyright (C) 2021 Roman Bogorodskiy
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include <config.h>
> +#include <dirent.h>
> +
> +#include "viralloc.h"
> +#include "virlog.h"
> +#include "virfile.h"
> +#include "bhyve_conf.h"
> +#include "bhyve_firmware.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_BHYVE
> +
> +VIR_LOG_INIT("bhyve.bhyve_firmware");
> +
> +
> +#define BHYVE_DEFAULT_FIRMWARE "BHYVE_UEFI.fd"
> +
> +int
> +bhyveFirmwareFillDomain(bhyveConnPtr driver,
> + virDomainDefPtr def,
> + unsigned int flags)
> +{
> + g_autoptr(DIR) dir = NULL;
> + virBhyveDriverConfigPtr cfg = virBhyveDriverGetConfig(driver);
virBhyveDriverGetConfig() returns a reference, thus needs to be coupled
with virObjectUnref(cfg); otherwise .. [1]
> + const char *firmware_dir_cfg = cfg->firmwareDir;
> + const char *firmware_dir_env = NULL, *firmware_dir = NULL;
One variable per line, please. It turned out to be useful (although in
this specific case it's not using virXXXPtr type so doesn't matter):
https://listman.redhat.com/archives/libvir-list/2021-March/msg00542.html
> + struct dirent *entry;
> + char *matching_firmware = NULL;
> + char *first_found = NULL;
> +
> + virCheckFlags(0, -1);
1: .. @cfg is leaked here .. [2]
> +
> + if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
> + return 0;
2: .. or here. You get the idea. Alternatively, you can take inspiration
from the QEMU driver, where we defined autoptr cleanup func and then use:
g_autoptr(virQEMUDriverConfig) cfg = NULL;
> +
> + if (virDirOpenIfExists(&dir, firmware_dir_cfg) > 0) {
> + while ((virDirRead(dir, &entry, firmware_dir)) > 0) {
> + if (STREQ(entry->d_name, BHYVE_DEFAULT_FIRMWARE)) {
> + matching_firmware = g_strdup(entry->d_name);
> + break;
> + }
> + if (!first_found)
> + first_found = g_strdup(entry->d_name);
> + }
> + }
> +
> + if (!matching_firmware) {
> + if (!first_found) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("no firmwares found in %s"),
> + firmware_dir_cfg);
> + return -1;
> + } else {
> + matching_firmware = first_found;
3: ^^^
> + }
> + }
> +
> + if (!def->os.loader)
> + def->os.loader = g_new0(virDomainLoaderDef, 1);
> +
> + def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH;
> + def->os.loader->readonly = VIR_TRISTATE_BOOL_YES;
> +
> + VIR_FREE(def->os.loader->path);
> +
> + firmware_dir_env = g_getenv("LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE");
> + firmware_dir = firmware_dir_env ? firmware_dir_env : firmware_dir_cfg;
> + def->os.loader->path = g_build_filename(firmware_dir, matching_firmware, NULL);
I don't think that g_build_filename() consumes @matching_firmware (even
though GLib doesn't document this, looking at its source code - it sure
doesn't). Therefore, @matching_firmware should be freed in the end. The
same goes for @first_round - I can too find a path through this function
which leaks it. For [3] I think you can use g_steal_pointer() to
transfer the ownership.
> +
> + return 0;
> +}
> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
> index 197334f9c4..13b8e34c2b 100644
> --- a/tests/bhyvexml2argvtest.c
> +++ b/tests/bhyvexml2argvtest.c
> @@ -142,6 +163,35 @@ mymain(void)
> if (!(driver.remotePorts = virPortAllocatorRangeNew("display", 5900, 65535)))
> return EXIT_FAILURE;
>
> + if (!(driver.config = virBhyveDriverConfigNew()))
> + return EXIT_FAILURE;
> +
> + fakefirmwaredir = g_strdup(FAKEFIRMWAREDIRTEMPLATE);
> +
> + if (!g_mkdtemp(fakefirmwaredir)) {
> + fprintf(stderr, "Cannot create fakefirmwaredir");
> + abort();
> + }
> + driver.config->firmwareDir = fakefirmwaredir;
> +
> + emptyfirmwaredir = g_strdup(FAKEFIRMWAREDIRTEMPLATE);
> +
> + if (!g_mkdtemp(emptyfirmwaredir)) {
> + fprintf(stderr, "Cannot create emptyfirmwaredir");
> + abort();
> + }
> +
> + i = 0;
> + while (firmwares[i]) {
> + g_autofree char *firmware_path = g_strdup_printf("%s/%s", fakefirmwaredir, firmwares[i++]);
for() loop would be much more readable IMO.
> +
> + if (virFileTouch(firmware_path, 0600) < 0) {
> + fprintf(stderr, "Cannot create firmware file");
> + abort();
> + }
> + }
> +
> + g_setenv("LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE", "test_firmware_dir", TRUE);
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal
© 2016 - 2024 Red Hat, Inc.