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).
>
> 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 the HDD boot and it seems to work.
I've tried to boot from CD and had a problem with that. It generates a
command like that:
/usr/local/sbin/grub-bhyve -v --root cd --device-map
/usr/local/var/run/libvirt/bhyve/grub-bhyve-device.map_63694 --memory
512 linux3
and the error is:
Could not create VM linux3
Error in initializing VM
Map file looks this way:
(hd0) /home/novel/linux.img
(cd) /home/novel/ubuntu-14.04-server-amd64.iso
Any idea what could be wrong with that?
> Sponsored by: EMC / Isilon storage division
>
> Signed-off-by: Conrad Meyer <conrad.meyer@xxxxxxxxxx>
> ---
> docs/drvbhyve.html.in | 28 ++++++-
> docs/formatdomain.html.in | 4 +-
> po/libvirt.pot | 4 +
> src/bhyve/bhyve_command.c | 204 ++++++++++++++++++++++++++++++++++++++++++----
> src/bhyve/bhyve_driver.c | 5 ++
> src/bhyve/bhyve_process.c | 5 ++
> src/bhyve/bhyve_utils.h | 1 +
> 7 files changed, 228 insertions(+), 23 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://root@xxxxxxxxxxx/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>
> <domain type='bhyve'>
> - <name>bhyve</name>
> - <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> + <name>bhyve</name>
> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid>
> <memory>219136</memory>
> <currentMemory>219136</currentMemory>
> <vcpu>1</vcpu>
> @@ -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>
> + ...
> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader>
> + <bootloader_args>...</bootloader_args>
> + ...
> +</pre>
> +
I think it would be also great to add a complete domain XML for Linux
guest into 'Example guest domain XML configurations' section.
> +<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..fcaf077 100644
> --- a/src/bhyve/bhyve_command.c
> +++ b/src/bhyve/bhyve_command.c
> @@ -26,6 +26,7 @@
> #include <net/if_tap.h>
>
> #include "bhyve_command.h"
> +#include "datatypes.h"
> #include "viralloc.h"
> #include "virfile.h"
> #include "virstring.h"
> @@ -294,51 +295,218 @@ virBhyveProcessBuildDestroyCmd(bhyveConnPtr driver ATTRIBUTE_UNUSED,
> return cmd;
> }
>
> -virCommandPtr
> -virBhyveProcessBuildLoadCmd(virConnectPtr conn,
> - virDomainDefPtr def)
> +static virCommandPtr
> +virBhyveProcessBuildBhyveloadCmd(virDomainDefPtr def, virDomainDiskDefPtr disk)
> {
> virCommandPtr cmd;
> - virDomainDiskDefPtr disk;
> + char **blargs;
>
> - 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("%s: bhyveload with default arguments", __func__);
Adding __func__ here is not needed because it'll be included in the log
message as well, e.g.:
2014-10-26 10:16:28.285+0000: 34498181120: info :
virBhyveProcessBuildGrubbhyveCmd:410 : virBhyveProcessBuildGrubbhyveCmd:
Picking /home/novel/linux.img as HDD
(Yes, it's a different log entry, but idea is the same).
> +
> + /* 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 {
> + /* XXX: Handle quoted? */
> + blargs = virStringSplit(def->os.bootloaderArgs, " ", 0);
> + virCommandAddArgSet(cmd, (const char * const *)blargs);
> + virStringFreeList(blargs);
As this block is used two times without changes, I'm wondering if it's
worth to move it out into a function as well or is it small enough for
that...
> + }
> +
> + return cmd;
> +}
> +
> +static virCommandPtr
> +virBhyveProcessBuildCustomLoaderCmd(virDomainDefPtr def)
> +{
> + virCommandPtr cmd;
> + char **blargs;
> +
> + if (def->os.bootloaderArgs == NULL) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Custom loader requires explicit %s configuration"),
> + "bootloader_args");
> return NULL;
> }
>
> - disk = def->disks[0];
> + cmd = virCommandNew(def->os.bootloader);
> +
> + VIR_DEBUG("%s: custom loader '%s' with arguments", __func__,
> + def->os.bootloader);
Ditto wrt __func__.
> +
> + /* XXX: Handle quoted? */
> + blargs = virStringSplit(def->os.bootloaderArgs, " ", 0);
> + virCommandAddArgSet(cmd, (const char * const *)blargs);
> + virStringFreeList(blargs);
> +
> + 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)
> +{
> + bhyveConnPtr privconn = conn->privateData;
> + virDomainDiskDefPtr disk, cd;
> + virCommandPtr cmd;
> + size_t i;
> + FILE *f;
> + int rc;
> +
> + if (def->os.bootloaderArgs != NULL)
> + return virBhyveProcessBuildCustomLoaderCmd(def);
> +
> + /* 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("%s: Picking %s as boot CD", __func__,
Ditto wrt __func__.
> + virDomainDiskGetSource(cd));
> + }
> +
> + if (disk == NULL &&
> + def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> + disk = def->disks[i];
> + VIR_INFO("%s: Picking %s as HDD", __func__,
> + virDomainDiskGetSource(disk));
Ditto wrt __func__.
Wondering if it would make sense to break the loop if cd != NULL and
disk != NULL after the iteration as we pick the first match anyway?
> + }
> + }
> +
> + cmd = virCommandNew(def->os.bootloader);
> +
> + VIR_DEBUG("%s: grub-bhyve with default arguments", __func__);
Ditto wrt __func__.
> +
> + /*
> + * XXX Default grub-bhyve has some minor caveats, but MAY work for some
> + * typical configurations. In particular:
> + *
> + * - For HDD boot, assumes a GRUB install on hd0,msdos1
> + */
> +
> + VIR_FREE(privconn->grub_devicesmap_file);
> + rc = virAsprintf(&privconn->grub_devicesmap_file,
> + "%s/grub-bhyve-device.map_%ju", BHYVE_STATE_DIR,
> + (uintmax_t)getpid());
Could you please elaborate on deciding to store this filename in
privconn? In this case we need to make sure that it's not a subject for
races. Probably other option would be to make it per-domain based, as
it's a per-domain resource anyway?
> + if (rc < 0)
> + goto error;
> +
> + f = fopen(privconn->grub_devicesmap_file, "wb");
> + if (f == NULL) {
> + virReportSystemError(errno, _("Failed to open '%s'"),
> + privconn->grub_devicesmap_file);
> + goto error;
> + }
> +
> + /* Grub device.map (just for boot) */
> + if (disk)
> + fprintf(f, "(hd0) %s\n", virDomainDiskGetSource(disk));
> +
> + if (cd != NULL) {
> + fprintf(f, "(cd) %s\n", virDomainDiskGetSource(cd));
> +
> + VIR_WARN("Trying to boot cd with grub-bhyve. If this is "
> + "not what you wanted, specify <bootloader_args>.");
IIRC, the common style is not to end messages with a period.
> + virCommandAddArg(cmd, "--root");
> + virCommandAddArg(cmd, "cd");
> + } else {
> + VIR_WARN("Trying to boot hd0,msdos1 with grub-bhyve. If this is "
> + "not what you wanted, specify <bootloader_args>.");
Dittro wrt period.
> + virCommandAddArg(cmd, "--root");
> + virCommandAddArg(cmd, "hd0,msdos1");
> + }
> +
> + if (VIR_FCLOSE(f) < 0) {
> + virReportSystemError(errno, "%s", _("failed to close file"));
> + goto error;
> + }
> +
> + virCommandAddArg(cmd, "--device-map");
> + virCommandAddArg(cmd, privconn->grub_devicesmap_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;
> +
> + error:
> + virCommandFree(cmd);
> + return NULL;
> +}
> +
> +virCommandPtr
> +virBhyveProcessBuildLoadCmd(virConnectPtr conn,
> + virDomainDefPtr def)
> +{
> + 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;
> +
> + if (def->ndisks > 1) {
> + VIR_WARN("%s: drvbhyve attempting to boot from device %s of multiple-"
> + "device configuration", __func__,
> + virDomainDiskGetSource(disk));
Dittro wrt __func__ and formatting is one space off.
> + }
> +
> + return virBhyveProcessBuildBhyveloadCmd(def, disk);
> + } else if (strstr(def->os.bootloader, "grub-bhyve") != NULL) {
> + return virBhyveProcessBuildGrubbhyveCmd(def, conn);
> + } else {
> + return virBhyveProcessBuildCustomLoaderCmd(def);
> + }
> }
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index eb0d455..5642f85 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -1128,6 +1128,11 @@ bhyveStateCleanup(void)
> if (bhyve_driver == NULL)
> return -1;
>
> + if (bhyve_driver->grub_devicesmap_file != NULL) {
> + ignore_value(unlink(bhyve_driver->grub_devicesmap_file));
> + VIR_FREE(bhyve_driver->grub_devicesmap_file);
> + }
> +
> virObjectUnref(bhyve_driver->domains);
> virObjectUnref(bhyve_driver->caps);
> virObjectUnref(bhyve_driver->xmlopt);
> diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
> index 0bbe388..3689a6a 100644
> --- a/src/bhyve/bhyve_process.c
> +++ b/src/bhyve/bhyve_process.c
> @@ -266,6 +266,11 @@ virBhyveProcessStop(bhyveConnPtr driver,
> virPidFileDelete(BHYVE_STATE_DIR, vm->def->name);
> virDomainDeleteConfig(BHYVE_STATE_DIR, NULL, vm);
>
> + if (driver->grub_devicesmap_file != NULL) {
> + ignore_value(unlink(driver->grub_devicesmap_file));
> + VIR_FREE(driver->grub_devicesmap_file);
> + }
> +
> return ret;
> }
>
> diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h
> index 848f9a1..c9c4a2c 100644
> --- a/src/bhyve/bhyve_utils.h
> +++ b/src/bhyve/bhyve_utils.h
> @@ -40,6 +40,7 @@ struct _bhyveConn {
> virCapsPtr caps;
> virDomainXMLOptionPtr xmlopt;
> char *pidfile;
> + char *grub_devicesmap_file;
> virSysinfoDefPtr hostsysinfo;
>
> virObjectEventStatePtr domainEventState;
> --
> 1.9.3
>
Roman Bogorodskiy
Attachment:
pgpFiHUjhJTI2.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list