Re: [pve-devel] [PATCH qemu-server 2/2] catch missing ovmf file

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 06/12/2022 um 14:11 schrieb Noel Ullreich:
> check to see if the OVMF_VARS file actually exists. otherwise lines
> 3666 and 3673 break and give a cryptic error message

I do not think that referencing lines in the commit message is helpful, rather
just describe it in general, e.g., ".. otherwise subsequent code breaks with
cryptic errors"

> 
> Signed-off-by: Noel Ullreich <n.ullreich@proxmox.com>
> ---
>  PVE/QemuServer.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 2a2f1f7..38f3145 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3640,6 +3640,7 @@ sub config_to_command {
>  
>  	my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35);
>  	die "EFI base image '$ovmf_code' not found\n" if ! -f $ovmf_code;
> +	die "EFI vars image '$ovmf_vars' not found\n" if ! -f $ovmf_vars;


why not move this check into the get_ovmf_files sub then, so that it's in a
central place? If we need to skip checking in some place (we shouldn't), one
could just add a $nocheck parameter to the helper for opting-out.

I mean, qemu-server's dependency on "pve-edk2-firmware" should already avoid
this error in the first place anyway...

>  
>  	my ($path, $format);
>  	my $read_only_str = '';