[libvirt] [PATCH 0/6] bhyve UEFI and graphics support

Fabian Freyer (5): bhyve: virBhyveProbeCaps: BHYVE_CAP_LPC_BOOTROM bhyve: add support for booting from UEFI bhyve: test cases for UEFI bhyvexml2argvtest bhyve: enumerate UEFI firmwares bhyve: add video support Roman Bogorodskiy (1): bhyve: test cases for VNC docs/formatdomain.html.in | 3 +- docs/schemas/domaincommon.rng | 1 + po/POTFILES.in | 1 + src/bhyve/bhyve_capabilities.c | 93 ++++++++++++++++ src/bhyve/bhyve_capabilities.h | 2 + src/bhyve/bhyve_command.c | 130 +++++++++++++++++++++- src/bhyve/bhyve_device.c | 11 ++ src/bhyve/bhyve_driver.c | 27 ++++- src/bhyve/bhyve_process.c | 55 ++++----- src/conf/domain_conf.c | 5 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 9 +- src/qemu/qemu_domain_address.c | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args | 11 ++ tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml | 23 ++++ tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 12 ++ tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml | 26 +++++ tests/bhyvexml2argvtest.c | 18 ++- tests/domaincapsschemadata/full.xml | 1 + 21 files changed, 394 insertions(+), 38 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml -- 2.11.0

From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de> Implement the BHACE_CAP_LPC_BOOTROM capability by checking the stderr output of 'bhyve -l bootrom'. If the bootrom option is unsupported, this will contain the following output: bhyve: invalid lpc device configuration 'bootrom' On newer bhyve versions that do support specifying a bootrom image, the standard help will be printed. --- src/bhyve/bhyve_capabilities.c | 25 +++++++++++++++++++++++++ src/bhyve/bhyve_capabilities.h | 1 + 2 files changed, 26 insertions(+) diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 1dc0593af..13b4835a8 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -239,6 +239,28 @@ bhyveProbeCapsNetE1000(unsigned int *caps, char *binary) return ret; } +static int +bhyveProbeCapsLPC_Bootrom(unsigned int *caps, char *binary) +{ + char *error; + virCommandPtr cmd = NULL; + int ret = -1, exit; + + cmd = virCommandNew(binary); + virCommandAddArgList(cmd, "-l", "bootrom", NULL); + virCommandSetErrorBuffer(cmd, &error); + if (virCommandRun(cmd, &exit) < 0) + goto cleanup; + + if (strstr(error, "bhyve: invalid lpc device configuration 'bootrom'") == NULL) + *caps |= BHYVE_CAP_LPC_BOOTROM; + + cleanup: + VIR_FREE(error); + virCommandFree(cmd); + return ret; +} + int virBhyveProbeCaps(unsigned int *caps) { @@ -260,6 +282,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsNetE1000(caps, binary))) goto out; + if ((ret = bhyveProbeCapsLPC_Bootrom(caps, binary))) + goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 690feadb8..746c77181 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -40,6 +40,7 @@ typedef enum { BHYVE_CAP_RTC_UTC = 1 << 0, BHYVE_CAP_AHCI32SLOT = 1 << 1, BHYVE_CAP_NET_E1000 = 1 << 2, + BHYVE_CAP_LPC_BOOTROM = 1 << 3, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); -- 2.11.0

On 02/12/2017 04:12 PM, Roman Bogorodskiy wrote:
From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Implement the BHACE_CAP_LPC_BOOTROM capability by checking the stderr output of 'bhyve -l bootrom'. If the bootrom option is unsupported, this will contain the following output:
bhyve: invalid lpc device configuration 'bootrom'
On newer bhyve versions that do support specifying a bootrom image, the standard help will be printed. --- src/bhyve/bhyve_capabilities.c | 25 +++++++++++++++++++++++++ src/bhyve/bhyve_capabilities.h | 1 + 2 files changed, 26 insertions(+)
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 1dc0593af..13b4835a8 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -239,6 +239,28 @@ bhyveProbeCapsNetE1000(unsigned int *caps, char *binary) return ret; }
+static int +bhyveProbeCapsLPC_Bootrom(unsigned int *caps, char *binary) +{ + char *error; + virCommandPtr cmd = NULL; + int ret = -1, exit; + + cmd = virCommandNew(binary); + virCommandAddArgList(cmd, "-l", "bootrom", NULL); + virCommandSetErrorBuffer(cmd, &error); + if (virCommandRun(cmd, &exit) < 0) + goto cleanup; + + if (strstr(error, "bhyve: invalid lpc device configuration 'bootrom'") == NULL) + *caps |= BHYVE_CAP_LPC_BOOTROM; + + cleanup: + VIR_FREE(error); + virCommandFree(cmd); + return ret; +} + int virBhyveProbeCaps(unsigned int *caps) { @@ -260,6 +282,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsNetE1000(caps, binary))) goto out;
+ if ((ret = bhyveProbeCapsLPC_Bootrom(caps, binary))) + goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 690feadb8..746c77181 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -40,6 +40,7 @@ typedef enum { BHYVE_CAP_RTC_UTC = 1 << 0, BHYVE_CAP_AHCI32SLOT = 1 << 1, BHYVE_CAP_NET_E1000 = 1 << 2, + BHYVE_CAP_LPC_BOOTROM = 1 << 3, } virBhyveCapsFlags;
int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps);
ACK, but we really need a better way to detect capabilites. For instance now, bhyve binary is executed 4 times just to find out whether it supports 4 capabilities. That's just madness. Maybe we can get in touch with bhyve developers and ask them? Maybe it could have a monitor just like qemu has, or something. Michal

On 08.03.2017 18:19, Michal Privoznik wrote:
ACK, but we really need a better way to detect capabilites. For instance now, bhyve binary is executed 4 times just to find out whether it supports 4 capabilities. That's just madness. Maybe we can get in touch with bhyve developers and ask them? Maybe it could have a monitor just like qemu has, or something. There's a patch[1] for it, but it would need to be moved along. Unfortunately, we would still need to somehow fall back to this for backwards compatibility...
Fabian [1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210111 -- A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail?

Fabian Freyer wrote:
On 08.03.2017 18:19, Michal Privoznik wrote:
ACK, but we really need a better way to detect capabilites. For instance now, bhyve binary is executed 4 times just to find out whether it supports 4 capabilities. That's just madness. Maybe we can get in touch with bhyve developers and ask them? Maybe it could have a monitor just like qemu has, or something. There's a patch[1] for it, but it would need to be moved along. Unfortunately, we would still need to somehow fall back to this for backwards compatibility...
Fabian
[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210111
Yeah, I'll at least rebase this patch onto the current codebase, but I plan to finish some of the libvirt stuff first (merge this uefi series with the changes suggested by Michal, add tablet support, update the docs and add virtio-console support). Roman Bogorodskiy

From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de> Allow to boot using UEFI rather than using an external boot loader such as bhyveload or grub-bhyve. Also, make LPC PCI-ISA bridge handling more flexible as now it's needed not only for serial ports, but for bootrom as well. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_command.c | 30 +++++++++++++++++++++++++- src/bhyve/bhyve_driver.c | 27 +++++++++++++++++++---- src/bhyve/bhyve_process.c | 55 +++++++++++++++++++++++++---------------------- 3 files changed, 81 insertions(+), 31 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index e7131625c..450800920 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -163,7 +163,6 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommandPtr cmd) return -1; } - virCommandAddArgList(cmd, "-s", "1,lpc", NULL); virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "com%d,%s", chr->target.port + 1, chr->source->data.file.path); @@ -283,6 +282,14 @@ bhyveBuildVirtIODiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, return 0; } +static int +bhyveBuildLPCArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, + virCommandPtr cmd) +{ + virCommandAddArgList(cmd, "-s", "1,lpc", NULL); + return 0; +} + virCommandPtr virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virDomainDefPtr def, bool dryRun) @@ -296,6 +303,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, * vm0 */ size_t i; + bool add_lpc = false; virCommandPtr cmd = virCommandNew(BHYVE); @@ -350,6 +358,21 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virCommandAddArg(cmd, "-P"); /* vmexit from guest on pause */ virCommandAddArgList(cmd, "-s", "0:0,hostbridge", NULL); + + if (def->os.bootloader == NULL && + def->os.loader) { + if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { + virCommandAddArg(cmd, "-l"); + virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); + add_lpc = true; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Installed bhyve binary does not support " + "UEFI loader")); + goto error; + } + } + /* Devices */ for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr controller = def->controllers[i]; @@ -389,8 +412,13 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, goto error; } } + + if (add_lpc || def->nserials) + bhyveBuildLPCArgStr(def, cmd); + if (bhyveBuildConsoleArgStr(def, cmd) < 0) goto error; + virCommandAddArg(cmd, def->name); return cmd; diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index f4ccef32b..0f9961ae0 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -734,15 +734,34 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, if (bhyveDomainAssignAddresses(def, NULL) < 0) goto cleanup; - if (!(loadcmd = virBhyveProcessBuildLoadCmd(conn, def, "<device.map>", + if (def->os.bootloader == NULL && + def->os.loader) { + + if ((def->os.loader->readonly != VIR_TRISTATE_BOOL_YES) + || (def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only read-only pflash is supported.")); + goto cleanup; + } + + if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM) == 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Installed bhyve binary does not support " + "bootrom")); + goto cleanup; + } + } else { + if (!(loadcmd = virBhyveProcessBuildLoadCmd(conn, def, "<device.map>", NULL))) - goto cleanup; + goto cleanup; + + virBufferAdd(&buf, virCommandToString(loadcmd), -1); + virBufferAddChar(&buf, '\n'); + } if (!(cmd = virBhyveProcessBuildBhyveCmd(conn, def, true))) goto cleanup; - virBufferAdd(&buf, virCommandToString(loadcmd), -1); - virBufferAddChar(&buf, '\n'); virBufferAdd(&buf, virCommandToString(cmd), -1); if (virBufferCheckError(&buf) < 0) diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 9d8097676..a97e300ff 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -165,38 +165,41 @@ virBhyveProcessStart(virConnectPtr conn, virCommandSetPidFile(cmd, privconn->pidfile); virCommandDaemonize(cmd); - /* Now bhyve command is constructed, meaning the - * domain is ready to be started, so we can build - * and execute bhyveload command */ - rc = virBhyveFormatDevMapFile(vm->def->name, &devmap_file); - if (rc < 0) - goto cleanup; + if (vm->def->os.loader == NULL) { + /* 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, devmap_file, - &devicemap))) - goto cleanup; - virCommandSetOutputFD(load_cmd, &logfd); - virCommandSetErrorFD(load_cmd, &logfd); + rc = virBhyveFormatDevMapFile(vm->def->name, &devmap_file); + if (rc < 0) + goto cleanup; - if (devicemap != NULL) { - rc = virFileWriteStr(devmap_file, devicemap, 0644); - if (rc) { - virReportSystemError(errno, - _("Cannot write device.map '%s'"), - devmap_file); + 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) - VIR_WARN("Unable to seek to end of logfile: %s", - virStrerror(errno, ebuf, sizeof(ebuf))); + /* Log generated command line */ + virCommandWriteArgLog(load_cmd, logfd); + if ((pos = lseek(logfd, 0, SEEK_END)) < 0) + VIR_WARN("Unable to seek to end of logfile: %s", + virStrerror(errno, ebuf, sizeof(ebuf))); - VIR_DEBUG("Loading domain '%s'", vm->def->name); - if (virCommandRun(load_cmd, NULL) < 0) - goto cleanup; + VIR_DEBUG("Loading domain '%s'", vm->def->name); + if (virCommandRun(load_cmd, NULL) < 0) + goto cleanup; + } /* Now we can start the domain */ VIR_DEBUG("Starting domain '%s'", vm->def->name); -- 2.11.0

On 02/12/2017 04:12 PM, Roman Bogorodskiy wrote:
From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Allow to boot using UEFI rather than using an external boot loader such as bhyveload or grub-bhyve.
Also, make LPC PCI-ISA bridge handling more flexible as now it's needed not only for serial ports, but for bootrom as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_command.c | 30 +++++++++++++++++++++++++- src/bhyve/bhyve_driver.c | 27 +++++++++++++++++++---- src/bhyve/bhyve_process.c | 55 +++++++++++++++++++++++++---------------------- 3 files changed, 81 insertions(+), 31 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index e7131625c..450800920 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -163,7 +163,6 @@ bhyveBuildConsoleArgStr(const virDomainDef *def, virCommandPtr cmd) return -1; }
- virCommandAddArgList(cmd, "-s", "1,lpc", NULL); virCommandAddArg(cmd, "-l"); virCommandAddArgFormat(cmd, "com%d,%s", chr->target.port + 1, chr->source->data.file.path); @@ -283,6 +282,14 @@ bhyveBuildVirtIODiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, return 0; }
+static int +bhyveBuildLPCArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, + virCommandPtr cmd) +{ + virCommandAddArgList(cmd, "-s", "1,lpc", NULL); + return 0; +} + virCommandPtr virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virDomainDefPtr def, bool dryRun) @@ -296,6 +303,7 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, * vm0 */ size_t i; + bool add_lpc = false;
virCommandPtr cmd = virCommandNew(BHYVE);
@@ -350,6 +358,21 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virCommandAddArg(cmd, "-P"); /* vmexit from guest on pause */
virCommandAddArgList(cmd, "-s", "0:0,hostbridge", NULL); + + if (def->os.bootloader == NULL && + def->os.loader) { + if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { + virCommandAddArg(cmd, "-l"); + virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); + add_lpc = true; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Installed bhyve binary does not support " + "UEFI loader")); + goto error; + } + } + /* Devices */ for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr controller = def->controllers[i]; @@ -389,8 +412,13 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, goto error; } } + + if (add_lpc || def->nserials) + bhyveBuildLPCArgStr(def, cmd); + if (bhyveBuildConsoleArgStr(def, cmd) < 0) goto error; + virCommandAddArg(cmd, def->name);
return cmd; diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index f4ccef32b..0f9961ae0 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -734,15 +734,34 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn, if (bhyveDomainAssignAddresses(def, NULL) < 0) goto cleanup;
- if (!(loadcmd = virBhyveProcessBuildLoadCmd(conn, def, "<device.map>", + if (def->os.bootloader == NULL && + def->os.loader) { + + if ((def->os.loader->readonly != VIR_TRISTATE_BOOL_YES) + || (def->os.loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH)) {
Please move the or operator at the end of the previous line.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only read-only pflash is supported.")); + goto cleanup; + }
ACK Michal

From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args | 11 +++++++++++ tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml | 23 +++++++++++++++++++++++ tests/bhyvexml2argvtest.c | 13 +++++++++++-- 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args new file mode 100644 index 000000000..8ff8673ed --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args @@ -0,0 +1,11 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-l bootrom,/path/to/test.fd \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 1,lpc bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs new file mode 100644 index 000000000..421376db9 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs @@ -0,0 +1 @@ +dummy diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml new file mode 100644 index 000000000..0b7d6bd27 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <loader readonly="yes" type="pflash">/path/to/test.fd</loader> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </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 fb404f9fb..8567ceeae 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -52,9 +52,13 @@ static int testCompareXMLToArgvFiles(const char *xml, conn->privateData = &driver; cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, false); - ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "<device.map>", + if (!vmdef->os.loader) + ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "<device.map>", &actualdm); + if ((ldcmd == NULL) && (vmdef->os.loader)) + ldcmd = virCommandNew("dummy"); + if ((cmd == NULL) || (ldcmd == NULL)) { if (flags & FLAG_EXPECT_FAILURE) { ret = 0; @@ -162,7 +166,8 @@ mymain(void) DO_TEST_FULL(name, FLAG_EXPECT_PARSE_ERROR) driver.grubcaps = BHYVE_GRUB_CAP_CONSDEV; - driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_AHCI32SLOT | BHYVE_CAP_NET_E1000; + driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_AHCI32SLOT | \ + BHYVE_CAP_NET_E1000 | BHYVE_CAP_LPC_BOOTROM; DO_TEST("base"); DO_TEST("acpiapic"); @@ -186,6 +191,7 @@ mymain(void) DO_TEST("serial-grub"); DO_TEST("localtime"); DO_TEST("net-e1000"); + DO_TEST("uefi"); /* Address allocation tests */ DO_TEST("addr-single-sata-disk"); @@ -208,6 +214,9 @@ mymain(void) DO_TEST_FAILURE("net-e1000"); + driver.bhyvecaps &= ~BHYVE_CAP_LPC_BOOTROM; + DO_TEST_FAILURE("uefi"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 2.11.0

On 02/12/2017 04:12 PM, Roman Bogorodskiy wrote:
From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args | 11 +++++++++++ tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml | 23 +++++++++++++++++++++++ tests/bhyvexml2argvtest.c | 13 +++++++++++-- 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args new file mode 100644 index 000000000..8ff8673ed --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args @@ -0,0 +1,11 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-l bootrom,/path/to/test.fd \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 1,lpc bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs new file mode 100644 index 000000000..421376db9 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs @@ -0,0 +1 @@ +dummy diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml new file mode 100644 index 000000000..0b7d6bd27 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml @@ -0,0 +1,23 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <loader readonly="yes" type="pflash">/path/to/test.fd</loader> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </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 fb404f9fb..8567ceeae 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -52,9 +52,13 @@ static int testCompareXMLToArgvFiles(const char *xml, conn->privateData = &driver;
cmd = virBhyveProcessBuildBhyveCmd(conn, vmdef, false); - ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "<device.map>", + if (!vmdef->os.loader) + ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "<device.map>", &actualdm);
Misaligned line.
+ if ((ldcmd == NULL) && (vmdef->os.loader)) + ldcmd = virCommandNew("dummy"); +
Or: if (vmdef->os.loader) ldcmd = virCommandNew("dummy"); else ldcmd = virBhyveProcessBuildLoadCmd(conn, vmdef, "<device.map>", &actualdm); ACK if you fix it. Michal

From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_capabilities.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 13b4835a8..9dec66b11 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -22,6 +22,9 @@ */ #include <config.h> #include <sys/utsname.h> +#include <dirent.h> +#include <stdio.h> +#include <sys/types.h> #include "viralloc.h" #include "virfile.h" @@ -114,11 +117,35 @@ virBhyveDomainCapsBuild(const char *emulatorbin, virDomainVirtType virttype) { virDomainCapsPtr caps = NULL; + DIR *dir; + struct dirent *entry; + const char *firmware_dir = "/usr/local/share/uefi-firmware"; + size_t firmwares_alloc = 0; if (!(caps = virDomainCapsNew(emulatorbin, machine, arch, virttype))) goto cleanup; caps->os.supported = true; + caps->os.loader.supported = true; + VIR_DOMAIN_CAPS_ENUM_SET(caps->os.loader.type, + VIR_DOMAIN_LOADER_TYPE_PFLASH); + VIR_DOMAIN_CAPS_ENUM_SET(caps->os.loader.readonly, + VIR_TRISTATE_BOOL_YES); + + if (virDirOpenIfExists(&dir, firmware_dir) > 0) { + while ((virDirRead(dir, &entry, firmware_dir)) > 0) { + if (VIR_RESIZE_N(caps->os.loader.values.values, + firmwares_alloc, caps->os.loader.values.nvalues, 2) < 0) + goto cleanup; + + if (virAsprintf( + &caps->os.loader.values.values[caps->os.loader.values.nvalues], + "%s/%s", firmware_dir, entry->d_name) < 0) + goto cleanup; + + caps->os.loader.values.nvalues++; + } + } caps->disk.supported = true; VIR_DOMAIN_CAPS_ENUM_SET(caps->disk.diskDevice, VIR_DOMAIN_DISK_DEVICE_DISK, @@ -129,6 +156,7 @@ virBhyveDomainCapsBuild(const char *emulatorbin, VIR_DOMAIN_DISK_BUS_VIRTIO); cleanup: + VIR_DIR_CLOSE(dir); return caps; } -- 2.11.0

On 02/12/2017 04:12 PM, Roman Bogorodskiy wrote:
From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_capabilities.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
ACK Michal

On Sun, Feb 12, 2017 at 07:12:32PM +0400, Roman Bogorodskiy wrote:
From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_capabilities.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 13b4835a8..9dec66b11 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -22,6 +22,9 @@ */ #include <config.h> #include <sys/utsname.h> +#include <dirent.h> +#include <stdio.h> +#include <sys/types.h>
#include "viralloc.h" #include "virfile.h" @@ -114,11 +117,35 @@ virBhyveDomainCapsBuild(const char *emulatorbin, virDomainVirtType virttype) { virDomainCapsPtr caps = NULL; + DIR *dir; + struct dirent *entry; + const char *firmware_dir = "/usr/local/share/uefi-firmware";
Is this path really something that's ok to hard code like this ? I'm not familiar with *BSD filesystem layout conventions, but on Lnux, /usr/local would not be a typical directory for system provided firmware - /usr/share would be expected. This feels like something that ought to be overridable by the user perhaps in a /etc/libvirt/bhyve.conf file, similar to how QEMU allows configurable firmware locations Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|

Daniel P. Berrange wrote:
On Sun, Feb 12, 2017 at 07:12:32PM +0400, Roman Bogorodskiy wrote:
From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/bhyve/bhyve_capabilities.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 13b4835a8..9dec66b11 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -22,6 +22,9 @@ */ #include <config.h> #include <sys/utsname.h> +#include <dirent.h> +#include <stdio.h> +#include <sys/types.h>
#include "viralloc.h" #include "virfile.h" @@ -114,11 +117,35 @@ virBhyveDomainCapsBuild(const char *emulatorbin, virDomainVirtType virttype) { virDomainCapsPtr caps = NULL; + DIR *dir; + struct dirent *entry; + const char *firmware_dir = "/usr/local/share/uefi-firmware";
Is this path really something that's ok to hard code like this ? I'm not familiar with *BSD filesystem layout conventions, but on Lnux, /usr/local would not be a typical directory for system provided firmware - /usr/share would be expected. This feels like something that ought to be overridable by the user perhaps in a /etc/libvirt/bhyve.conf file, similar to how QEMU allows configurable firmware locations
Yeah, things are a little different on FreeBSD compared to Linux, and almost all user-installed stuff goes into /usr/local, including the software installed via ports/packages [1]. Specifically, this very path points to the firmware installed via the port: https://www.freshports.org/sysutils/uefi-edk2-bhyve/ So it will work as expected for most users (I goes only few change the installation prefix). But you're right, it's not very good to hardcode it like that. Using some ${prefix}-based substitution will not work as well because libvirt prefix and uefi firmware paths are generally unrelated. The only thing I'm worried about is introducing of a config file just for the sake of a single option, though probably it has to happen sooner or later... 1: https://man.freebsd.org/hier(7) Roman Bogorodskiy

From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de> bhyve supports 'gop' video device that allows clients to connect to VMs using VNC clients. This commit adds support for that to the bhyve driver: - Introducr 'gop' video device type - Add capabilities probing for the 'fbuf' device that's responsible for graphics - Update command builder routines to let users configure domain's VNC via gop graphics. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/formatdomain.html.in | 3 +- docs/schemas/domaincommon.rng | 1 + po/POTFILES.in | 1 + src/bhyve/bhyve_capabilities.c | 40 +++++++++++++++ src/bhyve/bhyve_capabilities.h | 1 + src/bhyve/bhyve_command.c | 100 ++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_device.c | 11 ++++ src/conf/domain_conf.c | 5 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 9 ++-- src/qemu/qemu_domain_address.c | 1 + tests/domaincapsschemadata/full.xml | 1 + 12 files changed, 169 insertions(+), 5 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0a115f5dc..b853a7245 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5812,8 +5812,9 @@ qemu-kvm -net nic,model=? /dev/null <p> The <code>model</code> element has a mandatory <code>type</code> attribute which takes the value "vga", "cirrus", "vmvga", "xen", - "vbox", "qxl" (<span class="since">since 0.8.6</span>) or + "vbox", "qxl" (<span class="since">since 0.8.6</span>), "virtio" (<span class="since">since 1.3.0</span>) + or "gop" (<span class="since">since 3.1.0</span>) depending on the hypervisor features available. </p> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d715bff29..3e3793ea0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3152,6 +3152,7 @@ <value>xen</value> <value>vbox</value> <value>virtio</value> + <value>gop</value> </choice> </attribute> <group> diff --git a/po/POTFILES.in b/po/POTFILES.in index 365ea662f..c6d350da7 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -12,6 +12,7 @@ gnulib/lib/getopt.c gnulib/lib/regcomp.c src/access/viraccessdriverpolkit.c src/access/viraccessmanager.c +src/bhyve/bhyve_capabilities.c src/bhyve/bhyve_command.c src/bhyve/bhyve_device.c src/bhyve/bhyve_domain.c diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 9dec66b11..5539d2978 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -117,6 +117,7 @@ virBhyveDomainCapsBuild(const char *emulatorbin, virDomainVirtType virttype) { virDomainCapsPtr caps = NULL; + unsigned int bhyve_caps = 0; DIR *dir; struct dirent *entry; const char *firmware_dir = "/usr/local/share/uefi-firmware"; @@ -125,6 +126,12 @@ virBhyveDomainCapsBuild(const char *emulatorbin, if (!(caps = virDomainCapsNew(emulatorbin, machine, arch, virttype))) goto cleanup; + if (virBhyveProbeCaps(&bhyve_caps)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed probing capabilities")); + goto cleanup; + } + caps->os.supported = true; caps->os.loader.supported = true; VIR_DOMAIN_CAPS_ENUM_SET(caps->os.loader.type, @@ -155,6 +162,12 @@ virBhyveDomainCapsBuild(const char *emulatorbin, VIR_DOMAIN_DISK_BUS_SATA, VIR_DOMAIN_DISK_BUS_VIRTIO); + if (bhyve_caps & BHYVE_CAP_FBUF) { + caps->graphics.supported = true; + caps->video.supported = true; + VIR_DOMAIN_CAPS_ENUM_SET(caps->graphics.type, VIR_DOMAIN_GRAPHICS_TYPE_VNC); + VIR_DOMAIN_CAPS_ENUM_SET(caps->video.modelType, VIR_DOMAIN_VIDEO_TYPE_GOP); + } cleanup: VIR_DIR_CLOSE(dir); return caps; @@ -289,6 +302,30 @@ bhyveProbeCapsLPC_Bootrom(unsigned int *caps, char *binary) return ret; } + +static int +bhyveProbeCapsFramebuffer(unsigned int *caps, char *binary) +{ + char *error; + virCommandPtr cmd = NULL; + int ret = -1, exit; + + cmd = virCommandNew(binary); + virCommandAddArgList(cmd, "-s", "0,fbuf", NULL); + virCommandSetErrorBuffer(cmd, &error); + if (virCommandRun(cmd, &exit) < 0) + goto cleanup; + + if (strstr(error, "pci slot 0:0: unknown device \"fbuf\"") == NULL) + *caps |= BHYVE_CAP_FBUF; + + ret = 0; + cleanup: + VIR_FREE(error); + virCommandFree(cmd); + return ret; +} + int virBhyveProbeCaps(unsigned int *caps) { @@ -313,6 +350,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsLPC_Bootrom(caps, binary))) goto out; + if ((ret = bhyveProbeCapsFramebuffer(caps, binary))) + goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 746c77181..8fb97d730 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -41,6 +41,7 @@ typedef enum { BHYVE_CAP_AHCI32SLOT = 1 << 1, BHYVE_CAP_NET_E1000 = 1 << 2, BHYVE_CAP_LPC_BOOTROM = 1 << 3, + BHYVE_CAP_FBUF = 1 << 4, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 450800920..ec7a71572 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -290,6 +290,94 @@ bhyveBuildLPCArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, return 0; } +static int +bhyveBuildGraphicsArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, + virDomainGraphicsDefPtr graphics, + virDomainVideoDefPtr video, + virConnectPtr conn, + virCommandPtr cmd) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + virDomainGraphicsListenDefPtr glisten = NULL; + bool escapeAddr; + + if (!(bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM) || + def->os.bootloader || + !def->os.loader) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Graphics are only supported" + " when booting using UEFI")); + return -1; + } + + if (!(bhyveDriverGetCaps(conn) & BHYVE_CAP_FBUF)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Bhyve version does not support framebuffer")); + return -1; + } + + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only VNC supported")); + return -1; + } + + if (!(glisten = virDomainGraphicsGetListen(graphics, 0))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing listen element")); + goto error; + } + + virBufferAsprintf(&opt, "%d:%d,fbuf", video->info.addr.pci.slot, video->info.addr.pci.function); + + switch (glisten->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + virBufferAddLit(&opt, ",tcp="); + + if (!graphics->data.vnc.autoport && + (graphics->data.vnc.port < 5900 || + graphics->data.vnc.port > 65535)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc port must be in range [5900,65535]")); + goto error; + } + + if (graphics->data.vnc.auth.passwd) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc password auth not supported")); + goto error; + } else { + /* Bhyve doesn't support VNC Auth yet, so print a warning about + * unauthenticated VNC sessions */ + VIR_WARN("%s", _("Security warning: currently VNC auth is not" + " supported.")); + } + + if (glisten->address) { + escapeAddr = strchr(glisten->address, ':') != NULL; + if (escapeAddr) + virBufferAsprintf(&opt, "[%s]", glisten->address); + else + virBufferAdd(&opt, glisten->address, -1); + } + + virBufferAsprintf(&opt, ":%d", graphics->data.vnc.port); + break; + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Unsupported listen type")); + } + + virCommandAddArg(cmd, "-s"); + virCommandAddArgBuffer(cmd, &opt); + return 0; + + error: + virBufferFreeAndReset(&opt); + return -1; +} + virCommandPtr virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virDomainDefPtr def, bool dryRun) @@ -413,6 +501,18 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, } } + if (def->ngraphics && def->nvideos) { + if (def->ngraphics == 1 && def->nvideos == 1) { + if (bhyveBuildGraphicsArgStr(def, def->graphics[0], def->videos[0], conn, cmd) < 0) + goto error; + add_lpc = true; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Multiple graphics devices are not supported")); + goto error; + } + } + if (add_lpc || def->nserials) bhyveBuildLPCArgStr(def, cmd); diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 55ce631ec..a3a263b7e 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -145,6 +145,17 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, goto error; } + for (i = 0; i < def->nvideos; i++) { + if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info)) + continue; + if (virDomainPCIAddressReserveNextAddr(addrs, + &def->videos[i]->info, + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, + -1) < 0) + goto error; + } + + return 0; error: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1bc72a4e9..77fe87582 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -545,7 +545,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "vbox", "qxl", "parallels", - "virtio") + "virtio", + "gop") VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST, "mouse", @@ -13018,6 +13019,8 @@ virDomainVideoDefaultType(const virDomainDef *def) return VIR_DOMAIN_VIDEO_TYPE_VGA; else return VIR_DOMAIN_VIDEO_TYPE_PARALLELS; + case VIR_DOMAIN_VIRT_BHYVE: + return VIR_DOMAIN_VIDEO_TYPE_GOP; default: return -1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index dd79206f6..0876eabbf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1335,6 +1335,7 @@ typedef enum { VIR_DOMAIN_VIDEO_TYPE_QXL, VIR_DOMAIN_VIDEO_TYPE_PARALLELS, /* pseudo device for VNC in containers */ VIR_DOMAIN_VIDEO_TYPE_VIRTIO, + VIR_DOMAIN_VIDEO_TYPE_GOP, VIR_DOMAIN_VIDEO_TYPE_LAST } virDomainVideoType; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c00a47a91..aabdcdcee 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -100,7 +100,8 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "", /* don't support vbox */ "qxl", "", /* don't support parallels */ - "" /* no need for virtio */); + "", /* no need for virtio */ + "" /* don't support gop */); VIR_ENUM_DECL(qemuDeviceVideo) @@ -112,7 +113,8 @@ VIR_ENUM_IMPL(qemuDeviceVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "", /* don't support vbox */ "qxl-vga", "", /* don't support parallels */ - "virtio-vga"); + "virtio-vga", + "" /* don't support gop */); VIR_ENUM_DECL(qemuDeviceVideoSecondary) @@ -124,7 +126,8 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary, VIR_DOMAIN_VIDEO_TYPE_LAST, "", /* don't support vbox */ "qxl", "", /* don't support parallels */ - "virtio-gpu-pci"); + "virtio-gpu-pci", + "" /* don't support gop */); VIR_ENUM_DECL(qemuSoundCodec) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 70482f224..e22adacb2 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -741,6 +741,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_VIDEO_TYPE_PARALLELS: return pciFlags; + case VIR_DOMAIN_VIDEO_TYPE_GOP: case VIR_DOMAIN_VIDEO_TYPE_LAST: return 0; } diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml index 6abd4995a..6a676253c 100644 --- a/tests/domaincapsschemadata/full.xml +++ b/tests/domaincapsschemadata/full.xml @@ -70,6 +70,7 @@ <value>qxl</value> <value>parallels</value> <value>virtio</value> + <value>gop</value> </enum> </video> <hostdev supported='yes'> -- 2.11.0

