[pve-devel] [PATCH v2 storage 1/3] fix #2467 remove duplicate volumes & tag with correct content type

[Fabian Grünbichler]

On November 20, 2019 11:35 am, Tim Marx wrote:
> The bugfix for #2317 introduced a kind of odd api behavior, where each volume
> was returned twice from our api if a storage has both 'rootdir' & 'images' content
> types enabled. To give the content type of the volume an actual meaning, it is
> now inferred form the associated guest (qemu/lxc) and we now ignore 'rootdir'
> if both types are enabled. At the volume level, there is no option to list volumes
> based on content types, since the volumes do not know what type they are actually
> used for.
> 
> Signed-off-by: Tim Marx <t.marx at proxmox.com>
> ---
> changed in v2:
> * duplicate filtering is now based on actual content type of the volumes
> * if a volume is not classifiable, it will not show up anymore

besides Fabian's comment, this gets a NAK from me as well.

we always listed orphaned volumes on the storage level, and we need to 
keep doing so. just keep the default of 'images', it's the least broken 
way for now. maybe at some point we can re-evaluate whether to encode 
the volume 'type' into the volid somehow, but this needs more careful 
planning than just fixing the listing.

>  PVE/Storage/Plugin.pm | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 88da231..2bf6f0a 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -937,7 +937,7 @@ sub list_volumes {
>      my ($class, $storeid, $scfg, $vmid, $content_types) = @_;
> 
>      my $res = [];
> -
> +    my $vmlist = PVE::Cluster::get_vmlist();
>      foreach my $ct (@$content_types) {
>  	my $data;
> 
> @@ -960,7 +960,24 @@ sub list_volumes {
>  	next if !$data;
> 
>  	foreach my $item (@$data) {
> -	    $item->{content} = $ct;
> +	    if ($ct eq 'images' || $ct eq 'rootdir') {
> +
> +		my $vmtype = $vmlist->{ids}->{$item->{vmid}}->{type};
> +		next if (!defined($vmtype));
> +		if ($vmtype eq 'lxc') {
> +		    $item->{content} = 'rootdir';
> +		}
> +		if ($vmtype eq 'qemu') {
> +		    $item->{content} = 'images';
> +		}
> +		if (!($ct eq $item->{content})) {
> +		    next;
> +		}
> +
> +	    } else {
> +		$item->{content} = $ct;
> +	    }
> +
>  	    push @$res, $item;
>  	}
>      }
> --
> 2.20.1
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>