[pve-devel] applied: [PATCH pve-firewall 1/1] move clone_vmfw_conf && remove_vmfw_conf to a Helpers

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

applied, but had to do some follow ups (see below)

Am 29/06/2022 um 11:08 schrieb Alexandre Derumier:
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  src/PVE/Firewall.pm         | 25 ----------------------
>  src/PVE/Firewall/Helpers.pm | 41 +++++++++++++++++++++++++++++++++++++
>  src/PVE/Firewall/Makefile   | 12 +++++++++++
>  src/PVE/Makefile            |  2 ++
>  4 files changed, 55 insertions(+), 25 deletions(-)
>  create mode 100644 src/PVE/Firewall/Helpers.pm
>  create mode 100644 src/PVE/Firewall/Makefile
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 71746d2..e6cf4cd 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -3340,31 +3340,6 @@ sub save_vmfw_conf {
>      }
>  }
>  
> -sub remove_vmfw_conf {
> -    my ($vmid) = @_;
> -
> -    my $vmfw_conffile = "$pvefw_conf_dir/$vmid.fw";
> -
> -    unlink $vmfw_conffile;
> -}
> -
> -sub clone_vmfw_conf {
> -    my ($vmid, $newid) = @_;
> -
> -    my $sourcevm_conffile = "$pvefw_conf_dir/$vmid.fw";
> -    my $clonevm_conffile = "$pvefw_conf_dir/$newid.fw";
> -
> -    lock_vmfw_conf($newid, 10, sub {
> -	if (-f $clonevm_conffile) {
> -	    unlink $clonevm_conffile;
> -	}
> -	if (-f $sourcevm_conffile) {
> -	    my $data = PVE::Tools::file_get_contents($sourcevm_conffile);
> -	    PVE::Tools::file_set_contents($clonevm_conffile, $data);
> -	}
> -    });
> -}

keeping above as skeleton allows to avoid a breakage on older qemu-server/pve-container
so I did that in a followup.re


> -
>  sub read_vm_firewall_configs {
>      my ($cluster_conf, $vmdata, $dir) = @_;
>  
> diff --git a/src/PVE/Firewall/Helpers.pm b/src/PVE/Firewall/Helpers.pm
> new file mode 100644
> index 0000000..3112ebc
> --- /dev/null
> +++ b/src/PVE/Firewall/Helpers.pm
> @@ -0,0 +1,41 @@
> +package PVE::Firewall::Helpers;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Tools qw(file_get_contents file_set_contents);
> +
> +use base 'Exporter';
> +our @EXPORT_OK = qw(
> +remove_vmfw_conf
> +clone_vmfw_conf
> +);
> +
> +my $pvefw_conf_dir = "/etc/pve/firewall";
> +
> +sub remove_vmfw_conf {
> +    my ($vmid) = @_;
> +
> +    my $vmfw_conffile = "$pvefw_conf_dir/$vmid.fw";
> +
> +    unlink $vmfw_conffile;
> +}
> +
> +sub clone_vmfw_conf {
> +    my ($vmid, $newid) = @_;
> +
> +    my $sourcevm_conffile = "$pvefw_conf_dir/$vmid.fw";
> +    my $clonevm_conffile = "$pvefw_conf_dir/$newid.fw";
> +
> +    lock_vmfw_conf($newid, 10, sub {

this was not in scope here so it only worked by luck, if at all?

As it only depends on PVE::Cluster, which is probably in the use paths
in most more involved/API facing modules anyway, I moved it over in a follow up.