Date: Fri, 24 Oct 2014 08:58:45 +0400 From: Roman Bogorodskiy <bogorodskiy@gmail.com> To: Conrad Meyer <cse.cem@gmail.com> Cc: libvir-list@redhat.com, Conrad Meyer <conrad.meyer@isilon.com>, freebsd-virtualization@freebsd.org Subject: Re: [PATCHv2 1/2] bhyve: Support /domain/bootloader configuration for non-FreeBSD guests. Message-ID: <20141024045844.GA12422@dev.san.ru> In-Reply-To: <1414106923-32313-2-git-send-email-cse.cem@gmail.com> References: <1414106923-32313-1-git-send-email-cse.cem@gmail.com> <1414106923-32313-2-git-send-email-cse.cem@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Conrad Meyer wrote:
> We still default to bhyveloader(1) if no explicit bootloader
> configuration is supplied in the domain.
>=20
> 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.
>=20
> 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.meyer@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(-)
>=20
> 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@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 devel=
opment. So it supports
> -only limited number of features bhyve provides. All the supported featur=
es could be found
> -in this sample domain XML.
> +only limited number of features bhyve provides.
> </p>
> =20
> <p>
> @@ -50,8 +49,8 @@ up to 31 PCI devices.
> =20
> <pre>
> <domain type=3D'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=3D"storage.html">Storage documentation</a=
> for more details on storage
> management.</p>
> =20
> +<h3><a name=3D"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 partit=
ion of
> +the disk image.</p>
> +
> +<pre>
> + ...
> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader>
> + <bootloader_args>...</bootloader_args>
> + ...
> +</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 incorrect=
ly.</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>
> =20
> <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 ""
> =20
> +#: 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;
> =20
> if (def->ndisks < 1) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -326,19 +327,105 @@ virBhyveProcessBuildLoadCmd(virConnectPtr conn,
> return NULL;
> }
> =20
> - cmd =3D virCommandNew(BHYVELOAD);
> + if (def->os.bootloader =3D=3D NULL) {
> + bhyveload =3D true;
> + grub_bhyve =3D false;
> + cmd =3D virCommandNew(BHYVELOAD);
> + } else {
> + bhyveload =3D false;
> + if (strstr(def->os.bootloader, "grub-bhyve") =3D=3D 0)
> + grub_bhyve =3D true;
> + cmd =3D virCommandNew(def->os.bootloader);
> + }
> =20
> - /* Memory */
> - virCommandAddArg(cmd, "-m");
> - virCommandAddArgFormat(cmd, "%llu",
> - VIR_DIV_UP(def->mem.max_balloon, 1024));
> + if (bhyveload && def->os.bootloaderArgs =3D=3D 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 =3D=3D NULL) {
> + char tmpmapfile[128] =3D "/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
> + */
> =20
> - /* Image path */
> - virCommandAddArg(cmd, "-d");
> - virCommandAddArg(cmd, virDomainDiskGetSource(disk));
> + /* XXX cleanup this file. */
> + fd =3D 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 =3D VIR_FDOPEN(fd, "wb+");
> + if (f =3D=3D 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 fil=
e"));
> + goto error;
> + }
> =20
> - /* 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 =3D=3D NULL) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Custom loader requires explicit %s configurati=
on"),
> + "bootloader_args");
> + goto error;
> + } else {
> + char **blargs, **arg;
> +
> + VIR_DEBUG("%s: custom loader '%s' with arguments", __func__,
> + def->os.bootloader);
> +
> + /* XXX: Handle quoted? */
> + blargs =3D virStringSplit(def->os.bootloaderArgs, " ", 0);
> + for (arg =3D blargs; *arg; arg++)
> + virCommandAddArg(cmd, *arg);
> + virStringFreeList(blargs);
> + }
> =20
> return cmd;
> +
> +error:
> + virCommandFree(cmd);
> + return NULL;
> }
> --=20
> 1.9.3
>=20
> _______________________________________________
> freebsd-virtualization@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
> To unsubscribe, send any mail to "freebsd-virtualization-unsubscribe@free=
bsd.org"
Thanks,
Roman Bogorodskiy
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141024045844.GA12422>