[libvirt PATCH 04/10] conf: validate NVRAM template usage with R/W loader binary

[Michal Prívozník]

On 2/15/22 19:54, Daniel P. Berrangé wrote:
> The QEMU driver will populate the template to the nvram file any time it
> sees both the template and nvram paths present. It will auto-generate a
> nvram path per-VM if not provided by the user, but only if the loader
> is marked R/O.
> 
> So with a R/O loader we have these possible scenarios
> 
>   - No NVRAM path or template -> try to infer a template based on the
>                                  loader path, if not possible, fatal
> 				 error. Auto-generate NVRAM per per VM
>   - NVRAM path only -> try to infer a template based on the loader path,
>                        if not possible, app must have pre-created NVRAM
>   - NVRAM path + template -> QEMU driver will copy template to NVRAM
>   - NVRAM template only -> auto-generate NVRAM path per VM and then
>                            copy template
> 
> While with a R/W loader we have these possible scenarios
> 
>   - No NVRAM path or template -> do nothing
>   - NVRAM path only -> app must have pre-created NVRAM
>   - NVRAM path + template -> QEMU driver will copy template to NVRAM
>   - NVRAM template only -> silently ignored
> 
> This change improves the last scenario by reporting an error from the
> parser. Two alternative strategies though would be:
> 
>   - Auto-generate a NVRAM path per VM
>   - Don't support templates at all with R/W loader
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/conf/domain_conf.c                        | 16 ++++++--
>  ...-nvram-rw-template-vars.x86_64-latest.args | 41 +++++++++++++++++++
>  .../bios-nvram-rw-template-vars.xml           | 36 ++++++++++++++++
>  .../bios-nvram-rw-template.err                |  1 +
>  .../bios-nvram-rw-template.xml                | 36 ++++++++++++++++
>  .../bios-nvram-rw-vars.x86_64-latest.args     | 41 +++++++++++++++++++
>  tests/qemuxml2argvdata/bios-nvram-rw-vars.xml | 36 ++++++++++++++++
>  tests/qemuxml2argvtest.c                      |  3 ++
>  8 files changed, 207 insertions(+), 3 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.xml
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.err
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.xml
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.xml
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ab8f2a52cc..31b49c4ec9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4808,17 +4808,26 @@ virDomainDefPostParseMemory(virDomainDef *def,
>  }
>  
>  
> -static void
> +static int
>  virDomainDefPostParseOs(virDomainDef *def)
>  {
>      if (!def->os.loader)
> -        return;
> +        return 0;
>  
>      if (def->os.loader->path &&
>          def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_NONE) {
>          /* By default, loader is type of 'rom' */
>          def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_ROM;
>      }
> +
> +    if (def->os.loader->readonly != VIR_TRISTATE_BOOL_YES &&
> +        def->os.loader->templt && !def->os.loader->nvram) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("NVRAM template without VARs path not permitted with writable loader"));
> +        return -1;
> +    }

virDomainDefOSValidate() looks like a better fit for this check.
> +
> +    return 0;
>  }

Michal