[pve-devel] [PATCH qemu-server] do not add unused disk when already as unused on another storage

[Fabian Grünbichler]

On Mon, Nov 13, 2017 at 12:09:11PM +0100, Dominik Csapak wrote:
> when having an unused disk on a storage for which there are multiple
> definitions, we added it again on another storage when that storage
> was alphabetically before the already existing one
> 
> this happens for example when using our automatically generated
> ceph storages: 'pool_ct' and 'pool_vm' and having a vm with
> an unused disk
> 
> with this patch, we also leave the unused disks in the hash
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/QemuServer.pm | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 97f0f9d..1bba38d 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5393,8 +5393,15 @@ sub update_disksize {
>  	}
>      }
>  
> +    # we reuse the used(path) hashes
> +    # for duplicate unused disks
> +    # note: this does not duplicate the hash,
> +    # we only use a different name for clarity
> +    my $referenced = $used;
> +    my $referencedpath = $usedpath;

I am not sure if this isn't more confusing than just renaming the
variable - see below.. (also, 6 extra lines vs. 1 extra comment line
stating "used AND unused disks")

> +
>      # remove 'unusedX' entry if volume is used
> -    foreach my $opt (keys %$conf) {
> +    foreach my $opt (sort keys %$conf) {
>  	next if $opt !~ m/^unused\d+$/;
>  	my $volid = $conf->{$opt};
>  	my $path = $volid_hash->{$volid}->{path} if $volid_hash->{$volid};
> @@ -5402,17 +5409,20 @@ sub update_disksize {
>  	    $changes = 1;
>  	    delete $conf->{$opt};
>  	}
> +
> +	$referenced->{$volid} = 1;
> +	$referencedpath->{$path} = 1 if $path;
>      }

not visible in that hunk, but now this loop checks $used(path) and sets
$referenced(path). so I'd either change the 'used' to 'referenced' in
the check as well, or rename the variables altogether for the whole
method.

also not sure if the 'sort' is really necessary (it only makes a
difference for an already invalid config containing an alias unless I
missed something).

>  
>      foreach my $volid (sort keys %$volid_hash) {
>  	next if $volid =~ m/vm-$vmid-state-/;
> -	next if $used->{$volid};
> +	next if $referenced->{$volid};
>  	my $path = $volid_hash->{$volid}->{path};
>  	next if !$path; # just to be sure
> -	next if $usedpath->{$path};
> +	next if $referencedpath->{$path};
>  	$changes = 1;
>  	PVE::QemuConfig->add_unused_volume($conf, $volid);
> -	$usedpath->{$path} = 1; # avoid to add more than once (aliases)
> +	$referencedpath->{$path} = 1; # avoid to add more than once (aliases)
>      }
>  
>      return $changes;
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel