On Fri, Jun 22, 2018 at 03:06:26AM +0200, Alexandre Derumier wrote: > This add a new api to online reload networking configuration > with ifupdown2. > > This work with native ifupdown2 modules, as ifupdown2 have > interface dependency relationships. > > an optional "restart" param is available, for non native modules > like openvswitch.
I'm not sure that's a good idea. In theory `restart` would have worked before as well, but the consequence in this case is *always* that the API call fails due to a connection loss. IMO we should only ever do 'reloads' - they too can kill the connection of course, but restart *always* does. And it would probably look better for the caller (UI) if it returned a task UPID and runs in the background. That way the call can always finish first (the actual execution of the reload command should probably be delayed by a few seconds to be sure), and we can later see the task in the tasklog - if it works. This would probably make it easier to handle on the web UI side. > --- > PVE/API2/Network.pm | 65 > +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 65 insertions(+) > > diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm > index 92256863..3fb71617 100644 > --- a/PVE/API2/Network.pm > +++ b/PVE/API2/Network.pm > @@ -471,6 +471,71 @@ __PACKAGE__->register_method({ > }}); > > __PACKAGE__->register_method({ > + name => 'reload_network_config', > + path => '', > + method => 'PUT', > + permissions => { > + check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]], > + }, > + description => "Reload network configuration", > + protected => 1, > + proxyto => 'node', > + parameters => { > + additionalProperties => 0, > + properties => { > + node => get_standard_option('pve-node'), > + restart => { > + type => 'boolean', > + description => "restart networking.", > + optional => 1, > + } > + }, > + }, > + returns => { type => "null" }, > + code => sub { > + > + my ($param) = @_; > + > + my $action = $param->{restart} ? "restart" : "reload"; > + my $current_config_file = "/etc/network/interfaces"; > + my $new_config_file = "/etc/network/interfaces.new"; > + > + raise_param_exc({ config => "you need ifupdown2 to $action networking" > }) if !-e '/usr/share/ifupdown2'; > + raise_param_exc({ config => "no new network config to apply" }) if !-e > $new_config_file; > + > + my $tmp = PVE::INotify::read_file('interfaces', 1); > + my $config = $tmp->{data}; > + my $changes = $tmp->{changes}; > + > + raise_param_exc({ config => "no changes detected" }) if !$changes; > + > + my $ovs_changes = undef; > + my $bridges_delete = {}; > + my @lines = split(/\n/, $changes); > + foreach my $line (@lines) { > + if($line =~ m/^\-iface\s(vmbr(\S+))/) { > + $bridges_delete->{$1} = 1; > + } elsif ($line =~ m/ovs_type/) { > + $ovs_changes = 1; > + } > + } > + > + raise_param_exc({ config => "reloading config with ovs changes is not > possible currently, please use restart" }) > + if $ovs_changes && !$param->{restart}; > + > + #fixme : restart : check if vm are running on bridge, and try to > reattach them ? > + raise_param_exc({ config => "restart config with bridge delete is not > possible currently, please use reload" }) > + if keys %$bridges_delete && $param->{restart}; > + > + PVE::Tools::file_copy($new_config_file, $current_config_file); > + unlink $new_config_file; > + > + PVE::Tools::run_command(['systemctl', $action, 'networking']); > + > + return undef; > + }}); > + > +__PACKAGE__->register_method({ > name => 'delete_network', > path => '{iface}', > method => 'DELETE', > -- > 2.11.0 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel