public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH qemu-server] fix: print error message that the resource mapping entry does not exist
@ 2025-04-09 14:48 Markus Frank
  2025-04-10  7:03 ` Fiona Ebner
  2025-04-10 11:25 ` Fiona Ebner
  0 siblings, 2 replies; 3+ messages in thread
From: Markus Frank @ 2025-04-09 14:48 UTC (permalink / raw)
  To: pve-devel

Currently you get a "Can't use an undefined value..." error because all
get_node_mapping functions return undefined if there is no resource
mapping entry for the id.

Fixes: a52eb3c4e ("check local resources: extend for mapped resources")
Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 PVE/QemuServer.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index ccdceedc..c1584c19 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2512,6 +2512,7 @@ sub check_local_resources {
 	    } elsif ($type eq 'dir') {
 		$entry = PVE::Mapping::Dir::get_node_mapping($dir_map, $id, $node);
 	    }
+	    die "no $type resource mapping for id: $id\n" if !$entry;
 	    if (!scalar($entry->@*)) {
 		push @{$missing_mappings_by_node->{$node}}, $key;
 	    }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] fix: print error message that the resource mapping entry does not exist
  2025-04-09 14:48 [pve-devel] [PATCH qemu-server] fix: print error message that the resource mapping entry does not exist Markus Frank
@ 2025-04-10  7:03 ` Fiona Ebner
  2025-04-10 11:25 ` Fiona Ebner
  1 sibling, 0 replies; 3+ messages in thread
From: Fiona Ebner @ 2025-04-10  7:03 UTC (permalink / raw)
  To: Proxmox VE development discussion, Markus Frank

Am 09.04.25 um 16:48 schrieb Markus Frank:
> Currently you get a "Can't use an undefined value..." error because all
> get_node_mapping functions return undefined if there is no resource
> mapping entry for the id.
> 
> Fixes: a52eb3c4e ("check local resources: extend for mapped resources")
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  PVE/QemuServer.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index ccdceedc..c1584c19 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2512,6 +2512,7 @@ sub check_local_resources {
>  	    } elsif ($type eq 'dir') {
>  		$entry = PVE::Mapping::Dir::get_node_mapping($dir_map, $id, $node);
>  	    }
> +	    die "no $type resource mapping for id: $id\n" if !$entry;
>  	    if (!scalar($entry->@*)) {

Should it really be a die here instead of just adapting the if condition
to also check for definedness first? Because with a die the function
will immediately fail rather then produce the list of missing mappings
by node.

>  		push @{$missing_mappings_by_node->{$node}}, $key;
>  	    }



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH qemu-server] fix: print error message that the resource mapping entry does not exist
  2025-04-09 14:48 [pve-devel] [PATCH qemu-server] fix: print error message that the resource mapping entry does not exist Markus Frank
  2025-04-10  7:03 ` Fiona Ebner
@ 2025-04-10 11:25 ` Fiona Ebner
  1 sibling, 0 replies; 3+ messages in thread
From: Fiona Ebner @ 2025-04-10 11:25 UTC (permalink / raw)
  To: Proxmox VE development discussion, Markus Frank

Am 09.04.25 um 16:48 schrieb Markus Frank:
> Currently you get a "Can't use an undefined value..." error because all
> get_node_mapping functions return undefined if there is no resource
> mapping entry for the id.
> 
> Fixes: a52eb3c4e ("check local resources: extend for mapped resources")
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  PVE/QemuServer.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index ccdceedc..c1584c19 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2512,6 +2512,7 @@ sub check_local_resources {
>  	    } elsif ($type eq 'dir') {
>  		$entry = PVE::Mapping::Dir::get_node_mapping($dir_map, $id, $node);
>  	    }
> +	    die "no $type resource mapping for id: $id\n" if !$entry;

Please also print the $key (as a prefix?), then users will immediately
know where to look. Otherwise, the need to search for where the ID is
used first.

With that added:
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

>  	    if (!scalar($entry->@*)) {
>  		push @{$missing_mappings_by_node->{$node}}, $key;
>  	    }



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-04-10 11:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-04-09 14:48 [pve-devel] [PATCH qemu-server] fix: print error message that the resource mapping entry does not exist Markus Frank
2025-04-10  7:03 ` Fiona Ebner
2025-04-10 11:25 ` Fiona Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal