[pve-devel] [PATCH storage 1/2] vdisk list: only collect images from storages with an appropriate content type

[Thomas Lamprecht]

On 12.03.21 10:50, Fabian Ebner wrote:
> Only these storages are activated in the first place, and it's bad behavior to
> list images when no appropriate content type is not set.
> 
> For example, on VM destruction, this avoids unreferenced images to be deleted
> from a storage with only 'backup' content type set, which is supposedly what
> happened in this[0] forum thread.
> 

Seems to be related to a patch of mine, not that long ago:
https://git.proxmox.com/?p=qemu-server.git;a=commitdiff;h=75854662699b261915289f36e8fbe953c5f77b7c

That flag needs to still be made available in the UI, though

> (Some) callers expect all keys to be present and valid array references in the
> result, so initialization is needed.
> 
> Now, the enabled check is already done by the preceding code for every element
> that is iterated over, and thus isn't needed in the main loop anymore.
> 
> [0]: https://forum.proxmox.com/threads/erasing-all-vm-disks-after-a-failed-vm-migration-task.85068
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
> 
> Why isn't 'iterand' a word?
> 
>  PVE/Storage.pm | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 8ee2c92..18c03ec 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -938,7 +938,7 @@ sub vdisk_list {
>  
>      storage_check_enabled($cfg, $storeid) if ($storeid);
>  
> -    my $res = {};
> +    my $res = { map { $_ => [] } keys %{$ids} };
>  
>      # prepare/activate/refresh all storages
>  
> @@ -964,9 +964,8 @@ sub vdisk_list {
>  
>      activate_storage_list($cfg, $storage_list, $cache);
>  
> -    foreach my $sid (keys %$ids) {
> +    foreach my $sid (@{$storage_list}) {
>  	next if $storeid && $storeid ne $sid;
> -	next if !storage_check_enabled($cfg, $sid, undef, 1);
>  
>  	my $scfg = $ids->{$sid};
>  	my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>