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 8caa8f15e21eb78884da2c69430adde5465a9959..da91bdc492f27fb4c9ab2fb371e8ddc87aa8e993 100644 (file)
--- 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";