[PATCH v2 0/2] bhyve: improve loader handling on arm64
Changes since v1: - Make the default loader uboot path configurable in bhyve.conf. It might be useful for users and also way simpler to mock in the test than the hardcoded value in the code. Roman Bogorodskiy (2): bhyve: improve loader handling on arm64 bhyve: conf: do not hardcode datadir in bhyve.conf src/bhyve/{bhyve.conf => bhyve.conf.in} | 7 +++++- src/bhyve/bhyve_conf.c | 8 +++++++ src/bhyve/bhyve_domain.c | 23 +++++++++++++++++++ src/bhyve/bhyve_utils.h | 2 ++ src/bhyve/meson.build | 13 +++++++++-- .../aarch64/bhyvexml2argv-base.args | 1 + .../aarch64/bhyvexml2argv-base.ldargs | 8 +------ .../aarch64/bhyvexml2argv-console.args | 1 + .../aarch64/bhyvexml2argv-console.ldargs | 8 +------ tests/bhyvexml2argvmock.c | 20 +++++++++++++++- tests/bhyvexml2argvtest.c | 2 ++ .../aarch64/bhyvexml2xmlout-base.xml | 1 + .../aarch64/bhyvexml2xmlout-console.xml | 1 + tests/bhyvexml2xmltest.c | 4 ++++ 14 files changed, 81 insertions(+), 18 deletions(-) rename src/bhyve/{bhyve.conf => bhyve.conf.in} (73%) -- 2.52.0
Bhyve on arm64 does not have the bhyveload(8) tool. That means that it cannot be used as a default if the loader is not configured for the domain. To prevent users from getting unusable configurations, handle loader configuration on arm64 like that: - if loader is specified in the domain XML, just use it - if not specified, try to check whether the default uboot loader is available on the system. In case it is, set is as the loader, otherwise fail with the error. Additionally, the loader could be configured in bhyve.conf. By default, it uses the loader installed by the sysutils/u-boot-bhyve-arm64 port or a corresponding package. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve.conf | 5 ++++ src/bhyve/bhyve_conf.c | 8 +++++++ src/bhyve/bhyve_domain.c | 23 +++++++++++++++++++ src/bhyve/bhyve_utils.h | 2 ++ .../aarch64/bhyvexml2argv-base.args | 1 + .../aarch64/bhyvexml2argv-base.ldargs | 8 +------ .../aarch64/bhyvexml2argv-console.args | 1 + .../aarch64/bhyvexml2argv-console.ldargs | 8 +------ tests/bhyvexml2argvmock.c | 20 +++++++++++++++- tests/bhyvexml2argvtest.c | 2 ++ .../aarch64/bhyvexml2xmlout-base.xml | 1 + .../aarch64/bhyvexml2xmlout-console.xml | 1 + tests/bhyvexml2xmltest.c | 4 ++++ 13 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/bhyve/bhyve.conf b/src/bhyve/bhyve.conf index dc8d3d8fd8..a845937d87 100644 --- a/src/bhyve/bhyve.conf +++ b/src/bhyve/bhyve.conf @@ -6,6 +6,11 @@ # to the directory that sysutils/bhyve-firmware installs files into. #firmware_dir = "/usr/local/share/uefi-firmware" +# Path to the U-Boot loader for the arm64 guests. +# By default it's pointing to the loader installed +# by the sysutils/u-boot-bhyve-arm64 port +#uboot_path = "/usr/local/share/u-boot/u-boot-bhyve-arm64/u-boot.bin" + # Set timeout for the bhyveload(8) command. This might be necessary # because in case of errors bhyveload(8) drops to an interactive # loader and hangs indefinitely. These timeout values are passed diff --git a/src/bhyve/bhyve_conf.c b/src/bhyve/bhyve_conf.c index 182e00ee1d..f9a657f402 100644 --- a/src/bhyve/bhyve_conf.c +++ b/src/bhyve/bhyve_conf.c @@ -61,6 +61,8 @@ virBhyveDriverConfigNew(void) cfg->libDir = g_strdup_printf("%s/lib/libvirt/bhyve", LOCALSTATEDIR); cfg->nvramDir = g_strdup_printf("%s/nvram", cfg->libDir); + cfg->ubootPath = g_strdup(DATADIR "/u-boot/u-boot-bhyve-arm64/u-boot.bin"); + cfg->bhyveloadTimeout = 300; cfg->bhyveloadTimeoutKill = 15; @@ -85,6 +87,10 @@ virBhyveLoadDriverConfig(struct _virBhyveDriverConfig *cfg, &cfg->firmwareDir) < 0) return -1; + if (virConfGetValueString(conf, "uboot_path", + &cfg->ubootPath) < 0) + return -1; + if (virConfGetValueInt(conf, "bhyveload_timeout", &cfg->bhyveloadTimeout) < 0) return -1; @@ -111,6 +117,8 @@ virBhyveDriverConfigDispose(void *obj) g_free(cfg->firmwareDir); g_free(cfg->libDir); g_free(cfg->nvramDir); + + g_free(cfg->ubootPath); } void diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index df0a008ecd..8fc9733756 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -27,6 +27,7 @@ #include "bhyve_domain.h" #include "bhyve_capabilities.h" #include "viralloc.h" +#include "virfile.h" #include "virlog.h" #include "virutil.h" @@ -112,6 +113,28 @@ bhyveDomainDefPostParse(virDomainDef *def, !(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_RTC_UTC)) def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME; + /* bhyve/arm64 does not provide the bhyveload(8) tool, + * so if the loader is not specified and we cannot fall back to the + * default one, then this results in an unusable configuration. */ + if (ARCH_IS_ARM(def->os.arch)) { + if (def->os.loader == NULL) { + g_autoptr(virBhyveDriverConfig) cfg = virBhyveDriverGetConfig(driver); + char *uboot_path = cfg->ubootPath; + + if (virFileExists(uboot_path)) { + def->os.loader = virDomainLoaderDefNew(); + def->os.loader->path = g_strdup(uboot_path); + def->os.loader->readonly = true; + def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("loader is not specified and the default loader (%1$s) not found"), + uboot_path); + return -1; + } + } + } + return 0; } diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h index 8ed1fa5509..19369047fe 100644 --- a/src/bhyve/bhyve_utils.h +++ b/src/bhyve/bhyve_utils.h @@ -42,6 +42,8 @@ struct _virBhyveDriverConfig { char *libDir; char *nvramDir; + char *ubootPath; + int bhyveloadTimeout; int bhyveloadTimeoutKill; }; diff --git a/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-base.args b/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-base.args index aef3ebd017..37292aa3fe 100644 --- a/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-base.args +++ b/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-base.args @@ -2,6 +2,7 @@ bhyve \ -c 1 \ -m 214 \ -s 0:0,hostbridge \ +-o bootrom=fakeubootpath/u-boot.bin \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \ -s 2:0,virtio-blk,/tmp/freebsd.img \ bhyve diff --git a/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-base.ldargs b/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-base.ldargs index 264ae48441..421376db9e 100644 --- a/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-base.ldargs +++ b/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-base.ldargs @@ -1,7 +1 @@ -timeout \ ---foreground \ ---verbose \ --k 20s 300s bhyveload \ --m 214 \ --d /tmp/freebsd.img \ -bhyve +dummy diff --git a/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-console.args b/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-console.args index 4a031afb71..afd5f5d7b2 100644 --- a/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-console.args +++ b/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-console.args @@ -2,6 +2,7 @@ bhyve \ -c 1 \ -m 214 \ -s 0:0,hostbridge \ +-o bootrom=fakeubootpath/u-boot.bin \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \ -s 2:0,virtio-blk,/tmp/freebsd.img \ -o console=/dev/nmdm0A \ diff --git a/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-console.ldargs b/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-console.ldargs index 264ae48441..421376db9e 100644 --- a/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-console.ldargs +++ b/tests/bhyvexml2argvdata/aarch64/bhyvexml2argv-console.ldargs @@ -1,7 +1 @@ -timeout \ ---foreground \ ---verbose \ --k 20s 300s bhyveload \ --m 214 \ --d /tmp/freebsd.img \ -bhyve +dummy diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c index fe76564d51..191d6bdaf5 100644 --- a/tests/bhyvexml2argvmock.c +++ b/tests/bhyvexml2argvmock.c @@ -2,7 +2,9 @@ #include <dirent.h> +#include "configmake.h" #include "viralloc.h" +#include "virfile.h" #include "virstring.h" #include "virnetdev.h" #include "virnetdevtap.h" @@ -12,11 +14,16 @@ #define VIR_FROM_THIS VIR_FROM_BHYVE static DIR * (*real_opendir)(const char *name); +static bool (*real_virFileExists)(const char *path); static void init_syms(void) { - VIR_MOCK_REAL_INIT(opendir); + if (!real_opendir) + VIR_MOCK_REAL_INIT(opendir); + + if (!real_virFileExists) + VIR_MOCK_REAL_INIT(virFileExists); } #define FAKEFIRMWAREDIR abs_srcdir "/bhyvefirmwaredata/three_firmwares" @@ -89,3 +96,14 @@ int bind(int sockfd G_GNUC_UNUSED, { return 0; } + +bool +virFileExists(const char *path) +{ + init_syms(); + + if (STREQ(path, "fakeubootpath/u-boot.bin")) + return true; + + return real_virFileExists(path); +} diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index c7c18c3690..42c14b6ab3 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -151,6 +151,7 @@ mymain(void) g_autofree char *fakefirmwaredir = g_strdup("fakefirmwaredir"); g_autofree char *fakenvramdir = g_strdup("fakenvramdir"); g_autofree char *fakefirmwareemptydir = g_strdup("fakefirmwareemptydir"); + g_autofree char *fakeubootpath = g_strdup("fakeubootpath/u-boot.bin"); if ((driver.caps = virBhyveCapsBuild()) == NULL) return EXIT_FAILURE; @@ -166,6 +167,7 @@ mymain(void) driver.config->firmwareDir = fakefirmwaredir; driver.config->nvramDir = fakenvramdir; + driver.config->ubootPath = fakeubootpath; driver.config->bhyveloadTimeout = 0; driver.config->bhyveloadTimeoutKill = 0; diff --git a/tests/bhyvexml2xmloutdata/aarch64/bhyvexml2xmlout-base.xml b/tests/bhyvexml2xmloutdata/aarch64/bhyvexml2xmlout-base.xml index ee72370047..d6c9caa225 100644 --- a/tests/bhyvexml2xmloutdata/aarch64/bhyvexml2xmlout-base.xml +++ b/tests/bhyvexml2xmloutdata/aarch64/bhyvexml2xmlout-base.xml @@ -6,6 +6,7 @@ <vcpu placement='static'>1</vcpu> <os> <type arch='aarch64'>hvm</type> + <loader readonly='yes' type='pflash'>/usr/local/share/u-boot/u-boot-bhyve-arm64/u-boot.bin</loader> <boot dev='hd'/> </os> <clock offset='localtime'/> diff --git a/tests/bhyvexml2xmloutdata/aarch64/bhyvexml2xmlout-console.xml b/tests/bhyvexml2xmloutdata/aarch64/bhyvexml2xmlout-console.xml index d43ce8fd6f..d694ecfb8d 100644 --- a/tests/bhyvexml2xmloutdata/aarch64/bhyvexml2xmlout-console.xml +++ b/tests/bhyvexml2xmloutdata/aarch64/bhyvexml2xmlout-console.xml @@ -6,6 +6,7 @@ <vcpu placement='static'>1</vcpu> <os> <type arch='aarch64'>hvm</type> + <loader readonly='yes' type='pflash'>/usr/local/share/u-boot/u-boot-bhyve-arm64/u-boot.bin</loader> <boot dev='hd'/> </os> <clock offset='localtime'/> diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c index 5df1f2b6ba..50bf82bd52 100644 --- a/tests/bhyvexml2xmltest.c +++ b/tests/bhyvexml2xmltest.c @@ -5,6 +5,7 @@ #ifdef WITH_BHYVE # include "bhyve/bhyve_capabilities.h" +# include "bhyve/bhyve_conf.h" # include "bhyve/bhyve_domain.h" # include "bhyve/bhyve_utils.h" @@ -63,6 +64,9 @@ mymain(void) if ((driver.xmlopt = virBhyveDriverCreateXMLConf(&driver)) == NULL) return EXIT_FAILURE; + if (!(driver.config = virBhyveDriverConfigNew())) + return EXIT_FAILURE; + # define DO_TEST_FULL(name, flags) \ do { \ const struct testInfo info = {name, (flags)}; \ -- 2.52.0
Do not hardcode "/usr/local/share" directory in the configuration file for the firmware and uboot paths. Use meson's configure_file()/configuration_data() to substitute it with the "datadir" value. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/{bhyve.conf => bhyve.conf.in} | 4 ++-- src/bhyve/meson.build | 13 +++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) rename src/bhyve/{bhyve.conf => bhyve.conf.in} (87%) diff --git a/src/bhyve/bhyve.conf b/src/bhyve/bhyve.conf.in similarity index 87% rename from src/bhyve/bhyve.conf rename to src/bhyve/bhyve.conf.in index a845937d87..5ed6a14db5 100644 --- a/src/bhyve/bhyve.conf +++ b/src/bhyve/bhyve.conf.in @@ -4,12 +4,12 @@ # Path to a directory with firmware files. By default it's pointing # to the directory that sysutils/bhyve-firmware installs files into. -#firmware_dir = "/usr/local/share/uefi-firmware" +#firmware_dir = "@DATADIR@/uefi-firmware" # Path to the U-Boot loader for the arm64 guests. # By default it's pointing to the loader installed # by the sysutils/u-boot-bhyve-arm64 port -#uboot_path = "/usr/local/share/u-boot/u-boot-bhyve-arm64/u-boot.bin" +#uboot_path = "@DATADIR@/u-boot/u-boot-bhyve-arm64/u-boot.bin" # Set timeout for the bhyveload(8) command. This might be necessary # because in case of errors bhyveload(8) drops to an interactive diff --git a/src/bhyve/meson.build b/src/bhyve/meson.build index 11920d9c3e..564b7e5db2 100644 --- a/src/bhyve/meson.build +++ b/src/bhyve/meson.build @@ -50,12 +50,21 @@ if conf.has('WITH_BHYVE') ], } - virt_conf_files += files('bhyve.conf') + bhyve_conf_data = configuration_data({ + 'DATADIR': datadir, + }) + bhyve_conf = configure_file( + input: 'bhyve.conf.in', + output: 'bhyve.conf', + configuration: bhyve_conf_data, + ) + + virt_conf_files += bhyve_conf virt_aug_files += files('libvirtd_bhyve.aug') virt_test_aug_files += { 'name': 'test_libvirtd_bhyve.aug', 'aug': files('test_libvirtd_bhyve.aug.in'), - 'conf': files('bhyve.conf'), + 'conf': bhyve_conf, 'test_name': 'libvirtd_bhyve', 'test_srcdir': meson.current_source_dir(), 'test_builddir': meson.current_build_dir(), -- 2.52.0
participants (1)
-
Roman Bogorodskiy