Re: [PATCH storage v9 01/10] fix #3711: vdisk_free: print warning if storage does not exist

[Fiona Ebner <f.ebner@proxmox.com>]

Am 29.06.26 um 4:05 PM schrieb Michael Köppl:
> Removing a guest or one of its volumes should not fail when the volume's
> backing storage does not exist in the configuration. Instead of dying,
> warn and return so the removal can proceed. From PVE's point of view,
> the volume is gone anyway. Pass the noerr flag to storage_config() so
> this case is actually reached.
> 
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
>  src/PVE/Storage.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index 64ea9dad..55462ff6 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -1187,7 +1187,12 @@ sub vdisk_free {
>      my ($cfg, $volid) = @_;
>  
>      my ($storeid, $volname) = parse_volume_id($volid);
> -    my $scfg = storage_config($cfg, $storeid);
> +    my $scfg = storage_config($cfg, $storeid, 1);
> +    if (!$scfg) {
> +        log_warn("storage '$storeid' does not exist, volume '$volid' could not be freed");
> +        return;
> +    }
> +
>      my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>  
>      activate_storage($cfg, $storeid);

Not sure about this one. Note that vdisk_free() is also used for the
DELETE API, so the volume ID can also be user input and not something
generated by our stack. While the API call still returns an error code
for an invalid volume ID, that's more by chance, because of an earlier
call to path(). Failing does seem more natural and callers can decide
what they want to do. This change is not required for the rest of the
series to work or is it?