On 02/12/2017 04:12 PM, Roman Bogorodskiy wrote:
From: Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
bhyve supports 'gop' video device that allows clients to connect to VMs using VNC clients. This commit adds support for that to the bhyve driver:
- Introducr 'gop' video device type - Add capabilities probing for the 'fbuf' device that's responsible for graphics - Update command builder routines to let users configure domain's VNC via gop graphics.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/formatdomain.html.in | 3 +- docs/schemas/domaincommon.rng | 1 + po/POTFILES.in | 1 + src/bhyve/bhyve_capabilities.c | 40 +++++++++++++++ src/bhyve/bhyve_capabilities.h | 1 + src/bhyve/bhyve_command.c | 100 ++++++++++++++++++++++++++++++++++++ src/bhyve/bhyve_device.c | 11 ++++ src/conf/domain_conf.c | 5 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 9 ++-- src/qemu/qemu_domain_address.c | 1 + tests/domaincapsschemadata/full.xml | 1 + 12 files changed, 169 insertions(+), 5 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0a115f5dc..b853a7245 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5812,8 +5812,9 @@ qemu-kvm -net nic,model=? /dev/null <p> The <code>model</code> element has a mandatory <code>type</code> attribute which takes the value "vga", "cirrus", "vmvga", "xen", - "vbox", "qxl" (<span class="since">since 0.8.6</span>) or + "vbox", "qxl" (<span class="since">since 0.8.6</span>), "virtio" (<span class="since">since 1.3.0</span>) + or "gop" (<span class="since">since 3.1.0</span>)
since 3.2.0 actually :-)
depending on the hypervisor features available. </p> <p>
ACK Michal

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 12 +++++++++++ tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml | 26 ++++++++++++++++++++++++ tests/bhyvexml2argvtest.c | 7 ++++++- 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args new file mode 100644 index 000000000..90889b8f3 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args @@ -0,0 +1,12 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-l bootrom,/path/to/test.fd \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \ +-s 4:0,fbuf,tcp=127.0.0.1:5904 \ +-s 1,lpc bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs new file mode 100644 index 000000000..421376db9 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs @@ -0,0 +1 @@ +dummy diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml new file mode 100644 index 000000000..9bef80bb4 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml @@ -0,0 +1,26 @@ +<domain type='bhyve'> + <name>bhyve</name> + <uuid>df3be7e7-a104-11e3-aeb0-50e5492bd3dc</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type>hvm</type> + <loader readonly="yes" type="pflash">/path/to/test.fd</loader> + </os> + <devices> + <disk type='file'> + <driver name='file' type='raw'/> + <source file='/tmp/freebsd.img'/> + <target dev='hda' bus='sata'/> + <address type='drive' controller='0' bus='0' target='2' unit='0'/> + </disk> + <interface type='bridge'> + <model type='virtio'/> + <source bridge="virbr0"/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <graphics type='vnc' port='5904'> + <listen type='address' address='127.0.0.1'/> + </graphics> + </devices> +</domain> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c index 8567ceeae..9089807c6 100644 --- a/tests/bhyvexml2argvtest.c +++ b/tests/bhyvexml2argvtest.c @@ -167,7 +167,8 @@ mymain(void) driver.grubcaps = BHYVE_GRUB_CAP_CONSDEV; driver.bhyvecaps = BHYVE_CAP_RTC_UTC | BHYVE_CAP_AHCI32SLOT | \ - BHYVE_CAP_NET_E1000 | BHYVE_CAP_LPC_BOOTROM; + BHYVE_CAP_NET_E1000 | BHYVE_CAP_LPC_BOOTROM | \ + BHYVE_CAP_FBUF; DO_TEST("base"); DO_TEST("acpiapic"); @@ -192,6 +193,7 @@ mymain(void) DO_TEST("localtime"); DO_TEST("net-e1000"); DO_TEST("uefi"); + DO_TEST("vnc"); /* Address allocation tests */ DO_TEST("addr-single-sata-disk"); @@ -217,6 +219,9 @@ mymain(void) driver.bhyvecaps &= ~BHYVE_CAP_LPC_BOOTROM; DO_TEST_FAILURE("uefi"); + driver.bhyvecaps &= ~BHYVE_CAP_FBUF; + DO_TEST_FAILURE("vnc"); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 2.11.0

