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:

>> >> ---
>> >>  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..
>
I agree, I'll test with the `die` in place and send a v3 without the
'Dhcp.pm' patch.

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