[libvirt] [PATCH v2 11/15] qemu_firmware: Introduce qemuFirmwareFillDomain()

[Daniel P. Berrangé]

On Thu, Mar 07, 2019 at 10:29:21AM +0100, Michal Privoznik wrote:
> And finally the last missing piece. This is what puts it all
> together.
> 
> At the beginning, qemuFirmwareFillDomain() loads all possible
> firmware description files based on algorithm described earlier.
> Then it tries to find description which matches given domain.
> The criteria are:
> 
>   - firmware is the right type (e.g. it's bios when bios was
>     requested in domain XML)
>   - firmware is suitable for guest architecture/machine type
>   - firmware allows desired guest features to stay enabled (e.g.
>     if s3/s4 is enabled for guest then firmware has to support
>     it too)
> 
> Once the desired description has been found it is then used to
> set various bits of virDomainDef so that proper qemu cmd line is
> constructed as demanded by the description file. For instance,
> secure boot enabled firmware might request SMM -> it will be
> enabled if needed.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_firmware.c | 329 +++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_firmware.h |   7 +
>  2 files changed, 336 insertions(+)
> 
> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
> index a818f60c91..c8b337cf2a 100644
> --- a/src/qemu/qemu_firmware.c
> +++ b/src/qemu/qemu_firmware.c
> @@ -23,6 +23,8 @@
>  #include "qemu_firmware.h"
>  #include "configmake.h"
>  #include "qemu_capabilities.h"
> +#include "qemu_domain.h"
> +#include "qemu_process.h"
>  #include "virarch.h"
>  #include "virfile.h"
>  #include "virhash.h"
> @@ -1033,3 +1035,330 @@ qemuFirmwareFetchConfigs(char ***firmwares)
>  
>      return 0;
>  }
> +
> +
> +static bool
> +qemuFirmwareMatchDomain(const virDomainDef *def,
> +                        const qemuFirmware *fw,
> +                        const char *path)
> +{
> +    size_t i;
> +    bool supportsS3 = false;
> +    bool supportsS4 = false;
> +    bool requiresSMM = false;
> +    bool supportsSEV = false;
> +
> +    for (i = 0; i < fw->ninterfaces; i++) {
> +        if ((def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS &&
> +             fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_BIOS) ||
> +            (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI &&
> +             fw->interfaces[i] == QEMU_FIRMWARE_OS_INTERFACE_UEFI))
> +            break;
> +    }
> +
> +    if (i == fw->ninterfaces) {
> +        VIR_DEBUG("No matching interface in '%s'", path);
> +        return false;
> +    }
> +
> +    for (i = 0; i < fw->ntargets; i++) {
> +        size_t j;
> +
> +        if (def->os.arch != fw->targets[i]->architecture)
> +            continue;
> +
> +        for (j = 0; j < fw->targets[i]->nmachines; j++) {
> +            if (virStringMatch(def->os.machine, fw->targets[i]->machines[j]))

Hmm, virStringMatch does a regex match, but AFAIK, the firmware
files are using shell style globs. IOW, I think we need fnmatch
instead.

I think you got lucky during testing as the glob string

              pc-q35-*

is still going to match when interpreted as a regex. ie string prefix
"pc-q35" followed by zero or many "-", followed by arbitrary text
since it is not anchored with "$"



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|