Re: [libvirt] [PATCHv5 1/6] bhyve: Support /domain/bootloader configuration for non-FreeBSD guests.

On Tue, Oct 28, 2014 at 10:39 AM, Daniel P. Berrange > >> + >> + virCommandAddArg(cmd, "--root"); >> + virCommandAddArg(cmd, "cd"); >> + } else { >> + VIR_WARN("Trying to boot hd0,msdos1 with grub-bhyve. If this is " >> + "not what you wanted, specify <bootloader_args>"); >> + >> + virCommandAddArg(cmd, "--root"); >> + virCommandAddArg(cmd, "hd0,msdos1"); >> + } > > As mentioned above we have spport for per-device boot indexes, but in > the absence of that, I think you should at least be honouring the > traditianal <boot dev="cdrom|disk|network"> element in the XML config > rather than hardcoding priority for cdrom over disks. I think that's an orthogonal improvement (bhyveload currently doesn't support ordering at all). This patch set is already getting large, can this improvement wait?

>> diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h >> index b8ef22a..6ecd395 100644 >> --- a/src/bhyve/bhyve_domain.h >> +++ b/src/bhyve/bhyve_domain.h >> @@ -31,6 +31,7 @@ typedef bhyveDomainObjPrivate *bhyveDomainObjPrivatePtr; >> struct _bhyveDomainObjPrivate { >> virDomainPCIAddressSetPtr pciaddrs; >> bool persistentAddrs; >> + char *grub_devicesmap_file; >> }; > > I'm wondering if we need to store this filename here. If we restart > libvirtd while a bhyve guest is running, then I think we loose this > filename data, so we'd then miss the cleanup. > > Perhaps it is better if we just make the bhve guest shutdown method > re-create the filename string and unconditionally unlink it, ignoring > any ENOENT error. I think we can probably just remove it from the object if we're returning the string contents out to the caller — the file is very short-lived; we only need it in the routine that launches the loader, synchronously waits for it to complete, and then asynchronously launches bhyve itself.