[libvirt] [PATCHv8 0/7] Add non-FreeBSD guest support to Bhyve driver.

Drvbhyve hardcodes bhyveload(8) as the host bootloader for guests. bhyveload(8) loader only supports FreeBSD guests. This patch series adds <bootloader> and <bootloader_args> handling to bhyve_command, so libvirt can boot non-FreeBSD guests in Bhyve. Additionally, support for grub-bhyve(1)'s --cons-dev argument is added so that interactive GRUB menus can be manipulated with the domain-configured serial device. See patch logs for further details. Thanks, Conrad Changes in v8: - Fix typo in virBhyveProcessStart that prevented booting bhyve VMs. Conrad Meyer (7): bhyve: Support /domain/bootloader configuration for non-FreeBSD guests. bhyvexml2argv: Add loader argv tests. domaincommon.rng: Add 'bootloader' to os=hvm schema for Bhyve bhyvexml2argv: Add tests for domain-configured bootloader, args bhyve: Probe grub-bhyve for --cons-dev capability bhyve: Add console support for grub-bhyve bootloader bhyvexml2argv: Add test for grub console support docs/drvbhyve.html.in | 100 ++++++++++- docs/formatdomain.html.in | 4 +- docs/schemas/domaincommon.rng | 17 +- src/bhyve/bhyve_capabilities.c | 37 ++++ src/bhyve/bhyve_capabilities.h | 3 + src/bhyve/bhyve_command.c | 189 +++++++++++++++++++-- src/bhyve/bhyve_command.h | 5 +- src/bhyve/bhyve_driver.c | 16 +- src/bhyve/bhyve_driver.h | 2 + src/bhyve/bhyve_process.c | 38 ++++- src/bhyve/bhyve_utils.h | 2 + .../bhyvexml2argv-acpiapic.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs | 1 + .../bhyvexml2argv-bhyveload-explicitargs.args | 3 + .../bhyvexml2argv-bhyveload-explicitargs.ldargs | 1 + .../bhyvexml2argv-bhyveload-explicitargs.xml | 23 +++ .../bhyvexml2argvdata/bhyvexml2argv-console.ldargs | 1 + .../bhyvexml2argv-custom-loader.args | 3 + .../bhyvexml2argv-custom-loader.ldargs | 1 + .../bhyvexml2argv-custom-loader.xml | 24 +++ .../bhyvexml2argv-disk-cdrom-grub.args | 3 + .../bhyvexml2argv-disk-cdrom-grub.devmap | 1 + .../bhyvexml2argv-disk-cdrom-grub.ldargs | 2 + .../bhyvexml2argv-disk-cdrom-grub.xml | 23 +++ .../bhyvexml2argv-disk-cdrom.ldargs | 1 + .../bhyvexml2argv-disk-virtio.ldargs | 1 + .../bhyvexml2argv-grub-defaults.args | 3 + .../bhyvexml2argv-grub-defaults.devmap | 1 + .../bhyvexml2argv-grub-defaults.ldargs | 2 + .../bhyvexml2argv-grub-defaults.xml | 23 +++ .../bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs | 1 + .../bhyvexml2argv-serial-grub-nocons.args | 4 + .../bhyvexml2argv-serial-grub-nocons.devmap | 1 + .../bhyvexml2argv-serial-grub-nocons.ldargs | 2 + .../bhyvexml2argv-serial-grub-nocons.xml | 26 +++ .../bhyvexml2argv-serial-grub.args | 4 + .../bhyvexml2argv-serial-grub.devmap | 1 + .../bhyvexml2argv-serial-grub.ldargs | 2 + .../bhyvexml2argv-serial-grub.xml | 26 +++ .../bhyvexml2argvdata/bhyvexml2argv-serial.ldargs | 1 + tests/bhyvexml2argvtest.c | 71 +++++++- 41 files changed, 631 insertions(+), 39 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs -- 1.9.3

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> <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> @@ -76,6 +86,7 @@ up to 31 PCI devices. <driver name='file' type='raw'/> <source file='/path/to/cdrom.iso'/> <target dev='hdc' bus='sata'/> + <readonly/> </disk> <interface type='bridge'> <model type='virtio'/> @@ -85,6 +96,53 @@ up to 31 PCI devices. </domain> </pre> +<p>(The <disk> 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 <bootloader>. +</p> + +<pre> +<domain type='bhyve'> + <name>linux_guest</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>131072</memory> + <currentMemory>131072</currentMemory> + <vcpu>1</vcpu> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> + <os> + <type>hvm</type> + </os> + <features> + <apic/> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='file' type='raw'/> + <source file='/path/to/guest_hdd.img'/> + <target dev='hda' bus='sata'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='file' type='raw'/> + <source file='/path/to/cdrom.iso'/> + <target dev='hdc' bus='sata'/> + <readonly/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + </interface> + </devices> +</domain> +</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> + ... + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> + <bootloader_args>...</bootloader_args> + ... +</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 7196e75..45af8cc 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..21f8331 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 < 0) + 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

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

