From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Date: Mon, 10 Sep 2018 08:11:22 +0000 (+0200)
Subject: refactor is_valid_vm_diskname to private simpler sub
X-Git-Url: https://git.proxmox.com/?p=pve-storage.git;a=commitdiff_plain;h=83a40b8d3b9955004222650f12e88a789966c304

refactor is_valid_vm_diskname to private simpler sub

This was newly introduced and is only used once, so having a
wantarray return mechanism, without ever using it or knowing for sure
if this may help with reuse of the method is not ideal.

Make the sub a module private one just returning the vm disk number
or explicit undef. Pass it the $suffix variable, to avoid recomputing
it every time called by out caller's loop.

If there's re-use potential in the future we can actually decide what
makes sense to return.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 8caa8f1..da91bdc 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -525,12 +525,8 @@ sub create_base {
     return $newvolname;
 }
 
-sub is_valid_vm_diskname {
-    my ($disk_name, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
-
-    $vmid = qr/\d+/ if !defined($vmid);
-
-    my $suffix = (defined($fmt) && $add_fmt_suffix) ? ".$fmt" : '';
+my $get_vm_disk_number = sub {
+    my ($disk_name, $scfg, $vmid, $suffix) = @_;
 
     my $type = $scfg->{type};
     my $def = $defaultData->{plugindata}->{$type};
@@ -540,25 +536,26 @@ sub is_valid_vm_diskname {
     $disk_regex = qr/(vm|base|subvol|basevol)-$vmid-disk-(\d+)/
 	if $valid_formats->{subvol};
 
-    if($disk_name =~ m/$disk_regex/){
-	return wantarray ? (1, $2) : 1;
+    if ($disk_name =~ m/$disk_regex/) {
+	return $2;
     }
-}
+
+    return undef;
+};
 
 sub get_next_vm_diskname {
     my ($disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix) = @_;
 
-    my $disk_ids = {};
-    my ($match, $disknum);
-    foreach my $disk (@$disk_list) {
-	($match, $disknum) = is_valid_vm_diskname($disk,  $scfg, $vmid, $fmt, $add_fmt_suffix);
-	$disk_ids->{$disknum} = 1 if $match;
-    }
-
     $fmt //= '';
     my $prefix = ($fmt eq 'subvol') ? 'subvol' : 'vm';
     my $suffix = $add_fmt_suffix ? ".$fmt" : '';
 
+    my $disk_ids = {};
+    foreach my $disk (@$disk_list) {
+	my $disknum = $get_vm_disk_number->($disk, $scfg, $vmid, $suffix);
+	$disk_ids->{$disknum} = 1 if defined($disknum);
+    }
+
     for (my $i = 0; $i < $MAX_VOLUMES_PER_GUEST; $i++) {
 	if (!$disk_ids->{$i}) {
 	    return "$prefix-$vmid-disk-$i$suffix";