Re: [PATCH guest-common v6 04/18] add module to track previously used guest IDs

[Fiona Ebner <f.ebner@proxmox.com>]

Am 01.10.26 um 2:04 PM schrieb Fiona Ebner:
> Am 24.09.26 um 6:17 PM schrieb Michael Köppl:
>> diff --git a/src/Makefile b/src/Makefile
>> index 030e7f7..ceba738 100644
>> --- a/src/Makefile
>> +++ b/src/Makefile
>> @@ -14,6 +14,7 @@ install: PVE
>>  	install -m 0644 PVE/Replication.pm ${PERL5DIR}/PVE/
>>  	install -m 0644 PVE/StorageTunnel.pm ${PERL5DIR}/PVE/
>>  	install -m 0644 PVE/Tunnel.pm ${PERL5DIR}/PVE/
>> +	install -m 0644 PVE/GuestID.pm ${PERL5DIR}/PVE/
> 
> Nit: could be ordered alphabetically
> 
>>  	install -d ${PERL5DIR}/PVE/Mapping
>>  	install -m 0644 PVE/Mapping/Dir.pm ${PERL5DIR}/PVE/Mapping/
>>  	install -m 0644 PVE/Mapping/PCI.pm ${PERL5DIR}/PVE/Mapping/
>> diff --git a/src/PVE/GuestID.pm b/src/PVE/GuestID.pm
> 
> ---snip 8<---
> 
>> +my sub insert_id($ranges, $id) {
>> +    my $i = 0;
>> +
>> +    while ($i < @$ranges && $ranges->[$i]->[1] < $id) {
> 
> Style nit: I'd prefer having an explicit scalar()
> 
>> +        $i++;
>> +    }
>> +
>> +    return 0 if $i < @$ranges && $ranges->[$i]->[0] <= $id;
> 
> Same here.
> 
>> +
>> +    splice(@$ranges, $i, 0, [$id, $id]);
>> +
>> +    return 1;
>> +}
>> +
>> +sub register_used_id($id) {
>> +    cfs_lock_file(
>> +        $FILENAME,
>> +        10,
>> +        sub {
>> +            my $ranges = cfs_read_file($FILENAME);
>> +
>> +            return if !insert_id($ranges, $id);
>> +
>> +            cfs_write_file($FILENAME, $ranges);
>> +        },
>> +    );
>> +
> 
> Style nit: I'd avoid the blank to make it clear that it's connected
> (makes it harder for new code to accidentally sneak in in-between).
> 
>> +    if (my $err = $@) {
>> +        my $dc_conf = cfs_read_file('datacenter.cfg');
> 
> Edge case, but we should error out if reading the datacenter config fails.
> 

Sorry, it does already die (there's no eval around it).

>> +
>> +        my $emsg = "unable to record guest ID $id as used";
>> +        die "$emsg - $err" if $dc_conf->{'next-id'}->{unique};
>> +        warn "$emsg - $err";
>> +    }
>> +}
>> +
>> +cfs_register_file($FILENAME, \&parse_id_list, \&write_id_list);
>> +
>> +1;
> 
> 
> 
> 
>