On 03/19/2017 07:05 AM, Roman Bogorodskiy wrote:
* Extract filling bhyve capabilities from virBhyveDomainCapsBuild() into a new function virBhyveDomainCapsFill() to make testing easier by not having to mock firmware directory listing and hypervisor capabilities probing * Also, just presence of the firmware files is not sufficient to enable os.loader.supported, hypervisor should support UEFI boot too * Add tests to domaincapstest for the main caps possible flows: - when UEFI bootrom is supported - when video (fbus) is supported - neither of above is supported --- src/bhyve/bhyve_capabilities.c | 72 +++++++++++++++-------- src/bhyve/bhyve_capabilities.h | 3 + tests/Makefile.am | 4 ++ tests/domaincapsschemadata/bhyve_basic.x86_64.xml | 32 ++++++++++ tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml | 49 +++++++++++++++ tests/domaincapsschemadata/bhyve_uefi.x86_64.xml | 41 +++++++++++++ tests/domaincapstest.c | 65 ++++++++++++++++++++ 7 files changed, 242 insertions(+), 24 deletions(-) create mode 100644 tests/domaincapsschemadata/bhyve_basic.x86_64.xml create mode 100644 tests/domaincapsschemadata/bhyve_fbuf.x86_64.xml create mode 100644 tests/domaincapsschemadata/bhyve_uefi.x86_64.xml
diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 4bf1d84fa..6f8be45c4 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -87,6 +87,44 @@ virBhyveCapsBuild(void) return NULL; }
+int +virBhyveDomainCapsFill(virDomainCapsPtr caps, + unsigned int bhyvecaps, + virDomainCapsStringValuesPtr firmwares) +{ + caps->disk.supported = true; + VIR_DOMAIN_CAPS_ENUM_SET(caps->disk.diskDevice, + VIR_DOMAIN_DISK_DEVICE_DISK, + VIR_DOMAIN_DISK_DEVICE_CDROM); + + VIR_DOMAIN_CAPS_ENUM_SET(caps->disk.bus, + VIR_DOMAIN_DISK_BUS_SATA, + VIR_DOMAIN_DISK_BUS_VIRTIO); + + caps->os.supported = true; + + if (bhyvecaps & BHYVE_CAP_LPC_BOOTROM) { + 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); + + caps->os.loader.values.values = firmwares->values; + caps->os.loader.values.nvalues = firmwares->nvalues; + } + + + if (bhyvecaps & 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); + } + return 0; +} + + virDomainCapsPtr virBhyveDomainCapsBuild(bhyveConnPtr conn, const char *emulatorbin, @@ -101,6 +139,7 @@ virBhyveDomainCapsBuild(bhyveConnPtr conn, size_t firmwares_alloc = 0; virBhyveDriverConfigPtr cfg = virBhyveDriverGetConfig(conn); const char *firmware_dir = cfg->firmwareDir; + virDomainCapsStringValuesPtr firmwares = NULL;
if (!(caps = virDomainCapsNew(emulatorbin, machine, arch, virttype))) goto cleanup; @@ -111,46 +150,31 @@ virBhyveDomainCapsBuild(bhyveConnPtr conn, 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 (VIR_ALLOC(firmwares) < 0) + goto cleanup;
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) + if (VIR_RESIZE_N(firmwares->values, + firmwares_alloc, firmwares->nvalues, 1) < 0) goto cleanup;
if (virAsprintf( - &caps->os.loader.values.values[caps->os.loader.values.nvalues], + &firmwares->values[firmwares->nvalues], "%s/%s", firmware_dir, entry->d_name) < 0) goto cleanup;
- caps->os.loader.values.nvalues++; + firmwares->nvalues++; } } else { VIR_WARN("Cannot open firmware directory %s", firmware_dir); }
- caps->disk.supported = true; - VIR_DOMAIN_CAPS_ENUM_SET(caps->disk.diskDevice, - VIR_DOMAIN_DISK_DEVICE_DISK, - VIR_DOMAIN_DISK_DEVICE_CDROM); - - VIR_DOMAIN_CAPS_ENUM_SET(caps->disk.bus, - VIR_DOMAIN_DISK_BUS_SATA, - VIR_DOMAIN_DISK_BUS_VIRTIO); + if (virBhyveDomainCapsFill(caps, bhyve_caps, firmwares) < 0) + goto cleanup;
- 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_FREE(firmwares); VIR_DIR_CLOSE(dir); virObjectUnref(cfg); return caps; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 3db4f1b88..194061fde 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -28,6 +28,9 @@ # include "bhyve_utils.h"
virCapsPtr virBhyveCapsBuild(void); +int virBhyveDomainCapsFill(virDomainCapsPtr caps, + unsigned int bhyvecaps, + virDomainCapsStringValuesPtr firmwares); virDomainCapsPtr virBhyveDomainCapsBuild(bhyveConnPtr, const char *emulatorbin, const char *machine, diff --git a/tests/Makefile.am b/tests/Makefile.am index af69a3a84..d4eedaf17 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -964,6 +964,10 @@ domaincapstest_SOURCES += testutilsxen.c testutilsxen.h domaincapstest_LDADD += ../src/libvirt_driver_libxl_impl.la $(GNULIB_LIBS) endif WITH_LIBXL
+if WITH_BHYVE +domaincapstest_LDADD += ../src/libvirt_driver_bhyve_impl.la $(GNULIB_LIBS) +endif WITH_BHYVE + virnetmessagetest_SOURCES = \ virnetmessagetest.c testutils.h testutils.c virnetmessagetest_CFLAGS = $(XDR_CFLAGS) $(AM_CFLAGS) diff --git a/tests/domaincapsschemadata/bhyve_basic.x86_64.xml b/tests/domaincapsschemadata/bhyve_basic.x86_64.xml new file mode 100644 index 000000000..f6dfabed2 --- /dev/null +++ b/tests/domaincapsschemadata/bhyve_basic.x86_64.xml @@ -0,0 +1,32 @@ +<domainCapabilities> + <path>/usr/sbin/bhyve</path> + <domain>bhyve</domain> + <machine>(null)</machine>
This doesn't feel right. We should not output machine if it's NULL. We might need to change docs too: http://libvirt.org/formatdomaincaps.html#elements since there is no machine type in bhyve. I'll post a patch for that after which you'll need to regenerate the output of your tests. After that you have my ACK and you can push this one. Michal