[libvirt] [PATCH] domaincapstest: add bhyve caps test

Michal Privoznik mprivozn at redhat.com
Tue Mar 21 14:15:46 UTC 2017


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




More information about the libvir-list mailing list