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

[Roman Bogorodskiy]

  Conrad Rad wrote:

> 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?

Hm, that's strange. I didn't check if it logs stuff somewhere but
console as it stops on errors anyway. With your patch applied, I have
the following:

...
prohibit_mixed_case_abbreviations
0.00 prohibit_mixed_case_abbreviations
prohibit_mkstemp
src/bhyve/bhyve_command.c:371:        fd = mkstemp(tmpmapfile);
maint.mk: use mkostemp with O_CLOEXEC instead of mkstemp
gmake: *** [sc_prohibit_mkstemp] Error 1
(18:08) novel at kloomba:~/code/libvirt[bload] %> 

PS I've dropped freebsd-virtualization@ from CC, I guess it's moving out
of scope of that maillist.

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141024/5827b4be/attachment-0001.sig>