On 02/12/2017 04:12 PM, Roman Bogorodskiy wrote:
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 12 +++++++++++ tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml | 26 ++++++++++++++++++++++++ tests/bhyvexml2argvtest.c | 7 ++++++- 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml
ACK Michal

Michal Privoznik wrote:
On 02/12/2017 04:12 PM, Roman Bogorodskiy wrote:
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 12 +++++++++++ tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml | 26 ++++++++++++++++++++++++ tests/bhyvexml2argvtest.c | 7 ++++++- 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml
ACK
Michal
I pushed the series with the suggested fixes squashed in. Thanks a lot for reviewing. I'll send a follow up patch to support setting firmware directory via config file. Roman Bogorodskiy

Roman Bogorodskiy wrote:
Fabian Freyer (5): bhyve: virBhyveProbeCaps: BHYVE_CAP_LPC_BOOTROM bhyve: add support for booting from UEFI bhyve: test cases for UEFI bhyvexml2argvtest bhyve: enumerate UEFI firmwares bhyve: add video support
Roman Bogorodskiy (1): bhyve: test cases for VNC
ping?
docs/formatdomain.html.in | 3 +- docs/schemas/domaincommon.rng | 1 + po/POTFILES.in | 1 + src/bhyve/bhyve_capabilities.c | 93 ++++++++++++++++ src/bhyve/bhyve_capabilities.h | 2 + src/bhyve/bhyve_command.c | 130 +++++++++++++++++++++- src/bhyve/bhyve_device.c | 11 ++ src/bhyve/bhyve_driver.c | 27 ++++- src/bhyve/bhyve_process.c | 55 ++++----- src/conf/domain_conf.c | 5 +- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 9 +- src/qemu/qemu_domain_address.c | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args | 11 ++ tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml | 23 ++++ tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args | 12 ++ tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml | 26 +++++ tests/bhyvexml2argvtest.c | 18 ++- tests/domaincapsschemadata/full.xml | 1 + 21 files changed, 394 insertions(+), 38 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-uefi.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc.xml
-- 2.11.0
Roman Bogorodskiy
participants (4)
-
Daniel P. Berrange
-
Fabian Freyer
-
Michal Privoznik
-
Roman Bogorodskiy