On Tue, Nov 11, 2014 at 9:52 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 08.11.2014 17:48, Conrad Meyer wrote:
+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?
Yes. If you use bootloader_args, you get exactly what you ask for and no more.
+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?
This has come up before and the answer is yes, but I'd prefer to do so in a later patch series; this one is already growing unwieldy.
+ } + + 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
Thanks! Conrad

On 11.11.2014 16:17, Conrad Meyer wrote:
On Tue, Nov 11, 2014 at 9:52 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 08.11.2014 17:48, Conrad Meyer wrote:
+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?
Yes. If you use bootloader_args, you get exactly what you ask for and no more.
Well, if one is modifying XML to change booloader_args already he can change the memory amount there too. What I'm trying to say is, we should add '-m def->mem.max_balloon' unconditionally. Or do you intend to give users so much freedom? I worried it can bite us later when libvirt fails to see the real amount of memory that domain has.
+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?
This has come up before and the answer is yes, but I'd prefer to do so in a later patch series; this one is already growing unwieldy.
Agreed. This can be addressed later, when needed. Michal

On Tue, Nov 11, 2014 at 10:40 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 11.11.2014 16:17, Conrad Meyer wrote:
On Tue, Nov 11, 2014 at 9:52 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
One of the things I'm worried about is, if the boot args are not specified we properly add memory configured in domain XML.
However, IIUC the memory amount can be overridden with boot args. Is that expected?
Yes. If you use bootloader_args, you get exactly what you ask for and no more.
Well, if one is modifying XML to change booloader_args already he can change the memory amount there too. What I'm trying to say is, we should add '-m def->mem.max_balloon' unconditionally. Or do you intend to give users so much freedom? I worried it can bite us later when libvirt fails to see the real amount of memory that domain has.
I think also bhyve(8) itself will be unhappy. The man page specifies: """ -m size[K|k|M|m|G|g|T|t] Guest physical memory size in bytes. This must be the same size that was given to bhyveload(8). """ However, I think the messaging around bootloader_args for bhyve / bhyveload is and should remain: "if you need this, you're on your own and need to specify *everything* manually." We expect most users to not need <bootloader> for bhyve at all, especially in the medium-term future, and even fewer users to need <bootloader_args>. IMO, as a user it is more surprising to get additional arguments prepended to the arguments I have specified than to have them passed through unmodified. And we would have to prepend or otherwise shove the -m flag in the middle somewhere. I think it is probably too much magic.
+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?
This has come up before and the answer is yes, but I'd prefer to do so in a later patch series; this one is already growing unwieldy.
Agreed. This can be addressed later, when needed.
Michal
Thanks, Conrad

On 11.11.2014 17:12, Conrad Meyer wrote:
On Tue, Nov 11, 2014 at 10:40 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 11.11.2014 16:17, Conrad Meyer wrote:
On Tue, Nov 11, 2014 at 9:52 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
One of the things I'm worried about is, if the boot args are not specified we properly add memory configured in domain XML.
However, IIUC the memory amount can be overridden with boot args. Is that expected?
Yes. If you use bootloader_args, you get exactly what you ask for and no more.
Well, if one is modifying XML to change booloader_args already he can change the memory amount there too. What I'm trying to say is, we should add '-m def->mem.max_balloon' unconditionally. Or do you intend to give users so much freedom? I worried it can bite us later when libvirt fails to see the real amount of memory that domain has.
I think also bhyve(8) itself will be unhappy. The man page specifies:
""" -m size[K|k|M|m|G|g|T|t] Guest physical memory size in bytes. This must be the same size that was given to bhyveload(8). """
However, I think the messaging around bootloader_args for bhyve / bhyveload is and should remain: "if you need this, you're on your own and need to specify *everything* manually." We expect most users to not need <bootloader> for bhyve at all, especially in the medium-term future, and even fewer users to need <bootloader_args>.
IMO, as a user it is more surprising to get additional arguments prepended to the arguments I have specified than to have them passed through unmodified. And we would have to prepend or otherwise shove the -m flag in the middle somewhere. I think it is probably too much magic.
Okay, you've persuaded me that This patch is good as is. ACK then. Michal

--- .../bhyvexml2argv-acpiapic.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs | 1 + .../bhyvexml2argvdata/bhyvexml2argv-console.ldargs | 1 + .../bhyvexml2argv-disk-cdrom.ldargs | 1 + .../bhyvexml2argv-disk-virtio.ldargs | 1 + .../bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs | 1 + .../bhyvexml2argvdata/bhyvexml2argv-serial.ldargs | 1 + tests/bhyvexml2argvtest.c | 60 +++++++++++++++++++--- 8 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs new file mode 100644 index 0000000..215d65f --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs new file mode 100644 index 0000000..215d65f --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs new file mode 100644 index 0000000..215d65f --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs new file mode 100644 index 0000000..0eb3cc9 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -m 214 -d /tmp/cdrom.iso bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs new file mode 100644 index 0000000..215d65f --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs new file mode 100644 index 0000000..215d65f --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs new file mode 100644 index 0000000..215d65f --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -m 214 -d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index b9be378..3ff6696 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -15,14 +15,16 @@ static bhyveConn driver; static int testCompareXMLToArgvFiles(const char *xml, - const char *cmdline) + const char *cmdline, + const char *ldcmdline, + const char *dmcmdline) { - char *expectargv = NULL; + char *expectargv = NULL, *expectld = NULL, *expectdm = NULL; int len; - char *actualargv = NULL; + char *actualargv = NULL, *actualld = NULL, *actualdm = NULL; virDomainDefPtr vmdef = NULL; virDomainObj vm; - virCommandPtr cmd = NULL; + virCommandPtr cmd = NULL, ldcmd = NULL; virConnectPtr conn; int ret = -1; @@ -42,6 +44,16 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(actualargv = virCommandToString(cmd))) goto out; + if (!(ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "<device.map>", + &actualdm))) + goto out; + + if (actualdm != NULL) + virTrimSpaces(actualdm, NULL); + + if (!(actualld = virCommandToString(ldcmd))) + goto out; + len = virtTestLoadFile(cmdline, &expectargv); if (len < 0) goto out; @@ -49,17 +61,49 @@ static int testCompareXMLToArgvFiles(const char *xml, if (len && expectargv[len - 1] == '\n') expectargv[len - 1] = '\0'; + len = virtTestLoadFile(ldcmdline, &expectld); + if (len < 0) + goto out; + + if (len && expectld[len - 1] == '\n') + expectld[len - 1] = '\0'; + + len = virFileReadAllQuiet(dmcmdline, 1000, &expectdm); + if (len < 0) { + if (actualdm != NULL) { + virtTestDifference(stderr, "", actualdm); + goto out; + } + } else if (len && expectdm[len - 1] == '\n') { + expectdm[len - 1] = '\0'; + } + if (STRNEQ(expectargv, actualargv)) { virtTestDifference(stderr, expectargv, actualargv); goto out; } + if (STRNEQ(expectld, actualld)) { + virtTestDifference(stderr, expectld, actualld); + goto out; + } + + if (expectdm && STRNEQ(expectdm, actualdm)) { + virtTestDifference(stderr, expectdm, actualdm); + goto out; + } + ret = 0; out: VIR_FREE(expectargv); + VIR_FREE(expectld); + VIR_FREE(expectdm); VIR_FREE(actualargv); + VIR_FREE(actualld); + VIR_FREE(actualdm); virCommandFree(cmd); + virCommandFree(ldcmd); virDomainDefFree(vmdef); return ret; } @@ -70,15 +114,19 @@ testCompareXMLToArgvHelper(const void *data) int ret = -1; const char *name = data; char *xml = NULL; - char *args = NULL; + char *args = NULL, *ldargs = NULL, *dmargs = NULL; if (virAsprintf(&xml, "%s/bhyvexml2argvdata/bhyvexml2argv-%s.xml", abs_srcdir, name) < 0 || virAsprintf(&args, "%s/bhyvexml2argvdata/bhyvexml2argv-%s.args", + abs_srcdir, name) < 0 || + virAsprintf(&ldargs, "%s/bhyvexml2argvdata/bhyvexml2argv-%s.ldargs", + abs_srcdir, name) < 0 || + virAsprintf(&dmargs, "%s/bhyvexml2argvdata/bhyvexml2argv-%s.devmap", abs_srcdir, name) < 0) goto cleanup; - ret = testCompareXMLToArgvFiles(xml, args); + ret = testCompareXMLToArgvFiles(xml, args, ldargs, dmargs); cleanup: VIR_FREE(xml); -- 1.9.3

Additionally, make the <bootloader> tag optional (for bhyveload with custom arguments) (also, matches the actual parser). --- docs/schemas/domaincommon.rng | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 20d81ae..87ba888 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -234,6 +234,9 @@ </choice> </define> <define name="oshvm"> + <optional> + <ref name="bootloader"/> + </optional> <element name="os"> <ref name="ostypehvm"/> <interleave> @@ -1054,12 +1057,14 @@ --> <define name="bootloader"> <interleave> - <element name="bootloader"> - <choice> - <ref name="absFilePath"/> - <empty/> - </choice> - </element> + <optional> + <element name="bootloader"> + <choice> + <ref name="absFilePath"/> + <empty/> + </choice> + </element> + </optional> <optional> <element name="bootloader_args"> <text/> -- 1.9.3

