* [pve-devel] [PATCH qemu-server 0/2] fix #4378: standardized error for ovmf files @ 2022-12-06 13:11 Noel Ullreich 2022-12-06 13:11 ` [pve-devel] [PATCH qemu-server 1/2] " Noel Ullreich 2022-12-06 13:11 ` [pve-devel] [PATCH qemu-server 2/2] catch missing ovmf file Noel Ullreich 0 siblings, 2 replies; 4+ messages in thread From: Noel Ullreich @ 2022-12-06 13:11 UTC (permalink / raw) To: pve-devel * fixed the inconsistensies in the error messages for missing OVMF files * added a check for OVMF_VAR files so that errors are not as cryptic Noel Ullreich (2): fix #4378: standardized error for ovmf files catch missing ovmf file PVE/QemuServer.pm | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) -- 2.30.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] [PATCH qemu-server 1/2] fix #4378: standardized error for ovmf files 2022-12-06 13:11 [pve-devel] [PATCH qemu-server 0/2] fix #4378: standardized error for ovmf files Noel Ullreich @ 2022-12-06 13:11 ` Noel Ullreich 2022-12-06 13:11 ` [pve-devel] [PATCH qemu-server 2/2] catch missing ovmf file Noel Ullreich 1 sibling, 0 replies; 4+ messages in thread From: Noel Ullreich @ 2022-12-06 13:11 UTC (permalink / raw) To: pve-devel The error messages for missing OVMF_CODE and OVMF_VARS files were inconsistent as well as the error for the missing base var file not telling you the expected path. Signed-off-by: Noel Ullreich <n.ullreich@proxmox.com> --- PVE/QemuServer.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index a52a883..2a2f1f7 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -3639,7 +3639,7 @@ sub config_to_command { } my ($ovmf_code, $ovmf_vars) = get_ovmf_files($arch, $d, $q35); - die "uefi base image '$ovmf_code' not found\n" if ! -f $ovmf_code; + die "EFI base image '$ovmf_code' not found\n" if ! -f $ovmf_code; my ($path, $format); my $read_only_str = ''; @@ -8071,7 +8071,7 @@ sub get_efivars_size { $efidisk //= $conf->{efidisk0} ? parse_drive('efidisk0', $conf->{efidisk0}) : undef; my $smm = PVE::QemuServer::Machine::machine_type_is_q35($conf); my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm); - die "uefi vars image '$ovmf_vars' not found\n" if ! -f $ovmf_vars; + die "EFI vars image '$ovmf_vars' not found\n" if ! -f $ovmf_vars; return -s $ovmf_vars; } @@ -8099,7 +8099,7 @@ sub create_efidisk($$$$$$$) { my ($storecfg, $storeid, $vmid, $fmt, $arch, $efidisk, $smm) = @_; my (undef, $ovmf_vars) = get_ovmf_files($arch, $efidisk, $smm); - die "EFI vars default image not found\n" if ! -f $ovmf_vars; + die "EFI vars default image '$ovmf_vars' not found\n" if ! -f $ovmf_vars; my $vars_size_b = -s $ovmf_vars; my $vars_size = PVE::Tools::convert_size($vars_size_b, 'b' => 'kb'); -- 2.30.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] [PATCH qemu-server 2/2] catch missing ovmf file 2022-12-06 13:11 [pve-devel] [PATCH qemu-server 0/2] fix #4378: standardized error for ovmf files Noel Ullreich 2022-12-06 13:11 ` [pve-devel] [PATCH qemu-server 1/2] " Noel Ullreich @ 2022-12-06 13:11 ` Noel Ullreich 2022-12-06 15:55 ` Thomas Lamprecht 1 sibling, 1 reply; 4+ messages in thread From: Noel Ullreich @ 2022-12-06 13:11 UTC (permalink / raw) To: pve-devel check to see if the OVMF_VARS file actually exists. otherwise lines 3666 and 3673 break and give a cryptic error message 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; my ($path, $format); my $read_only_str = ''; -- 2.30.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH qemu-server 2/2] catch missing ovmf file 2022-12-06 13:11 ` [pve-devel] [PATCH qemu-server 2/2] catch missing ovmf file Noel Ullreich @ 2022-12-06 15:55 ` Thomas Lamprecht 0 siblings, 0 replies; 4+ messages in thread From: Thomas Lamprecht @ 2022-12-06 15:55 UTC (permalink / raw) To: Proxmox VE development discussion, Noel Ullreich 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 = ''; ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-12-06 15:55 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-06 13:11 [pve-devel] [PATCH qemu-server 0/2] fix #4378: standardized error for ovmf files Noel Ullreich 2022-12-06 13:11 ` [pve-devel] [PATCH qemu-server 1/2] " Noel Ullreich 2022-12-06 13:11 ` [pve-devel] [PATCH qemu-server 2/2] catch missing ovmf file Noel Ullreich 2022-12-06 15:55 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox