Conrad Meyer wrote:

> We still default to bhyveloader(1) if no explicit bootloader
> configuration is supplied in the domain.
> 
> If the /domain/bootloader looks like grub-bhyve and the user doesn't
> supply /domain/bootloader_args, we make an intelligent guess and try
> chainloading the first partition on the disk.
> 
> Caveats:
> - We can't install from CD without explicit bootloader_args.
> - We leave a device.map file lying around in /tmp. I don't see a good
>   way not to do so without reworking the API somewhat.
>
> Sponsored by:   EMC / Isilon storage division

Hi Conrad.

Thanks for the patch. As for the /tmp, probably storing it somewhere in
BHYVE_STATE_DIR would be more clean.

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.

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

Actually, I've implemented that already, but didn't send because it
needs some more testing.

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.

There are some comments inline.

> Signed-off-by: Conrad Meyer <conrad.me...@isilon.com>
> ---
>  docs/drvbhyve.html.in     |  28 ++++++++++--
>  docs/formatdomain.html.in |   4 +-
>  po/libvirt.pot            |   4 ++
>  src/bhyve/bhyve_command.c | 107 
> +++++++++++++++++++++++++++++++++++++++++-----
>  4 files changed, 128 insertions(+), 15 deletions(-)
> 
> diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in
> index 39afdf5..6e85800 100644
> --- a/docs/drvbhyve.html.in
> +++ b/docs/drvbhyve.html.in
> @@ -37,8 +37,7 @@ bhyve+ssh://r...@example.com/system (remote access, SSH 
> tunnelled)
>  <h3>Example config</h3>
>  <p>
>  The bhyve driver in libvirt is in its early stage and under active 
> development. So it supports
> -only limited number of features bhyve provides. All the supported features 
> could be found
> -in this sample domain XML.
> +only limited number of features bhyve provides.
>  </p>
>  
>  <p>
> @@ -50,8 +49,8 @@ up to 31 PCI devices.
>  
>  <pre>
>  &lt;domain type='bhyve'&gt;
> -  &lt;name&gt;bhyve&lt;/name&gt;
> -  &lt;uuid&gt;df3be7e7-a104-11e3-aeb0-50e5492bd3dc&lt;/uuid&gt;
> +    &lt;name&gt;bhyve&lt;/name&gt;
> +    &lt;uuid&gt;df3be7e7-a104-11e3-aeb0-50e5492bd3dc&lt;/uuid&gt;
>      &lt;memory&gt;219136&lt;/memory&gt;
>      &lt;currentMemory&gt;219136&lt;/currentMemory&gt;
>      &lt;vcpu&gt;1&lt;/vcpu&gt;
> @@ -157,5 +156,26 @@ An example of domain XML device entry for that will look 
> like:</p>
>  <p>Please refer to the <a href="storage.html">Storage documentation</a> for 
> more details on storage
>  management.</p>
>  
> +<h3><a name="grubbhyve">Using grub2-bhyve or Alternative Bootloaders</a></h3>
> +
> +<p>It's possible to boot non-FreeBSD guests by specifying an explicit
> +bootloader, e.g. <code>grub-bhyve(1)</code>. Arguments to the bootloader may 
> be
> +specified as well. If no arguments are given and bootloader is
> +<code>grub-bhyve</code>, libvirt will try and boot from the first partition 
> of
> +the disk image.</p>
> +
> +<pre>
> +  ...
> +    &lt;bootloader&gt;/usr/local/sbin/grub-bhyve&lt;/bootloader&gt;
> +    &lt;bootloader_args&gt;...&lt;/bootloader_args&gt;
> +  ...
> +</pre>
> +
> +<p>(Of course, to install from a CD a user will have to supply explicit
> +arguments to <code>grub-bhyve</code>.)</p>
> +
> +<p>Caveat: <code>bootloader_args</code> does not support any quoting.
> +Filenames, etc, must not have spaces or they will be tokenized 
> incorrectly.</p>
> +
>    </body>
>  </html>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0099ce7..b7b6c46 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -217,7 +217,9 @@
>        a BIOS, and instead the host is responsible to kicking off the
>        operating system boot. This may use a pseudo-bootloader in the
>        host to provide an interface to choose a kernel for the guest.
> -      An example is <code>pygrub</code> with Xen.
> +      An example is <code>pygrub</code> with Xen. The Bhyve hypervisor
> +      also uses a host bootloader, either <code>bhyveload</code> or
> +      <code>grub-bhyve</code>.
>      </p>
>  
>  <pre>
> diff --git a/po/libvirt.pot b/po/libvirt.pot
> index 0b44ad7..d8c9a4d 100644
> --- a/po/libvirt.pot
> +++ b/po/libvirt.pot
> @@ -851,6 +851,10 @@ msgstr ""
>  msgid "domain should have at least one disk defined"
>  msgstr ""
>  
> +#: src/bhyve/bhyve_command.c:407
> +msgid "Custom loader requires explicit %s configuration"
> +msgstr ""
> +
>  #: src/bhyve/bhyve_device.c:50
>  msgid "PCI bus 0 slot 1 is reserved for the implicit LPC PCI-ISA bridge"
>  msgstr ""
> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index bea4a59..99956ae 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -300,6 +300,7 @@ virBhyveProcessBuildLoadCmd(virConnectPtr conn,
>  {
>      virCommandPtr cmd;
>      virDomainDiskDefPtr disk;
> +    bool bhyveload, grub_bhyve;
>  
>      if (def->ndisks < 1) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -326,19 +327,105 @@ virBhyveProcessBuildLoadCmd(virConnectPtr conn,
>          return NULL;
>      }
>  
> -    cmd = virCommandNew(BHYVELOAD);
> +    if (def->os.bootloader == NULL) {
> +        bhyveload = true;
> +        grub_bhyve = false;
> +        cmd = virCommandNew(BHYVELOAD);
> +    } else {
> +        bhyveload = false;
> +        if (strstr(def->os.bootloader, "grub-bhyve") == 0)
> +            grub_bhyve = true;
> +        cmd = virCommandNew(def->os.bootloader);
> +    }
>  
> -    /* Memory */
> -    virCommandAddArg(cmd, "-m");
> -    virCommandAddArgFormat(cmd, "%llu",
> -                           VIR_DIV_UP(def->mem.max_balloon, 1024));
> +    if (bhyveload && def->os.bootloaderArgs == NULL) {
> +        VIR_DEBUG("%s: bhyveload with default arguments", __func__);
> +
> +        /* Memory (MB) */
> +        virCommandAddArg(cmd, "-m");
> +        virCommandAddArgFormat(cmd, "%llu",
> +                               VIR_DIV_UP(def->mem.max_balloon, 1024));
> +
> +        /* Image path */
> +        virCommandAddArg(cmd, "-d");
> +        virCommandAddArg(cmd, virDomainDiskGetSource(disk));
> +
> +        /* VM name */
> +        virCommandAddArg(cmd, def->name);
> +    } else if (grub_bhyve && def->os.bootloaderArgs == NULL) {
> +        char tmpmapfile[128] = "/tmp/grub-bhyve-device.map_XXXXXX";
> +        FILE *f;
> +        int fd;
> +
> +        VIR_DEBUG("%s: grub-bhyve with default arguments", __func__);
> +
> +        /*
> +         * XXX Default grub-bhyve has some BIG caveats, but MAY work for some
> +         * typical configurations. In particular:
> +         *
> +         *   - Can't create a new VM this way (no CD, no boot from CD)
> +         *   - Assumes a GRUB install on hd0,msdos1
> +         */
>  
> -    /* 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.

> +        if (fd < 0) {
> +                virReportError(VIR_ERR_OPEN_FAILED, tmpmapfile);
> +                goto error;
> +        }
> +
> +        f = VIR_FDOPEN(fd, "wb+");
> +        if (f == NULL) {
> +                VIR_FORCE_CLOSE(fd);
> +                virReportError(VIR_ERR_OPEN_FAILED, tmpmapfile);
> +                goto error;
> +        }
> +
> +        /* Grub device.map */
> +        fprintf(f, "(hd0) %s\n", virDomainDiskGetSource(disk));
> +        /* XXX CDs would look like: "(cd0) /path/to/CD" */
> +
> +        if (VIR_FCLOSE(f) < 0) {
> +                virReportSystemError(errno, "%s", _("failed to close file"));
> +                goto error;
> +        }
>  
> -    /* VM name */
> -    virCommandAddArg(cmd, def->name);
> +
> +        virCommandAddArg(cmd, "--device-map");
> +        virCommandAddArg(cmd, tmpmapfile);
> +
> +        /* Memory in MB */
> +        virCommandAddArg(cmd, "--memory");
> +        virCommandAddArgFormat(cmd, "%llu",
> +                               VIR_DIV_UP(def->mem.max_balloon, 1024));
> +
> +        /* To boot from CD, "cd0" here. */
> +        virCommandAddArg(cmd, "--root");
> +        virCommandAddArg(cmd, "hd0,msdos1");
> +
> +        /* VM name */
> +        virCommandAddArg(cmd, def->name);
> +    } else if (def->os.bootloaderArgs == NULL) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Custom loader requires explicit %s configuration"),
> +                       "bootloader_args");
> +        goto error;
> +    } else {
> +        char **blargs, **arg;
> +
> +        VIR_DEBUG("%s: custom loader '%s' with arguments", __func__,
> +                  def->os.bootloader);
> +
> +        /* XXX: Handle quoted? */
> +        blargs = virStringSplit(def->os.bootloaderArgs, " ", 0);
> +        for (arg = blargs; *arg; arg++)
> +                virCommandAddArg(cmd, *arg);
> +        virStringFreeList(blargs);
> +    }
>  
>      return cmd;
> +
> +error:
> +    virCommandFree(cmd);
> +    return NULL;
>  }
> -- 
> 1.9.3
> 
> _______________________________________________
> freebsd-virtualization@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
> To unsubscribe, send any mail to 
> "freebsd-virtualization-unsubscr...@freebsd.org"

Thanks,

Roman Bogorodskiy
_______________________________________________
freebsd-virtualization@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

Reply via email to