On 08.11.2014 17:48, 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 (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/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));


One of the things I'm worried about is, if the boot args are not specified we properly add memory configured in domain XML.

+
+        /* Image path */
+        virCommandAddArg(cmd, "-d");
+        virCommandAddArg(cmd, virDomainDiskGetSource(disk));
+
+        /* VM name */
+        virCommandAddArg(cmd, def->name);
+    } else {
+        VIR_DEBUG("bhyveload with arguments");
+        virAppendBootloaderArgs(cmd, def);
+    }
+

However, IIUC the memory amount can be overridden with boot args. Is that expected?

+    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));
+        }

Can we utilize <boot order='X'/> attribute here?

+    }
+
+    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;
  }

Otherwise looking good.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to