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

[Stoiko Ivanov]

On Mon, Sep 10, 2018 at 08:51:47AM +0200, Thomas Lamprecht wrote:
> 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.
Would be my preferred solution as well from an aesthetic point of view.
Fixup when/if applying would be great! - Thanks

> 
> > 
> >>  	if (!$disk_ids->{$i}) {
> >>  	    return "$prefix-$vmid-disk-$i$suffix";
> >>  	}
> >> -- 
> >> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel