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

[Thomas Lamprecht]

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