Re: [PATCH pve-firewall v2 04/12] fix #5759: firewall: do not remove chains when host is shutting down

[Stefan Hanreich <s.hanreich@proxmox.com>]

> The assumption until now was that the firewall rules should always be
> cleared when the daemon is shutting down. This is correct when the
> shutdown happens as the result of `systemctl stop`, should however not
> be handled the same way in case of a system shutdown.
> 
> Because of the way pve-firewall is ordered, it comes down before the
> network does. Clearing the rules in the host shutdown case creates a
> window where the network is up without firewall protection.
> 
> When shutting down, check the operational state of the system and only
> remove the chains if the system is _not_ currently shutting down.
> 
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5759
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> Reviewed-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
> Tested-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
>
> diff --git a/src/PVE/Service/pve_firewall.pm b/src/PVE/Service/pve_firewall.pm
> index f9fa1dd..b39fea7 100755
> --- a/src/PVE/Service/pve_firewall.pm
> +++ b/src/PVE/Service/pve_firewall.pm
> @@ -41,6 +41,29 @@ my $updatetime = 10;
>  
>  my $initial_memory_usage;
>  
> +=head3 is_host_shutdown()
> +
> +Query C<systemctl is-system-running>, returning C<1> if the system's running state is C<stopping>,
> +C<0> otherwise.
> +
> +Note that C<1> is also returned if the running state of the system could not be determined, in order
> +to fail closed.
> +
> +=cut
> +
> +sub is_host_shutdown {
> +    my $state = undef;
> +
> +    PVE::Tools::run_command(
> +        ['systemctl', 'is-system-running'],
> +        outfunc => sub { $state = PVE::Tools::trim(shift) },
> +        noerr => 1,
> +    );

While it is an alias, I think newer code should use the dedicated Cmd
module.

Using eval instead of noerr should catch any general errors with 
run_command and allow handling them instead of relying on $state. It'd
also give a better indication of *why* this failed and we can log it.

-- 
Stefan Hanreich <s.hanreich@proxmox.com>