I'd prefer a commit title like "fix #3711: make removal of VM possible
if storage doesn't exit anymore" and add a brief rationale to the commit
message. The current title does not convey any information about the "why".
Am 20.05.25 um 11:08 schrieb Michael Köppl:
> Co-authored-by: Stefan Hrdlicka
As already, mentioned, needs to be adapted.
> Signed-off-by: Michael Köppl <m.koe...@proxmox.com>
With the comments addressed
Reviewed-by: Fiona Ebner <f.eb...@proxmox.com>
> ---
> PVE/QemuServer.pm | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index e02bf7d4..68bbf4ce 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2132,7 +2132,8 @@ sub destroy_vm {
> return if !$volid || $volid =~ m|^/|;
> return if $volids->{$volid};
>
> - my ($path, $owner) = PVE::Storage::path($storecfg, $volid);
> + my ($path, $owner) = eval { PVE::Storage::path($storecfg, $volid) };
> + log_warn("failed to get path and owner of volume '$volid': $@\n") if $@;
No need for the trailing "\n". In fact, it will be duplicate if $@
already contains it. Semi-related note: log_warn() will add a trailing
newline if the passed-in message doesn't contain it.
> return if !$path || !$owner || ($owner != $vmid);
>
> $volids->{$volid} = 1;
There are two warnings further below in the function when vdisk_free()
fails, please adapt those to use log_warn() too for consistency and
better visibility for users. Should be its own patch.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
