[libvirt] [PATCHv8 1/7] bhyve: Support /domain/bootloader configuration for non-FreeBSD guests.
Michal Privoznik
mprivozn at redhat.com
Tue Nov 11 14:52:42 UTC 2014
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
More information about the libvir-list
mailing list