Re: [pve-network v2 1/4] vnets: introduce get_subnets_with_config

[Hannes Laimer <h.laimer@proxmox.com>]

On 2026-08-31 16:42, Daniel Herzig wrote:
> 
> 
> 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`.
> 

not really, at least i don't think we should. the premis of this sub is,
'give me a config, and ill get you its subnets', the assertion here is
'you give me a config'. this should not go and fetch its own..

> > 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
> >> 
> >>