[pve-devel] [PATCH pve-firewall 1/3] don't update if /etc/pve is not mounted

[Alexandre DERUMIER]

>>but actually you still have a race here, what if the pmxcfs gets unmounted after the 
>>check above but before the read below. The possible correct, and relative simple, 
>>thing to do would be to transform the FW configs residing in /etc/pve to observed 
>>files which then can be requested over IPCC via the cfs_{read,write}_file methods. 

>>Those then use the IPCC connection to get the content (cached) and if a restart 
>>happens I introduced some logic to detect restarts[0] from inside an IPCC call and 
>>if the restart is faster than 5 seconds the reader/writer won't even notice that 
>>it got restarted. 

Do you have an example ? I really don't known how to implement that.


----- Mail original -----
De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com>
Envoyé: Mercredi 9 Janvier 2019 15:15:23
Objet: Re: [pve-devel] [PATCH pve-firewall 1/3] don't update if /etc/pve is not mounted

On 1/9/19 2:56 PM, Alexandre Derumier wrote: 
> --- 
> src/PVE/Firewall.pm | 3 +++ 
> 1 file changed, 3 insertions(+) 
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm 
> index 39f79d4..71327b0 100644 
> --- a/src/PVE/Firewall.pm 
> +++ b/src/PVE/Firewall.pm 
> @@ -4186,6 +4186,9 @@ sub init { 
> sub update { 
> my $code = sub { 
> 
> + eval { PVE::Cluster::check_cfs_is_mounted() }; 

you could use the $noerr param and just do: 

return if !PVE::Cluster::check_cfs_is_mounted(1); 

but actually you still have a race here, what if the pmxcfs gets unmounted after the 
check above but before the read below. The possible correct, and relative simple, 
thing to do would be to transform the FW configs residing in /etc/pve to observed 
files which then can be requested over IPCC via the cfs_{read,write}_file methods. 

Those then use the IPCC connection to get the content (cached) and if a restart 
happens I introduced some logic to detect restarts[0] from inside an IPCC call and 
if the restart is faster than 5 seconds the reader/writer won't even notice that 
it got restarted. 

Besides that, we could then also add cluster wide config file locking for the API 
methods which allow to modify the FW configs. While currently a digest check adds 
some protection against concurrent modifications a (small) problematic time window 
still exists after the digest check and before the new config gets written out. 

[0]: https://git.proxmox.com/?p=pve-cluster.git;a=commitdiff;h=68da707062f1062556926b299bec84de1082e5e6 

> + return if $@; 
> + 
> my $cluster_conf = load_clusterfw_conf(); 
> my $cluster_options = $cluster_conf->{options}; 
> 
>