Re: [pve-devel] [PATCH v2 pve-manager] API2 : Network : add network config reload

2018-09-24 Thread Alexandre DERUMIER
>>Any idea why json output is not supported? 

seem to work with -o instead -t:

ifquery -a -o json


  -o {native,json}, --format {native,json}
 interface display format


-t seem to be for input format.



BTW,
it seem possible to compare running vs /etc/network/interfaces

ifquery --running -a
ifquery -a



- Mail original -
De: "dietmar" 
À: "aderumier" 
Cc: "pve-devel" 
Envoyé: Lundi 24 Septembre 2018 11:27:14
Objet: Re: [pve-devel] [PATCH v2 pve-manager] API2 : Network : add network 
config reload

> I'm not sure if it's possible to try to reload directly 
> /etc/network/interfaces.new, then if it's ok, overwrite 
> /etc/network/interfaces. I'll look at this. 

Thanks. 

Unrelated topic, but I get the following with ifquery: 

# ifquery -a -t json 
error: No JSON object could be decoded 

Any idea why json output is not supported? 

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH v2 pve-manager] API2 : Network : add network config reload

2018-09-24 Thread Dietmar Maurer
> I'm not sure if it's possible to try to reload directly 
> /etc/network/interfaces.new, then if it's ok, overwrite 
> /etc/network/interfaces. I'll look at this.

Thanks.

Unrelated topic, but I get the following with ifquery:

# ifquery -a -t json
error: No JSON object could be decoded

Any idea why json output is not supported?

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH v2 pve-manager] API2 : Network : add network config reload

2018-09-24 Thread Alexandre DERUMIER
>>This looks fragile. Maybe we should first 'reload', and only rewrite if 
>>reload is successful? Even better, a 'reload' should return the applied 
>>configuration, and we only commit that? 

It's possible to test the syntax of configuration with

"ifreload -a -s"



ifupdown2 reload should work for almost 99% of cases, only interface options 
not able to be change online in kernel can give error. (mostly some vxlan 
options from what I've seen).
maybe for theses known exceptions (we can catch them with ifreload -a, "error: 
interface: "), then force ifdown/ifup interfaces theses interfaces?

I'm not sure if it's possible to try to reload directly 
/etc/network/interfaces.new, then if it's ok, overwrite 
/etc/network/interfaces. I'll look at this.





- Mail original -
De: "dietmar" 
À: "pve-devel" , "aderumier" 
Envoyé: Lundi 24 Septembre 2018 09:31:55
Objet: Re: [pve-devel] [PATCH v2 pve-manager] API2 : Network : add network 
config reload

> + raise_param_exc({ config => "reloading config with ovs changes is not 
> possible currently\n" }) 
> + if $ovs_changes && !$param->{restart}; 
> + 
> + foreach my $bridge (keys %$bridges_delete) { 
> + 
> + my (undef, $interface) = dir_glob_regex("/sys/class/net/$bridge/brif", 
> '(tap|veth|fwpr).*'); 
> + raise_param_exc({ config => "bridge deletion is not possible currently if 
> vm or ct are running on this bridge\n" }) 
> + if defined($interface); 
> + } 
> + 
> + PVE::Tools::file_copy($new_config_file, $current_config_file); 
> + unlink $new_config_file; 
> + 
> + my $worker = sub { 
> + PVE::Tools::run_command(['systemctl', 'reload', 'networking']); 
> + }; 
> + return $rpcenv->fork_worker('srvreload', 'networking', $authuser, $worker); 
> + }}); 

This looks fragile. Maybe we should first 'reload', and only rewrite if reload 
is successful? Even better, a 'reload' should return the applied configuration, 
and we only commit that? 

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH v2 pve-manager] API2 : Network : add network config reload

2018-09-24 Thread Dietmar Maurer

> + raise_param_exc({ config => "reloading config with ovs changes is not 
> possible currently\n" }) 
> + if $ovs_changes && !$param->{restart}; 
> + 
> + foreach my $bridge (keys %$bridges_delete) { 
> + 
> + my (undef, $interface) = dir_glob_regex("/sys/class/net/$bridge/brif", 
> '(tap|veth|fwpr).*'); 
> + raise_param_exc({ config => "bridge deletion is not possible currently if 
> vm or ct are running on this bridge\n" }) 
> + if defined($interface); 
> + } 
> + 
> + PVE::Tools::file_copy($new_config_file, $current_config_file); 
> + unlink $new_config_file; 
> + 
> + my $worker = sub { 
> + PVE::Tools::run_command(['systemctl', 'reload', 'networking']); 
> + }; 
> + return $rpcenv->fork_worker('srvreload', 'networking', $authuser, $worker); 
> + }}); 

This looks fragile. Maybe we should first 'reload', and only rewrite if reload 
is successful? Even better, a 'reload' should return the applied configuration, 
and we only commit that?

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH v2 pve-manager] API2 : Network : add network config reload

2018-09-23 Thread Alexandre DERUMIER
Hi,

seem to be lost in the mailing, can somebody review this V2 ?


- Mail original -
De: "aderumier" 
À: "pve-devel" 
Cc: "aderumier" 
Envoyé: Mercredi 27 Juin 2018 04:53:57
Objet: [PATCH v2 pve-manager] API2 : Network : add network config reload

changelog: 

- remove restart option 
- check if vm|ct are running on a bridge delete 
- run the networking service reload in a task 


This add a new api to online reload networking configuration 
with ifupdown2. 

This work with native ifupdown2 modules, as ifupdown2 have 
interface dependency relationships. 

--- 
PVE/API2/Network.pm | 69 - 
1 file changed, 68 insertions(+), 1 deletion(-) 

diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm 
index c0ae1df3..9475396f 100644 
--- a/PVE/API2/Network.pm 
+++ b/PVE/API2/Network.pm 
@@ -4,7 +4,7 @@ use strict; 
use warnings; 

use Net::IP qw(:PROC); 
-use PVE::Tools qw(extract_param); 
+use PVE::Tools qw(extract_param dir_glob_regex); 
use PVE::SafeSyslog; 
use PVE::INotify; 
use PVE::Exception qw(raise_param_exc); 
@@ -477,6 +477,73 @@ __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'), 
+ }, 
+ }, 
+ returns => { type => 'string' }, 
+ code => sub { 
+ 
+ my ($param) = @_; 
+ 
+ my $rpcenv = PVE::RPCEnvironment::get(); 
+ 
+ my $authuser = $rpcenv->get_user(); 
+ 
+ my $current_config_file = "/etc/network/interfaces"; 
+ my $new_config_file = "/etc/network/interfaces.new"; 
+ 
+ raise_param_exc({ config => "you need ifupdown2 to reload 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\n" }) 
+ if $ovs_changes && !$param->{restart}; 
+ 
+ foreach my $bridge (keys %$bridges_delete) { 
+ 
+ my (undef, $interface) = dir_glob_regex("/sys/class/net/$bridge/brif", 
'(tap|veth|fwpr).*'); 
+ raise_param_exc({ config => "bridge deletion is not possible currently if vm 
or ct are running on this bridge\n" }) 
+ if defined($interface); 
+ } 
+ 
+ PVE::Tools::file_copy($new_config_file, $current_config_file); 
+ unlink $new_config_file; 
+ 
+ my $worker = sub { 
+ PVE::Tools::run_command(['systemctl', 'reload', 'networking']); 
+ }; 
+ return $rpcenv->fork_worker('srvreload', 'networking', $authuser, $worker); 
+ }}); 
+ 
+__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