[pve-devel] [PATCH storage v2 2/3] make max number of disks a constant

[Thomas Lamprecht]

On 9/7/18 5:10 PM, Fabian Grünbichler wrote:
> tiny nit inline
> 
> On Fri, Sep 07, 2018 at 03:08:41PM +0200, Stoiko Ivanov wrote:
>> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
>> ---
>>  PVE/Storage/Plugin.pm | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
>> index 32acf69..7516f4e 100644
>> --- a/PVE/Storage/Plugin.pm
>> +++ b/PVE/Storage/Plugin.pm
>> @@ -31,6 +31,8 @@ our @SHARED_STORAGE = (
>>      'zfs',
>>      'drbd');
>>  
>> +our $MAX_VOLUMES_PER_GUEST = 1024;
>> +
>>  cfs_register_file ('storage.cfg',
>>  		   sub { __PACKAGE__->parse_config(@_); },
>>  		   sub { __PACKAGE__->write_config(@_); });
>> @@ -557,7 +559,7 @@ sub get_next_vm_diskname {
>>      my $prefix = ($fmt eq 'subvol') ? 'subvol' : 'vm';
>>      my $suffix = $add_fmt_suffix ? ".$fmt" : '';
>>  
>> -    for (my $i = 1; $i < 100; $i++) {
>> +    for (my $i = 1; $i < $MAX_VOLUMES_PER_GUEST; $i++) {
> 
> but this only allows MAX_VOLUMES_PER_GUEST - 1 volumes since we start
> counting at 1 (which is a bit strange, since almost all the other
> indices in PVE start counting at 0 - scsi0, mp0, net0, ... ). so I'd
> suggest either starting at 0 as well, or comparing with <=

I'd fix it up and go the "allow 0 way", more commonly used in computer
science, and as you said also more common in PVE - if there are no
objections.

> 
>>  	if (!$disk_ids->{$i}) {
>>  	    return "$prefix-$vmid-disk-$i$suffix";
>>  	}
>> -- 
>> 2.11.0