--- .../bhyvexml2argv-bhyveload-explicitargs.args | 3 +++ .../bhyvexml2argv-bhyveload-explicitargs.ldargs | 1 + .../bhyvexml2argv-bhyveload-explicitargs.xml | 23 +++++++++++++++++++++ .../bhyvexml2argv-custom-loader.args | 3 +++ .../bhyvexml2argv-custom-loader.ldargs | 1 + .../bhyvexml2argv-custom-loader.xml | 24 ++++++++++++++++++++++ .../bhyvexml2argv-disk-cdrom-grub.args | 3 +++ .../bhyvexml2argv-disk-cdrom-grub.devmap | 1 + .../bhyvexml2argv-disk-cdrom-grub.ldargs | 2 ++ .../bhyvexml2argv-disk-cdrom-grub.xml | 23 +++++++++++++++++++++ .../bhyvexml2argv-grub-defaults.args | 3 +++ .../bhyvexml2argv-grub-defaults.devmap | 1 + .../bhyvexml2argv-grub-defaults.ldargs | 2 ++ .../bhyvexml2argv-grub-defaults.xml | 23 +++++++++++++++++++++ tests/bhyvexml2argvtest.c | 4 ++++ 15 files changed, 117 insertions(+) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.xml diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args new file mode 100644 index 0000000..4122e62 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args @@ -0,0 +1,3 @@ +/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs new file mode 100644 index 0000000..a3f58f0 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs @@ -0,0 +1 @@ +/usr/sbin/bhyveload -X -Y -Z diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml new file mode 100644 index 0000000..65823cd --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <bootloader_args>-X -Y -Z</bootloader_args> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args new file mode 100644 index 0000000..4122e62 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args @@ -0,0 +1,3 @@ +/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs new file mode 100644 index 0000000..100e163 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs @@ -0,0 +1 @@ +/fizz_buzz_bazz -X -Y -Z diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml new file mode 100644 index 0000000..e9e0d5e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml @@ -0,0 +1,24 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <bootloader>/fizz_buzz_bazz</bootloader> + <bootloader_args>-X -Y -Z</bootloader_args> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args new file mode 100644 index 0000000..eb38969 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args @@ -0,0 +1,3 @@ +/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-cd,/tmp/cdrom.iso bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.devmap b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.devmap new file mode 100644 index 0000000..47d4364 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.devmap @@ -0,0 +1 @@ +(cd) /tmp/cdrom.iso diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs new file mode 100644 index 0000000..d9161ba --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs @@ -0,0 +1,2 @@ +/usr/local/sbin/grub-bhyve --root cd --device-map '<device.map>' --memory 214 \ +bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml new file mode 100644 index 0000000..00708e6 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file' device='cdrom'> + <driver name='file' type='raw'/> + <source file='/tmp/cdrom.iso'/> + <target dev='hdc' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args new file mode 100644 index 0000000..4122e62 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args @@ -0,0 +1,3 @@ +/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.devmap b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.devmap new file mode 100644 index 0000000..68c35da --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.devmap @@ -0,0 +1 @@ +(hd0) /tmp/freebsd.img diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs new file mode 100644 index 0000000..91c15ce --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs @@ -0,0 +1,2 @@ +/usr/local/sbin/grub-bhyve --root hd0,msdos1 --device-map '<device.map>' \ +--memory 214 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.xml new file mode 100644 index 0000000..27b7fd9 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 3ff6696..64e6c0d 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -160,6 +160,10 @@ mymain(void) DO_TEST("macaddr"); DO_TEST("serial"); DO_TEST("console"); + DO_TEST("grub-defaults"); + DO_TEST("bhyveload-explicitargs"); + DO_TEST("custom-loader"); + DO_TEST("disk-cdrom-grub"); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.9.3

