Re: [pve-network v2 4/4] dhcp: make use of get_subnets_with_config

[Daniel Herzig <d.herzig@proxmox.com>]

Hannes Laimer <h.laimer@proxmox.com> writes:

> On 2026-08-31 13:01, Daniel Herzig wrote:
>> At the given point we already have the subnet configuration by hand,
>> so make sure to use it, instead of inherently falling back to
>> re-reading '/etc/pve/sdn/.running-config' via the original
>> 'get_subnets($vnet_id, 1)'.
>> 
>
> before this was `get_subnets($vnetid)`, so it did not take it from
> running, now, it does since `$subnet_cfg` is the running config that has
> the changes commited to it. this is a behaviour change, arguably this is
> more correct here.. currently the node PUT `../network` endpoint also
> calls this generate, that applies still pending configs, which is
> probably a bug tbh
>
> so chnage is good, but the commit message should be updated

Thanks for catching this.

I was scanning callers that have `$subnet_cfg` available at calltime and
admittedly missed that we're calling without `$running=1` here.

So this one here is essentially superflouus, without `$running=1` we're
reading from (already updated) '/etc/pve/sdn/subnets.cfg' file via
`Subnets::config` (without `$running` given) anyway. 

ATM, I'm not sure what's better -- calling the original
`get_subnets($vnetid)` without `$running` should yield the same as
`$subnets` as calling `get_subnets_with_config($vnetid,$subnet_cfg)` at
this point.

For general noise reduction reasons I'd think about dropping this patch
altogether, on the other hand, with it, we'd once less look into the
file, which doesn't really seem to be necessary at this point.


>
>> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
>> ---
>>  src/PVE/Network/SDN/Dhcp.pm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
>> index 65e40d4..1fdec89 100644
>> --- a/src/PVE/Network/SDN/Dhcp.pm
>> +++ b/src/PVE/Network/SDN/Dhcp.pm
>> @@ -94,7 +94,7 @@ sub regenerate_config {
>>              next if $vnet->{zone} ne $zoneid;
>>  
>>              my $config = [];
>> -            my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
>> +            my $subnets = PVE::Network::SDN::Vnets::get_subnets_with_config($vnetid, $subnet_cfg);
>>  
>>              foreach my $subnet_id (sort keys %{$subnets}) {
>>                  my $subnet_config = $subnets->{$subnet_id};
>> -- 
>> 2.47.3
>> 
>> 
>> 
>>