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

On Fri, Oct 24, 2014 at 12:58 AM, Roman Bogorodskiy <bogorodskiy(a)gmail.com&gt; wrote: > Conrad Meyer wrote: > Hi Conrad. > > Thanks for the patch. As for the /tmp, probably storing it somewhere in > BHYVE_STATE_DIR would be more clean. Sure. > However, I'm concerned about libvirt dealing with the grub-bhyve > specifics as it involves some assumptions about its behaviour ahd has > limitations, like the one you pointed with installing from CD. Sure. However, the user can always override the assumptions. And I think the CD case is actually quite easy to fix, if that's a blocking issue. > My idea was to add the following behaviour: > > * bootloader specified and bootloader_args specified -- use that as is > * bootloader not specified, bootloader_args specified -- call > bhyveload $bootloader_args $default_bhyveloader_args > * bootloader and bootloader_args not set -- use defaults (current > behaviour) > * bootloader specified, bootloader_args missing -- fail here Ok. I think the grub-bhyve stuff is just a special case of the 4th bullet here; but otherwise my patch matches up with this behavior. > Actually, I've implemented that already, but didn't send because it > needs some more testing. Ok. I am happy to guinea-pig bhyve improvements for libvirt, if you need it. > The reason of my concern about adding a special handling of grub-bhyve > case that, as I've mentioned, we have to make assumptions and there are > also caveats you mentioned. Probably it'd worth to wait for bhyve to > provide single-step VM boot, i.e. UEFI support, that should appear soon > anyway. Any idea how soon? Months? A year? 5 years? I'm not comfortable postponing improvements indefinitely for vaporware. In the wonderful bhyve-UEFI future, we can ignore/warn about <bootloader>. > There are some comments inline. Thanks! >> - /* Image path */ >> - virCommandAddArg(cmd, "-d"); >> - virCommandAddArg(cmd, virDomainDiskGetSource(disk)); >> + /* XXX cleanup this file. */ >> + fd = mkstemp(tmpmapfile); > > This yields an error in 'syntax-check'. Running 'make syntax-check' > allows to avoid some of the general code and formatting issues. I did run syntax-check, but did not see any error around there. syntax-check has lots of console output; does it log somewhere else too?