--- src/bhyve/bhyve_capabilities.c | 37 +++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_capabilities.h | 3 +++ 2 files changed, 40 insertions(+) diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 132ce91..6e9a943 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -23,6 +23,7 @@ #include <sys/utsname.h> #include "viralloc.h" +#include "virfile.h" #include "virlog.h" #include "virstring.h" #include "cpu/cpu.h" @@ -104,3 +105,39 @@ virBhyveCapsBuild(void) virObjectUnref(caps); return NULL; } + +int +virBhyveProbeGrubCaps(unsigned *caps) +{ + char *binary, *help; + virCommandPtr cmd; + int ret, exit; + + ret = 0; + *caps = 0; + cmd = NULL; + help = NULL; + + binary = virFindFileInPath("grub-bhyve"); + if (binary == NULL) + goto out; + if (!virFileIsExecutable(binary)) + goto out; + + cmd = virCommandNew(binary); + virCommandAddArg(cmd, "--help"); + virCommandSetOutputBuffer(cmd, &help); + if (virCommandRun(cmd, &exit) < 0) { + ret = -1; + goto out; + } + + if (strstr(help, "--cons-dev") != NULL) + *caps |= BHYVE_GRUB_CAP_CONSDEV; + + out: + VIR_FREE(help); + virCommandFree(cmd); + VIR_FREE(binary); + return ret; +} diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index c52e0d0..f4ebd90 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -26,4 +26,7 @@ virCapsPtr virBhyveCapsBuild(void); +# define BHYVE_GRUB_CAP_CONSDEV 0x00000001 +int virBhyveProbeGrubCaps(unsigned *caps); + #endif -- 1.9.3

On 08.11.2014 17:48, Conrad Meyer wrote:
--- src/bhyve/bhyve_capabilities.c | 37 +++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_capabilities.h | 3 +++ 2 files changed, 40 insertions(+)
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 132ce91..6e9a943 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -23,6 +23,7 @@ #include <sys/utsname.h>
#include "viralloc.h" +#include "virfile.h" #include "virlog.h" #include "virstring.h" #include "cpu/cpu.h" @@ -104,3 +105,39 @@ virBhyveCapsBuild(void) virObjectUnref(caps); return NULL; } + +int +virBhyveProbeGrubCaps(unsigned *caps) +{ + char *binary, *help; + virCommandPtr cmd; + int ret, exit; + + ret = 0; + *caps = 0; + cmd = NULL; + help = NULL; + + binary = virFindFileInPath("grub-bhyve"); + if (binary == NULL) + goto out; + if (!virFileIsExecutable(binary)) + goto out; + + cmd = virCommandNew(binary); + virCommandAddArg(cmd, "--help"); + virCommandSetOutputBuffer(cmd, &help); + if (virCommandRun(cmd, &exit) < 0) { + ret = -1; + goto out; + } + + if (strstr(help, "--cons-dev") != NULL) + *caps |= BHYVE_GRUB_CAP_CONSDEV; + + out: + VIR_FREE(help); + virCommandFree(cmd); + VIR_FREE(binary); + return ret; +} diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index c52e0d0..f4ebd90 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -26,4 +26,7 @@
virCapsPtr virBhyveCapsBuild(void);
+# define BHYVE_GRUB_CAP_CONSDEV 0x00000001
This should be rather an enum.
+int virBhyveProbeGrubCaps(unsigned *caps); + #endif
Michal

--- src/bhyve/bhyve_capabilities.c | 37 +++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_capabilities.h | 6 ++++++ 2 files changed, 43 insertions(+) diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 132ce91..6e9a943 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -23,6 +23,7 @@ #include <sys/utsname.h> #include "viralloc.h" +#include "virfile.h" #include "virlog.h" #include "virstring.h" #include "cpu/cpu.h" @@ -104,3 +105,39 @@ virBhyveCapsBuild(void) virObjectUnref(caps); return NULL; } + +int +virBhyveProbeGrubCaps(unsigned *caps) +{ + char *binary, *help; + virCommandPtr cmd; + int ret, exit; + + ret = 0; + *caps = 0; + cmd = NULL; + help = NULL; + + binary = virFindFileInPath("grub-bhyve"); + if (binary == NULL) + goto out; + if (!virFileIsExecutable(binary)) + goto out; + + cmd = virCommandNew(binary); + virCommandAddArg(cmd, "--help"); + virCommandSetOutputBuffer(cmd, &help); + if (virCommandRun(cmd, &exit) < 0) { + ret = -1; + goto out; + } + + if (strstr(help, "--cons-dev") != NULL) + *caps |= BHYVE_GRUB_CAP_CONSDEV; + + out: + VIR_FREE(help); + virCommandFree(cmd); + VIR_FREE(binary); + return ret; +} diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index c52e0d0..a559d2a 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -26,4 +26,10 @@ virCapsPtr virBhyveCapsBuild(void); +/* These are bit flags: */ +enum { + BHYVE_GRUB_CAP_CONSDEV = 0x00000001, +}; +int virBhyveProbeGrubCaps(unsigned *caps); + #endif -- 1.9.3

On 11.11.2014 16:35, Conrad Meyer wrote:
--- src/bhyve/bhyve_capabilities.c | 37 +++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_capabilities.h | 6 ++++++ 2 files changed, 43 insertions(+)
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 132ce91..6e9a943 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -23,6 +23,7 @@ #include <sys/utsname.h>
#include "viralloc.h" +#include "virfile.h" #include "virlog.h" #include "virstring.h" #include "cpu/cpu.h" @@ -104,3 +105,39 @@ virBhyveCapsBuild(void) virObjectUnref(caps); return NULL; } + +int +virBhyveProbeGrubCaps(unsigned *caps) +{ + char *binary, *help; + virCommandPtr cmd; + int ret, exit; + + ret = 0; + *caps = 0; + cmd = NULL; + help = NULL; + + binary = virFindFileInPath("grub-bhyve"); + if (binary == NULL) + goto out; + if (!virFileIsExecutable(binary)) + goto out; + + cmd = virCommandNew(binary); + virCommandAddArg(cmd, "--help"); + virCommandSetOutputBuffer(cmd, &help); + if (virCommandRun(cmd, &exit) < 0) { + ret = -1; + goto out; + } + + if (strstr(help, "--cons-dev") != NULL) + *caps |= BHYVE_GRUB_CAP_CONSDEV; + + out: + VIR_FREE(help); + virCommandFree(cmd); + VIR_FREE(binary); + return ret; +} diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index c52e0d0..a559d2a 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -26,4 +26,10 @@
virCapsPtr virBhyveCapsBuild(void);
+/* These are bit flags: */ +enum { + BHYVE_GRUB_CAP_CONSDEV = 0x00000001, +};
I think this should be rather typedef enum {...} virBhyveGrubCapsFlags;
+int virBhyveProbeGrubCaps(unsigned *caps);
And hence s/unsigned/virBhyveGrubCapsFlags/
+ #endif
I'm fixing this and pushing. ACK.

On Wed, Nov 12, 2014 at 4:07 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 11.11.2014 16:35, Conrad Meyer wrote:
+/* These are bit flags: */ +enum { + BHYVE_GRUB_CAP_CONSDEV = 0x00000001, +};
I think this should be rather typedef enum {...} virBhyveGrubCapsFlags;
Ok.
+int virBhyveProbeGrubCaps(unsigned *caps);
And hence s/unsigned/virBhyveGrubCapsFlags/
This seems like a confusing use of an enum to me — with 2+ flags, you can return values that are no longer present in the enum as enum type.
+ #endif
I'm fixing this and pushing. ACK.
Thanks, Conrad

This enables booting interactive GRUB menus (e.g. install CDs) with libvirt-bhyve. Caveat: A terminal other than the '--console' option to 'virsh start' (e.g. 'cu -l /dev/nmdm0B -s 115200') must be used to connect to grub-bhyve because the bhyve loader path is synchronous and must occur before the VM actually starts. Changing the bhyveProcessStart logic around to accommodate '--console' for interactive loader use seems like a significant project and probably not worth it, if UEFI/BIOS support for bhyve is "coming soon." --- src/bhyve/bhyve_command.c | 18 ++++++++++++++++++ src/bhyve/bhyve_driver.c | 13 +++++++++++++ src/bhyve/bhyve_driver.h | 2 ++ src/bhyve/bhyve_utils.h | 2 ++ 4 files changed, 35 insertions(+) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 203495c..26d4797 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -25,8 +25,10 @@ #include <net/if.h> #include <net/if_tap.h> +#include "bhyve_capabilities.h" #include "bhyve_command.h" #include "bhyve_domain.h" +#include "bhyve_driver.h" #include "datatypes.h" #include "viralloc.h" #include "virfile.h" @@ -447,6 +449,22 @@ virBhyveProcessBuildGrubbhyveCmd(virDomainDefPtr def, virCommandAddArgFormat(cmd, "%llu", VIR_DIV_UP(def->mem.max_balloon, 1024)); + if ((bhyveDriverGetGrubCaps(conn) & BHYVE_GRUB_CAP_CONSDEV) != 0 && + def->nserials > 0) { + virDomainChrDefPtr chr; + + chr = def->serials[0]; + + if (chr->source.type != VIR_DOMAIN_CHR_TYPE_NMDM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only nmdm console types are supported")); + return NULL; + } + + virCommandAddArg(cmd, "--cons-dev"); + virCommandAddArg(cmd, chr->source.data.file.path); + } + /* VM name */ virCommandAddArg(cmd, def->name); diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 4aee249..3820737 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1169,6 +1169,9 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED, if (!(bhyve_driver->caps = virBhyveCapsBuild())) goto cleanup; + if (virBhyveProbeGrubCaps(&bhyve_driver->grubcaps) < 0) + goto cleanup; + if (!(bhyve_driver->xmlopt = virDomainXMLOptionNew(&virBhyveDriverDomainDefParserConfig, &virBhyveDriverPrivateDataCallbacks, NULL))) @@ -1226,6 +1229,16 @@ bhyveStateInitialize(bool priveleged ATTRIBUTE_UNUSED, return -1; } +unsigned +bhyveDriverGetGrubCaps(virConnectPtr conn) +{ + bhyveConnPtr driver = conn->privateData; + + if (driver != NULL) + return driver->grubcaps; + return 0; +} + static void bhyveStateAutoStart(void) { diff --git a/src/bhyve/bhyve_driver.h b/src/bhyve/bhyve_driver.h index b70991d..af2424a 100644 --- a/src/bhyve/bhyve_driver.h +++ b/src/bhyve/bhyve_driver.h @@ -25,4 +25,6 @@ int bhyveRegister(void); +unsigned bhyveDriverGetGrubCaps(virConnectPtr conn); + #endif /* __BHYVE_DRIVER_H__ */ diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h index 848f9a1..bbaa3a3 100644 --- a/src/bhyve/bhyve_utils.h +++ b/src/bhyve/bhyve_utils.h @@ -45,6 +45,8 @@ struct _bhyveConn { virObjectEventStatePtr domainEventState; virCloseCallbacksPtr closeCallbacks; + + unsigned grubcaps; }; typedef struct _bhyveConn bhyveConn; -- 1.9.3

--- .../bhyvexml2argv-serial-grub-nocons.args | 4 ++++ .../bhyvexml2argv-serial-grub-nocons.devmap | 1 + .../bhyvexml2argv-serial-grub-nocons.ldargs | 2 ++ .../bhyvexml2argv-serial-grub-nocons.xml | 26 ++++++++++++++++++++++ .../bhyvexml2argv-serial-grub.args | 4 ++++ .../bhyvexml2argv-serial-grub.devmap | 1 + .../bhyvexml2argv-serial-grub.ldargs | 2 ++ .../bhyvexml2argv-serial-grub.xml | 26 ++++++++++++++++++++++ tests/bhyvexml2argvtest.c | 7 ++++++ 9 files changed, 73 insertions(+) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args new file mode 100644 index 0000000..df50290 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args @@ -0,0 +1,4 @@ +/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img \ +-s 1,lpc -l com1,/dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap new file mode 100644 index 0000000..68c35da --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap @@ -0,0 +1 @@ +(hd0) /tmp/freebsd.img diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs new file mode 100644 index 0000000..91c15ce --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs @@ -0,0 +1,2 @@ +/usr/local/sbin/grub-bhyve --root hd0,msdos1 --device-map '<device.map>' \ +--memory 214 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml new file mode 100644 index 0000000..8c451f7 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <serial type='nmdm'> + <source master='/dev/nmdm0A' slave='/dev/nmdm0B'/> + </serial> + </devices> +</domain> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args new file mode 100644 index 0000000..df50290 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args @@ -0,0 +1,4 @@ +/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 2:0,ahci-hd,/tmp/freebsd.img \ +-s 1,lpc -l com1,/dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap new file mode 100644 index 0000000..68c35da --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap @@ -0,0 +1 @@ +(hd0) /tmp/freebsd.img diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs new file mode 100644 index 0000000..5645097 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs @@ -0,0 +1,2 @@ +/usr/local/sbin/grub-bhyve --root hd0,msdos1 --device-map '<device.map>' \ +--memory 214 --cons-dev /dev/nmdm0A bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml new file mode 100644 index 0000000..8c451f7 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <bootloader>/usr/local/sbin/grub-bhyve</bootloader> + <os> + <type>hvm</type> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <serial type='nmdm'> + <source master='/dev/nmdm0A' slave='/dev/nmdm0B'/> + </serial> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 64e6c0d..ec57160 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -37,6 +37,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; vm.def = vmdef; + conn->privateData = &driver; if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, false))) goto out; @@ -152,6 +153,7 @@ mymain(void) ret = -1; \ } while (0) + driver.grubcaps = BHYVE_GRUB_CAP_CONSDEV; DO_TEST("base"); DO_TEST("acpiapic"); @@ -164,6 +166,11 @@ mymain(void) DO_TEST("bhyveload-explicitargs"); DO_TEST("custom-loader"); DO_TEST("disk-cdrom-grub"); + DO_TEST("serial-grub"); + + driver.grubcaps = 0; + + DO_TEST("serial-grub-nocons"); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.9.3

