Re: [pve-network v2 1/4] vnets: introduce 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:
>> Add 'get_subnets_with_config' to return the subnets of a vnet with
>> certain subnet configuration and integrate into the current
>> 'get_subnets'.
>> 
>> This allows to keep the original behaviour of 'get_subnets' --
>> retrieving subnet configurations (retreiving it either from the
>> running sdn-config, or from the 'subnets.cfg' config file) when it's
>> not available to the caller -- without doubling doubling up the code
>> to to loop through the subnet ids to handle the other case (subnet
>> configuration already present at the callsite).
>> 
>> Suggested-by: Gabriel Goller <g.goller@proxmox.com>
>> Suggested-by: Hannes Laimer <h.laimer@proxmox.com>
>> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
>> ---
>>  src/PVE/Network/SDN/Vnets.pm | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
>> index c327a4b..aa2e842 100644
>> --- a/src/PVE/Network/SDN/Vnets.pm
>> +++ b/src/PVE/Network/SDN/Vnets.pm
>> @@ -70,9 +70,15 @@ sub get_vnet {
>>  sub get_subnets {
>>      my ($vnetid, $running) = @_;
>>  
>> -    my $subnets = undef;
>>      my $subnets_cfg = PVE::Network::SDN::Subnets::config($running);
>> +    return get_subnets_with_config($vnetid, $subnets_cfg);
>> +
>> +}
>>  
>> +sub get_subnets_with_config {
>> +    my ($vnetid, $subnets_cfg) = @_;
>> +
>
> we should probably `die` here if the passed cfg is undef, for the
> deletion `ids` will be empty, and an empty conf will be generated,
> that's fine. but we dont want undef here..

I guess we could possibly also fall back to re-reading
`/etc/pve/sdn/subnets.cfg` in case anything got lost in transmission, or
in a disaster-situation, where the file got lost alltogether, fall back
to get the values from `.running-config` and issue a warning instead of
`die`-ing here. I need to wrap my head around it, but I'm not sure if
this is a good place to `die`.

> on that note, the guard in
> Dhcp.pm should maybe be updated
>

I've sent a separate answer regarding the patch in Dhcp.pm -- in a
nutshell, we could possibly just skip that one (also, because it's not
really related to the dry-run topic).


>> +    my $subnets = undef;
>>      foreach my $subnetid (sort keys %{ $subnets_cfg->{ids} }) {
>>          my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets_cfg, $subnetid);
>>          next if !$subnet->{vnet} || ($vnetid && $subnet->{vnet} ne $vnetid);
>> -- 
>> 2.47.3
>> 
>>