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

[Conrad Rad]

On Fri, Oct 24, 2014 at 12:58 AM, Roman Bogorodskiy
<bogorodskiy at gmail.com> 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?

Thanks,
Conrad