On 08.11.2014 17:48, Conrad Meyer wrote:
Drvbhyve hardcodes bhyveload(8) as the host bootloader for guests. bhyveload(8) loader only supports FreeBSD guests.
This patch series adds <bootloader> and <bootloader_args> handling to bhyve_command, so libvirt can boot non-FreeBSD guests in Bhyve.
Additionally, support for grub-bhyve(1)'s --cons-dev argument is added so that interactive GRUB menus can be manipulated with the domain-configured serial device.
See patch logs for further details.
Thanks, Conrad
Changes in v8: - Fix typo in virBhyveProcessStart that prevented booting bhyve VMs.
Conrad Meyer (7): bhyve: Support /domain/bootloader configuration for non-FreeBSD guests. bhyvexml2argv: Add loader argv tests. domaincommon.rng: Add 'bootloader' to os=hvm schema for Bhyve bhyvexml2argv: Add tests for domain-configured bootloader, args bhyve: Probe grub-bhyve for --cons-dev capability bhyve: Add console support for grub-bhyve bootloader bhyvexml2argv: Add test for grub console support
docs/drvbhyve.html.in | 100 ++++++++++- docs/formatdomain.html.in | 4 +- docs/schemas/domaincommon.rng | 17 +- src/bhyve/bhyve_capabilities.c | 37 ++++ src/bhyve/bhyve_capabilities.h | 3 + src/bhyve/bhyve_command.c | 189 +++++++++++++++++++-- src/bhyve/bhyve_command.h | 5 +- src/bhyve/bhyve_driver.c | 16 +- src/bhyve/bhyve_driver.h | 2 + src/bhyve/bhyve_process.c | 38 ++++- src/bhyve/bhyve_utils.h | 2 + .../bhyvexml2argv-acpiapic.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs | 1 + .../bhyvexml2argv-bhyveload-explicitargs.args | 3 + .../bhyvexml2argv-bhyveload-explicitargs.ldargs | 1 + .../bhyvexml2argv-bhyveload-explicitargs.xml | 23 +++ .../bhyvexml2argvdata/bhyvexml2argv-console.ldargs | 1 + .../bhyvexml2argv-custom-loader.args | 3 + .../bhyvexml2argv-custom-loader.ldargs | 1 + .../bhyvexml2argv-custom-loader.xml | 24 +++ .../bhyvexml2argv-disk-cdrom-grub.args | 3 + .../bhyvexml2argv-disk-cdrom-grub.devmap | 1 + .../bhyvexml2argv-disk-cdrom-grub.ldargs | 2 + .../bhyvexml2argv-disk-cdrom-grub.xml | 23 +++ .../bhyvexml2argv-disk-cdrom.ldargs | 1 + .../bhyvexml2argv-disk-virtio.ldargs | 1 + .../bhyvexml2argv-grub-defaults.args | 3 + .../bhyvexml2argv-grub-defaults.devmap | 1 + .../bhyvexml2argv-grub-defaults.ldargs | 2 + .../bhyvexml2argv-grub-defaults.xml | 23 +++ .../bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs | 1 + .../bhyvexml2argv-serial-grub-nocons.args | 4 + .../bhyvexml2argv-serial-grub-nocons.devmap | 1 + .../bhyvexml2argv-serial-grub-nocons.ldargs | 2 + .../bhyvexml2argv-serial-grub-nocons.xml | 26 +++ .../bhyvexml2argv-serial-grub.args | 4 + .../bhyvexml2argv-serial-grub.devmap | 1 + .../bhyvexml2argv-serial-grub.ldargs | 2 + .../bhyvexml2argv-serial-grub.xml | 26 +++ .../bhyvexml2argvdata/bhyvexml2argv-serial.ldargs | 1 + tests/bhyvexml2argvtest.c | 71 +++++++- 41 files changed, 631 insertions(+), 39 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub-nocons.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.devmap create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs
Looking good. Basically ACK to the patches. BUT, please post a follow-up patches to 1/7 and 5/7 (as reply to individual patches or cover letter ideally). I don't want to make you to send another version. I'll merge the patches then. Michal

On Tue, Nov 11, 2014 at 9:52 AM, Michal Privoznik <mprivozn@redhat.com> wrote:
Looking good. Basically ACK to the patches. BUT, please post a follow-up patches to 1/7 and 5/7 (as reply to individual patches or cover letter ideally). I don't want to make you to send another version. I'll merge the patches then.
Michal
Great. I've posted a minor follow-up to 5/7; if you think the 1/7 is completely trivial, I can do the follow up there, but I'd rather fix it up separately. Thanks, Conrad

On 11.11.2014 15:52, Michal Privoznik wrote:
On 08.11.2014 17:48, Conrad Meyer wrote:
Looking good. Basically ACK to the patches. BUT, please post a follow-up patches to 1/7 and 5/7 (as reply to individual patches or cover letter ideally). I don't want to make you to send another version. I'll merge the patches then.
I've fixed the small issues in 5/7 and pushed the patched. Congratulations on your first libvirt contribution! Michal
participants (2)
-
Conrad Meyer
-
Michal Privoznik