On 2/11/21 11:05 AM, Roman Bogorodskiy wrote: > Cole Robinson wrote: > >> On 2/6/21 8:18 AM, Roman Bogorodskiy wrote: >>> Bhyve requires explicit loader configuration. So query >>> domain capabilities, try to find the "official" >>> firmware and configure all the necessary loader options. >>> >>> Signed-off-by: Roman Bogorodskiy <bogorodskiy@xxxxxxxxx> >>> --- >>> virtinst/domain/os.py | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/virtinst/domain/os.py b/virtinst/domain/os.py >>> index 59fbc43e..4a9fbc81 100644 >>> --- a/virtinst/domain/os.py >>> +++ b/virtinst/domain/os.py >>> @@ -127,3 +127,19 @@ class DomainOs(XMLBuilder): >>> self.init = "/sbin/init" >>> else: >>> self.init = "/bin/sh" >>> + if self.conn.is_bhyve(): >>> + dom_caps = guest.lookup_domcaps() >>> + firmware_files = [f.value for f in dom_caps.os.loader.values] >>> + if not firmware_files: >>> + return >>> + # Prefer the known BHYVE_UEFI.fd firmware >>> + # from the sysutils/bhyve-firmware port. If not found, >>> + # just pick the first one >>> + firmware_file = None >>> + for f in firmware_files: >>> + if 'BHYVE_UEFI.fd' in f: >>> + firmware_file = f >>> + break >>> + self.loader = firmware_file or firmware_files[0] >>> + self.loader_type = "pflash" >>> + self.loader_ro = True >>> >> >> Following on from my cover letter response. I don't think anything needs >> to be changed here in os.py. I think this should be two patches: > > Oh, I've just re-read it before responding and noticed you suggested to > do that in two patches, and I've already sent v2. Change #2 appears to > be a one line change though, I'll look, maybe I've missed some tests > that need to be updated. > No problem your patches are fine, I pushed them now >> 1) teaching guest.py:get_uefi_path() and >> domcaps.find_uefi_path_for_arch() about the preferred bhyve path. You >> can test this in test_cli.py first by `--boot uefi` generating correct XML. >> >> 2) extending guest.py:prefers_uefi() to handle bhyve too. That should >> change the virt-install/virt-manager default automatically. >> >> question: does bhyve libvirt driver support <os firmware='efi'/> ? It >> would be nice if virt-install didn't need to add more of this firmware >> filename scraping > > IIUC, <os firmware='efi'/> instructs libvirt to find a proper firmware > automatically, similarly to what I'm trying to implement here in > virt-manager. I'll see if I can implement that for the bhyve driver. > The main benefit for virt-install is that it lets us stop doing this sort of filename scraping which in qemu land was a treadmill of changes that needed to be duplicated across multiple tools. firmware='efi' is the preferred way forward Thanks, Cole
