[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Hi all,

There is an issue in this patch as-is; the short version is this is needed:

--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -162,7 +162,7 @@ virBhyveProcessStart(virConnectPtr conn,
      * domain is ready to be started, so we can build
      * and execute bhyveload command */
     rc = virBhyveFormatDevMapFile(vm->def->name, &devmap_file);
-    if (rc)
+    if (rc < 0)
         goto cleanup;

     if (!(load_cmd = virBhyveProcessBuildLoadCmd(conn, vm->def, devmap_file,

Apologies for sending a broken patchset. At the time I think compile
was broken due to the multicast stuff on FreeBSD and I failed to test
it. I will send a v8 set, but I'm at a conference on the laptop and
don't have git-send-email set up here. I will try and resend
Wednesday, if not sooner.

Thanks,
Conrad


On Thu, Oct 30, 2014 at 11:56 AM, Conrad Meyer <cse cem gmail com> 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 (or a CD if one exists,
> under the assumption that for a VM a CD is likely an install source).
>
> Caveat: Assumes the HDD boots from the msdos1 partition. I think this is
> a pretty reasonable assumption for a VM. (DrvBhyve with Bhyveload
> already assumes that the first disk should be booted.)
>
> I've tested both HDD and CD boot and they seem to work.
> ---
>  docs/drvbhyve.html.in     | 100 +++++++++++++++++++++++++--
>  docs/formatdomain.html.in |   4 +-
>  src/bhyve/bhyve_command.c | 173 +++++++++++++++++++++++++++++++++++++++++-----
>  src/bhyve/bhyve_command.h |   5 +-
>  src/bhyve/bhyve_driver.c  |   3 +-
>  src/bhyve/bhyve_process.c |  38 +++++++++-
>  6 files changed, 295 insertions(+), 28 deletions(-)
>
> diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in
> index 39afdf5..bd4b35e 100644
> --- a/docs/drvbhyve.html.in
> +++ b/docs/drvbhyve.html.in
> @@ -37,8 +37,7 @@ bhyve+ssh://root 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>
> @@ -48,10 +47,21 @@ disk device were supported per-domain. However,
>  up to 31 PCI devices.
>  </p>
>
> +<p>
> +Note: the Bhyve driver in libvirt will boot whichever device is first. If you
> +want to install from CD, put the CD device first. If not, put the root HDD
> +first.
> +</p>
> +
> +<p>
> +Note: Only the SATA bus is supported. Only <code>cdrom</code>- and
> +<code>disk</code>-type disks are supported.
> +</p>
> +
>  <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;
> @@ -76,6 +86,7 @@ up to 31 PCI devices.
>          &lt;driver name='file' type='raw'/&gt;
>          &lt;source file='/path/to/cdrom.iso'/&gt;
>          &lt;target dev='hdc' bus='sata'/&gt;
> +        &lt;readonly/&gt;
>        &lt;/disk&gt;
>        &lt;interface type='bridge'&gt;
>          &lt;model type='virtio'/&gt;
> @@ -85,6 +96,53 @@ up to 31 PCI devices.
>  &lt;/domain&gt;
>  </pre>
>
> +<p>(The &lt;disk&gt; sections may be swapped in order to install from
> +<em>cdrom.iso</em>.)</p>
> +
> +<h3>Example config (Linux guest)</h3>
> +
> +<p>
> +Note the addition of &lt;bootloader&gt;.
> +</p>
> +
> +<pre>
> +&lt;domain type='bhyve'&gt;
> +    &lt;name&gt;linux_guest&lt;/name&gt;
> +    &lt;uuid&gt;df3be7e7-a104-11e3-aeb0-50e5492bd3dc&lt;/uuid&gt;
> +    &lt;memory&gt;131072&lt;/memory&gt;
> +    &lt;currentMemory&gt;131072&lt;/currentMemory&gt;
> +    &lt;vcpu&gt;1&lt;/vcpu&gt;
> +    &lt;bootloader&gt;/usr/local/sbin/grub-bhyve&lt;/bootloader&gt;
> +    &lt;os&gt;
> +       &lt;type&gt;hvm&lt;/type&gt;
> +    &lt;/os&gt;
> +    &lt;features&gt;
> +      &lt;apic/&gt;
> +      &lt;acpi/&gt;
> +    &lt;/features&gt;
> +    &lt;clock offset='utc'/&gt;
> +    &lt;on_poweroff&gt;destroy&lt;/on_poweroff&gt;
> +    &lt;on_reboot&gt;restart&lt;/on_reboot&gt;
> +    &lt;on_crash&gt;destroy&lt;/on_crash&gt;
> +    &lt;devices&gt;
> +      &lt;disk type='file' device='disk'&gt;
> +        &lt;driver name='file' type='raw'/&gt;
> +        &lt;source file='/path/to/guest_hdd.img'/&gt;
> +        &lt;target dev='hda' bus='sata'/&gt;
> +      &lt;/disk&gt;
> +      &lt;disk type='file' device='cdrom'&gt;
> +        &lt;driver name='file' type='raw'/&gt;
> +        &lt;source file='/path/to/cdrom.iso'/&gt;
> +        &lt;target dev='hdc' bus='sata'/&gt;
> +        &lt;readonly/&gt;
> +      &lt;/disk&gt;
> +      &lt;interface type='bridge'&gt;
> +        &lt;model type='virtio'/&gt;
> +        &lt;source bridge="virbr0"/&gt;
> +      &lt;/interface&gt;
> +    &lt;/devices&gt;
> +&lt;/domain&gt;
> +</pre>
>
>  <h2><a name="usage">Guest usage / management</a></h2>
>
> @@ -119,6 +177,20 @@ to let a guest boot or start a guest using:</p>
>
>  <pre>start --console domname</pre>
>
> +<p><b>NB:</b> An bootloader configured to require user interaction will prevent
> +the domain from starting (and thus <code>virsh console</code> or <code>start
> +--console</code> from functioning) until the user interacts with it manually on
> +the VM host. Because users typically do not have access to the VM host,
> +interactive bootloaders are unsupported by libvirt. <em>However,</em> if you happen to
> +run into this scenario and also happen to have access to the Bhyve host
> +machine, you may select a boot option and allow the domain to finish starting
> +by using an alternative terminal client on the VM host to connect to the
> +domain-configured null modem device. One example (assuming
> +<code>/dev/nmdm0B</code> is configured as the slave end of the domain serial
> +device) is:</p>
> +
> +<pre>cu -l /dev/nmdm0B</pre>
> +
>  <h3><a name="xmltonative">Converting from domain XML to Bhyve args</a></h3>
>
>  <p>
> @@ -157,5 +229,25 @@ 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 the bootloader is <code>grub-bhyve</code> and arguments
> +are omitted, libvirt will try and boot the first disk in the domain (either
> +<code>cdrom</code>- or <code>disk</code>-type devices). If the disk type is
> +<code>disk</code>, it will attempt to boot from the first partition in 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>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/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
> index bea4a59..203495c 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -26,6 +26,8 @@
>  #include <net/if_tap.h>
>
>  #include "bhyve_command.h"
> +#include "bhyve_domain.h"
> +#include "datatypes.h"
>  #include "viralloc.h"
>  #include "virfile.h"
>  #include "virstring.h"
> @@ -294,51 +296,186 @@ virBhyveProcessBuildDestroyCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED,
>      return cmd;
>  }
>
> -virCommandPtr
> -virBhyveProcessBuildLoadCmd(virConnectPtr conn,
> -                            virDomainDefPtr def)
> +static void
> +virAppendBootloaderArgs(virCommandPtr cmd, virDomainDefPtr def)
> +{
> +    char **blargs;
> +
> +    /* XXX: Handle quoted? */
> +    blargs = virStringSplit(def->os.bootloaderArgs, " ", 0);
> +    virCommandAddArgSet(cmd, (const char * const *)blargs);
> +    virStringFreeList(blargs);
> +}
> +
> +static virCommandPtr
> +virBhyveProcessBuildBhyveloadCmd(virDomainDefPtr def, virDomainDiskDefPtr disk)
>  {
>      virCommandPtr cmd;
> -    virDomainDiskDefPtr disk;
>
> -    if (def->ndisks < 1) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("domain should have at least one disk defined"));
> +    cmd = virCommandNew(BHYVELOAD);
> +
> +    if (def->os.bootloaderArgs == NULL) {
> +        VIR_DEBUG("bhyveload with default arguments");
> +
> +        /* 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 {
> +        VIR_DEBUG("bhyveload with arguments");
> +        virAppendBootloaderArgs(cmd, def);
> +    }
> +
> +    return cmd;
> +}
> +
> +static virCommandPtr
> +virBhyveProcessBuildCustomLoaderCmd(virDomainDefPtr def)
> +{
> +    virCommandPtr cmd;
> +
> +    if (def->os.bootloaderArgs == NULL) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Custom loader requires explicit %s configuration"),
> +                       "bootloader_args");
>          return NULL;
>      }
>
> -    disk = def->disks[0];
> +    VIR_DEBUG("custom loader '%s' with arguments", def->os.bootloader);
> +
> +    cmd = virCommandNew(def->os.bootloader);
> +    virAppendBootloaderArgs(cmd, def);
> +    return cmd;
> +}
> +
> +static bool
> +virBhyveUsableDisk(virConnectPtr conn, virDomainDiskDefPtr disk)
> +{
>
>      if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
> -        return NULL;
> +        return false;
>
>      if ((disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) &&
>          (disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("unsupported disk device"));
> -        return NULL;
> +        return false;
>      }
>
>      if ((virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_FILE) &&
>          (virDomainDiskGetType(disk) != VIR_STORAGE_TYPE_VOLUME)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                         _("unsupported disk type"));
> -        return NULL;
> +        return false;
>      }
>
> -    cmd = virCommandNew(BHYVELOAD);
> +    return true;
> +}
>
> -    /* Memory */
> -    virCommandAddArg(cmd, "-m");
> +static virCommandPtr
> +virBhyveProcessBuildGrubbhyveCmd(virDomainDefPtr def,
> +                                 virConnectPtr conn,
> +                                 const char *devmap_file,
> +                                 char **devicesmap_out)
> +{
> +    virDomainDiskDefPtr disk, cd;
> +    virBuffer devicemap;
> +    virCommandPtr cmd;
> +    size_t i;
> +
> +    if (def->os.bootloaderArgs != NULL)
> +        return virBhyveProcessBuildCustomLoaderCmd(def);
> +
> +    devicemap = (virBuffer)VIR_BUFFER_INITIALIZER;
> +
> +    /* Search disk list for CD or HDD device. */
> +    cd = disk = NULL;
> +    for (i = 0; i < def->ndisks; i++) {
> +        if (!virBhyveUsableDisk(conn, def->disks[i]))
> +            continue;
> +
> +        if (cd == NULL &&
> +            def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
> +            cd = def->disks[i];
> +            VIR_INFO("Picking %s as boot CD", virDomainDiskGetSource(cd));
> +        }
> +
> +        if (disk == NULL &&
> +            def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> +            disk = def->disks[i];
> +            VIR_INFO("Picking %s as HDD", virDomainDiskGetSource(disk));
> +        }
> +    }
> +
> +    cmd = virCommandNew(def->os.bootloader);
> +
> +    VIR_DEBUG("grub-bhyve with default arguments");
> +
> +    if (devicesmap_out != NULL) {
> +        /* Grub device.map (just for boot) */
> +        if (disk != NULL)
> +            virBufferAsprintf(&devicemap, "(hd0) %s\n",
> +                              virDomainDiskGetSource(disk));
> +
> +        if (cd != NULL)
> +            virBufferAsprintf(&devicemap, "(cd) %s\n",
> +                              virDomainDiskGetSource(cd));
> +
> +        *devicesmap_out = virBufferContentAndReset(&devicemap);
> +    }
> +
> +    if (cd != NULL) {
> +        virCommandAddArg(cmd, "--root");
> +        virCommandAddArg(cmd, "cd");
> +    } else {
> +        virCommandAddArg(cmd, "--root");
> +        virCommandAddArg(cmd, "hd0,msdos1");
> +    }
> +
> +    virCommandAddArg(cmd, "--device-map");
> +    virCommandAddArg(cmd, devmap_file);
> +
> +    /* Memory in MB */
> +    virCommandAddArg(cmd, "--memory");
>      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);
>
>      return cmd;
>  }
> +
> +virCommandPtr
> +virBhyveProcessBuildLoadCmd(virConnectPtr conn, virDomainDefPtr def,
> +                            const char *devmap_file, char **devicesmap_out)
> +{
> +    virDomainDiskDefPtr disk;
> +
> +    if (def->ndisks < 1) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("domain should have at least one disk defined"));
> +        return NULL;
> +    }
> +
> +    if (def->os.bootloader == NULL) {
> +        disk = def->disks[0];
> +
> +        if (!virBhyveUsableDisk(conn, disk))
> +            return NULL;
> +
> +        return virBhyveProcessBuildBhyveloadCmd(def, disk);
> +    } else if (strstr(def->os.bootloader, "grub-bhyve") != NULL) {
> +        return virBhyveProcessBuildGrubbhyveCmd(def, conn, devmap_file,
> +                                                devicesmap_out);
> +    } else {
> +        return virBhyveProcessBuildCustomLoaderCmd(def);
> +    }
> +}
> diff --git a/src/bhyve/bhyve_command.h b/src/bhyve/bhyve_command.h
> index 5b323bf..22a959d 100644
> --- a/src/bhyve/bhyve_command.h
> +++ b/src/bhyve/bhyve_command.h
> @@ -22,6 +22,7 @@
>  #ifndef __BHYVE_COMMAND_H__
>  # define __BHYVE_COMMAND_H__
>
> +# include "bhyve_domain.h"
>  # include "bhyve_utils.h"
>
>  # include "domain_conf.h"
> @@ -38,7 +39,7 @@ virBhyveProcessBuildDestroyCmd(bhyveConnPtr driver,
>                                 virDomainDefPtr def);
>
>  virCommandPtr
> -virBhyveProcessBuildLoadCmd(virConnectPtr conn,
> -                            virDomainDefPtr def);
> +virBhyveProcessBuildLoadCmd(virConnectPtr conn, virDomainDefPtr def,
> +                            const char *devmap_file, char **devicesmap_out);
>
>  #endif /* __BHYVE_COMMAND_H__ */
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index eb0d455..4aee249 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -689,7 +689,8 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn,
>      if (bhyveDomainAssignAddresses(def, NULL) < 0)
>          goto cleanup;
>
> -    if (!(loadcmd = virBhyveProcessBuildLoadCmd(conn, def)))
> +    if (!(loadcmd = virBhyveProcessBuildLoadCmd(conn, def, "<device.map>",
> +                                                NULL)))
>          goto cleanup;
>
>      if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, def, true)))
> diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
> index 0bbe388..71dc8ee 100644
> --- a/src/bhyve/bhyve_process.c
> +++ b/src/bhyve/bhyve_process.c
> @@ -88,6 +88,14 @@ bhyveNetCleanup(virDomainObjPtr vm)
>      }
>  }
>
> +static int
> +virBhyveFormatDevMapFile(const char *vm_name, char **fn_out)
> +{
> +
> +    return virAsprintf(fn_out, "%s/grub_bhyve-%s-device.map", BHYVE_STATE_DIR,
> +                       vm_name);
> +}
> +
>  int
>  virBhyveProcessStart(virConnectPtr conn,
>                       bhyveConnPtr driver,
> @@ -95,6 +103,8 @@ virBhyveProcessStart(virConnectPtr conn,
>                       virDomainRunningReason reason,
>                       unsigned int flags)
>  {
> +    char *devmap_file = NULL;
> +    char *devicemap = NULL;
>      char *logfile = NULL;
>      int logfd = -1;
>      off_t pos = -1;
> @@ -102,7 +112,7 @@ virBhyveProcessStart(virConnectPtr conn,
>      virCommandPtr cmd = NULL;
>      virCommandPtr load_cmd = NULL;
>      bhyveConnPtr privconn = conn->privateData;
> -    int ret = -1;
> +    int ret = -1, rc;
>
>      if (virAsprintf(&logfile, "%s/%s.log",
>                      BHYVE_LOG_DIR, vm->def->name) < 0)
> @@ -151,11 +161,26 @@ virBhyveProcessStart(virConnectPtr conn,
>      /* Now bhyve command is constructed, meaning the
>       * domain is ready to be started, so we can build
>       * and execute bhyveload command */
> -    if (!(load_cmd = virBhyveProcessBuildLoadCmd(conn, vm->def)))
> +    rc = virBhyveFormatDevMapFile(vm->def->name, &devmap_file);
> +    if (rc)
> +        goto cleanup;
> +
> +    if (!(load_cmd = virBhyveProcessBuildLoadCmd(conn, vm->def, devmap_file,
> +                                                 &devicemap)))
>          goto cleanup;
>      virCommandSetOutputFD(load_cmd, &logfd);
>      virCommandSetErrorFD(load_cmd, &logfd);
>
> +    if (devicemap != NULL) {
> +        rc = virFileWriteStr(devmap_file, devicemap, 0644);
> +        if (rc) {
> +            virReportSystemError(errno,
> +                                 _("Cannot write device.map '%s'"),
> +                                 devmap_file);
> +            goto cleanup;
> +        }
> +    }
> +
>      /* Log generated command line */
>      virCommandWriteArgLog(load_cmd, logfd);
>      if ((pos = lseek(logfd, 0, SEEK_END)) < 0)
> @@ -193,6 +218,15 @@ virBhyveProcessStart(virConnectPtr conn,
>      ret = 0;
>
>   cleanup:
> +    if (devicemap != NULL) {
> +        rc = unlink(devmap_file);
> +        if (rc < 0 && errno != ENOENT)
> +            virReportSystemError(errno, _("cannot unlink file '%s'"),
> +                                 devmap_file);
> +        VIR_FREE(devicemap);
> +    }
> +    VIR_FREE(devmap_file);
> +
>      if (ret < 0) {
>          int exitstatus; /* Needed to avoid logging non-zero status */
>          virCommandPtr destroy_cmd;
> --
> 1.9.3
>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]