improve concurrent update handling

compute digest per section.

diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm
index 10ad06253949b8c6a9e98411762c994c5e69a5ed..8d575da5e85985d2b75cca6d86629dd91c520752 100644 (file)
--- a/src/PVE/API2/Firewall/Cluster.pm
+++ b/src/PVE/API2/Firewall/Cluster.pm
@@ -85,9 +85,7 @@ __PACKAGE__->register_method({
 
        my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
 
-       my $options = $cluster_conf->{options};
-
-       return $options;
+       return PVE::Firewall::copy_opject_with_digest($cluster_conf->{options});
     }});
 
 my $option_properties = {
@@ -121,6 +119,7 @@ __PACKAGE__->register_method({
                description => "A list of settings you want to delete.",
                optional => 1,
            },
+           digest => get_standard_option('pve-config-digest'),
        }),
     },
     returns => { type => "null" },
@@ -129,6 +128,9 @@ __PACKAGE__->register_method({
 
        my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
 
+       my (undef, $digest) = PVE::Firewall::copy_opject_with_digest($cluster_conf->{options});
+       PVE::Tools::assert_if_modified($digest, $param->{digest});
+
        if ($param->{delete}) {
            foreach my $opt (PVE::Tools::split_list($param->{delete})) {
                raise_param_exc({ delete => "no such option '$opt'" }) 

diff --git a/src/PVE/API2/Firewall/Groups.pm b/src/PVE/API2/Firewall/Groups.pm
index 5577d43321ad951879e998dbc0d10fe5b958a007..c5fdc8b54c5f24148322e264c42067f7aa684ea6 100644 (file)
--- a/src/PVE/API2/Firewall/Groups.pm
+++ b/src/PVE/API2/Firewall/Groups.pm
@@ -12,6 +12,25 @@ use Data::Dumper; # fixme: remove
 
 use base qw(PVE::RESTHandler);
 
+my $get_security_group_list = sub {
+    my ($cluster_conf) = @_;
+
+    my $res = [];
+    foreach my $group (keys %{$cluster_conf->{groups}}) {
+       my $data = { 
+           name => $group,
+       };
+       if (my $comment = $cluster_conf->{group_comments}->{$group}) {
+           $data->{comment} = $comment;
+       }
+       push @$res, $data;
+    }
+
+    my ($list, $digest) = PVE::Firewall::copy_list_with_digest($res);
+
+    return wantarray ? ($list, $digest) : $list;
+};
+
 __PACKAGE__->register_method({
     name => 'list_security_groups',
     path => '',
@@ -40,22 +59,7 @@ __PACKAGE__->register_method({
 
        my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
 
-       my $digest = $cluster_conf->{digest};
-
-       my $res = [];
-       foreach my $group (keys %{$cluster_conf->{groups}}) {
-           my $data = { 
-               name => $group,
-               digest => $digest,
-               count => scalar(@{$cluster_conf->{groups}->{$group}}) 
-           };
-           if (my $comment = $cluster_conf->{group_comments}->{$group}) {
-               $data->{comment} = $comment;
-           }
-           push @$res, $data;
-       }
-
-       return $res;
+       return &$get_security_group_list($cluster_conf);
     }});
 
 __PACKAGE__->register_method({
@@ -85,20 +89,13 @@ __PACKAGE__->register_method({
 
        my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
 
-       my $digest = $cluster_conf->{digest};
-
-       PVE::Tools::assert_if_modified($digest, $param->{digest});
-
-       if (!$param->{rename}) {        
-           foreach my $name (keys %{$cluster_conf->{groups}}) {
-               raise_param_exc({ name => "Security group '$name' already exists" }) 
-                   if $name eq $param->{name};
-           }
-       }
-
        if ($param->{rename}) {
+           my (undef, $digest) = &$get_security_group_list($cluster_conf);
+           PVE::Tools::assert_if_modified($digest, $param->{digest});
+
            raise_param_exc({ name => "Security group '$param->{rename}' does not exists" }) 
                if !$cluster_conf->{groups}->{$param->{rename}};
+
            my $data = delete $cluster_conf->{groups}->{$param->{rename}};
            $cluster_conf->{groups}->{$param->{name}} = $data;
            if (my $comment = delete $cluster_conf->{group_comments}->{$param->{rename}}) {
@@ -106,6 +103,11 @@ __PACKAGE__->register_method({
            }
            $cluster_conf->{group_comments}->{$param->{name}} = $param->{comment} if defined($param->{comment});
        } else {
+           foreach my $name (keys %{$cluster_conf->{groups}}) {
+               raise_param_exc({ name => "Security group '$name' already exists" }) 
+                   if $name eq $param->{name};
+           }
+
            $cluster_conf->{groups}->{$param->{name}} = [];
            $cluster_conf->{group_comments}->{$param->{name}} = $param->{comment} if defined($param->{comment});
        }
@@ -134,10 +136,11 @@ __PACKAGE__->register_method({
            
        my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
 
-       PVE::Tools::assert_if_modified($cluster_conf->{digest}, $param->{digest});
-
        return undef if !$cluster_conf->{groups}->{$param->{name}};
 
+       my (undef, $digest) = &$get_security_group_list($cluster_conf);
+       PVE::Tools::assert_if_modified($digest, $param->{digest});
+
        die "Security group '$param->{name}' is not empty\n" 
            if scalar(@{$cluster_conf->{groups}->{$param->{name}}});
 

diff --git a/src/PVE/API2/Firewall/Host.pm b/src/PVE/API2/Firewall/Host.pm
index 0ebdcceb494baa4688254b6abf82c883ade25a4e..680bc5820a17ff08f3ae0c14a85c73861d73d86e 100644 (file)
--- a/src/PVE/API2/Firewall/Host.pm
+++ b/src/PVE/API2/Firewall/Host.pm
@@ -68,13 +68,7 @@ __PACKAGE__->register_method({
 
        my $hostfw_conf = PVE::Firewall::load_hostfw_conf();
 
-       my $options = $hostfw_conf->{options} || {};
-
-       my $digest = $hostfw_conf->{digest};
-
-       $options->{digest} = $digest;
-
-       return $options;
+       return PVE::Firewall::copy_opject_with_digest($hostfw_conf->{options});
     }});
 
 1;

diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index 0e73dd4abf83fae1cd1f90749bf24444af5dd06c..c473f130894c505beb5487bba71fc9f755e3c1cd 100644 (file)
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -97,18 +97,7 @@ sub register_get_ipset {
 
            my ($fw_conf, $ipset) = $class->load_config($param);
 
-           my $digest = $fw_conf->{digest};
-
-           my $res = [];
-           foreach my $entry (@$ipset) {
-               my $data = {digest => $digest};
-               foreach my $k (qw(cidr comment nomatch)) {
-                   $data->{$k} = $entry->{$k} if $entry->{$k};
-               }
-               push @$res, $data;
-           }
-
-           return $res;
+           return PVE::Firewall::copy_list_with_digest($ipset);
        }});
 }
 
@@ -180,11 +169,11 @@ sub register_read_ip {
            my ($param) = @_;
 
            my ($fw_conf, $ipset) = $class->load_config($param);
-           my $digest = $fw_conf->{digest};
 
-           foreach my $entry (@$ipset) {
+           my $list = PVE::Firewall::copy_list_with_digest($ipset);
+
+           foreach my $entry (@$list) {
                if ($entry->{cidr} eq $param->{cidr}) {
-                   $entry->{digest} = $digest;
                    return $entry;
                }
            }
@@ -220,7 +209,9 @@ sub register_update_ip {
 
            my ($fw_conf, $ipset) = $class->load_config($param);
 
-           PVE::Tools::assert_if_modified($fw_conf->{digest}, $param->{digest});
+           my (undef, $digest) = PVE::Firewall::copy_list_with_digest($ipset);
+           PVE::Tools::assert_if_modified($digest, $param->{digest});
+           warn "TEST:$digest:$param->{digest}:\n";
 
            foreach my $entry (@$ipset) {
                if($entry->{cidr} eq $param->{cidr}) {
@@ -260,7 +251,8 @@ sub register_delete_ip {
 
            my ($fw_conf, $ipset) = $class->load_config($param);
 
-           PVE::Tools::assert_if_modified($fw_conf->{digest}, $param->{digest});
+           my (undef, $digest) = PVE::Firewall::copy_list_with_digest($ipset);
+           PVE::Tools::assert_if_modified($digest, $param->{digest});
 
            my $new = [];
    
@@ -320,6 +312,25 @@ use PVE::Firewall;
 
 use base qw(PVE::RESTHandler);
 
+my $get_ipset_list = sub {
+    my ($fw_conf) = @_;
+
+    my $res = [];
+    foreach my $name (keys %{$fw_conf->{ipset}}) {
+       my $data = { 
+           name => $name,
+       };
+       if (my $comment = $fw_conf->{ipset_comments}->{$name}) {
+           $data->{comment} = $comment;
+       }
+       push @$res, $data;
+    }
+
+    my ($list, $digest) = PVE::Firewall::copy_list_with_digest($res);
+
+    return wantarray ? ($list, $digest) : $list;
+};
+
 sub register_index {
     my ($class) = @_;
 
@@ -351,22 +362,7 @@ sub register_index {
            
            my $fw_conf = $class->load_config();
 
-           my $digest = $fw_conf->{digest};
-
-           my $res = [];
-           foreach my $name (keys %{$fw_conf->{ipset}}) {
-               my $data = { 
-                   name => $name,
-                   digest => $digest,
-                   count => scalar(@{$fw_conf->{ipset}->{$name}}) 
-               };
-               if (my $comment = $fw_conf->{ipset_comments}->{$name}) {
-                   $data->{comment} = $comment;
-               }
-               push @$res, $data;
-           }
-
-           return $res;
+           return &$get_ipset_list($fw_conf); 
        }});
 }
 
@@ -400,27 +396,25 @@ sub register_create {
            
            my $fw_conf = $class->load_config();
 
-           my $digest = $fw_conf->{digest};
-
-           PVE::Tools::assert_if_modified($digest, $param->{digest});
-
-           if (!$param->{rename}) {
-               foreach my $name (keys %{$fw_conf->{ipset}}) {
-                   raise_param_exc({ name => "IPSet '$name' already exists" }) 
-                       if $name eq $param->{name};
-               }
-           }
-
            if ($param->{rename}) {
+               my (undef, $digest) = &$get_ipset_list($fw_conf);
+               PVE::Tools::assert_if_modified($digest, $param->{digest});
+
                raise_param_exc({ name => "IPSet '$param->{rename}' does not exists" }) 
                    if !$fw_conf->{ipset}->{$param->{rename}};
+
                my $data = delete $fw_conf->{ipset}->{$param->{rename}};
                $fw_conf->{ipset}->{$param->{name}} = $data;
                if (my $comment = delete $fw_conf->{ipset_comments}->{$param->{rename}}) {
                    $fw_conf->{ipset_comments}->{$param->{name}} = $comment;
                }
                $fw_conf->{ipset_comments}->{$param->{name}} = $param->{comment} if defined($param->{comment});
-           } else {
+           } else { 
+               foreach my $name (keys %{$fw_conf->{ipset}}) {
+                   raise_param_exc({ name => "IPSet '$name' already exists" }) 
+                       if $name eq $param->{name};
+               }
+
                $fw_conf->{ipset}->{$param->{name}} = [];
                $fw_conf->{ipset_comments}->{$param->{name}} = $param->{comment} if defined($param->{comment});
            }
@@ -453,10 +447,11 @@ sub register_delete {
            
            my $fw_conf = $class->load_config();
 
-           PVE::Tools::assert_if_modified($fw_conf->{digest}, $param->{digest});
-
            return undef if !$fw_conf->{ipset}->{$param->{name}};
 
+           my (undef, $digest) = &$get_ipset_list($fw_conf);
+           PVE::Tools::assert_if_modified($digest, $param->{digest});
+
            die "IPSet '$param->{name}' is not empty\n" 
                if scalar(@{$fw_conf->{ipset}->{$param->{name}}});
 

diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
index 0054dfca60105953e1ad1f77d10930d70da38fd1..65fe8a65e2be48d56beaaa379d09409951553f7b 100644 (file)
--- a/src/PVE/API2/Firewall/Rules.pm
+++ b/src/PVE/API2/Firewall/Rules.pm
@@ -82,16 +82,14 @@ sub register_get_rules {
 
            my ($fw_conf, $rules) = $class->load_config($param);
 
-           my $digest = $fw_conf->{digest};
-
-           my $res = [];
+           my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules);
 
            my $ind = 0;
-           foreach my $rule (@$rules) {
-               push @$res, PVE::Firewall::cleanup_fw_rule($rule, $digest, $ind++);
+           foreach my $rule (@$list) {
+               $rule->{pos} = $ind++;
            }
 
-           return $res;
+           return $list;
        }});
 }
 
@@ -124,13 +122,14 @@ sub register_get_rule {
 
            my ($fw_conf, $rules) = $class->load_config($param);
 
-           my $digest = $fw_conf->{digest};
+           my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules);
        
-           die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
+           die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$list);
        
-           my $rule = $rules->[$param->{pos}];
-           
-           return PVE::Firewall::cleanup_fw_rule($rule, $digest, $param->{pos});
+           my $rule = $list->[$param->{pos}];
+           $rule->{pos} = $param->{pos};
+
+           return $rule;
        }});
 }
 
@@ -212,7 +211,8 @@ sub register_update_rule {
 
            my ($fw_conf, $rules) = $class->load_config($param);
 
-           PVE::Tools::assert_if_modified($fw_conf->{digest}, $param->{digest});
+           my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
+           PVE::Tools::assert_if_modified($digest, $param->{digest});
 
            die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
        
@@ -274,7 +274,8 @@ sub register_delete_rule {
 
            my ($fw_conf, $rules) = $class->load_config($param);
 
-           PVE::Tools::assert_if_modified($fw_conf->{digest}, $param->{digest});
+           my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
+           PVE::Tools::assert_if_modified($digest, $param->{digest});
        
            die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
        

diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm
index 3d39978fc3b91ff6b563f1aeccd35e61f0666a72..b143a39c19bc61769c19a50638e5711346ae5548 100644 (file)
--- a/src/PVE/API2/Firewall/VM.pm
+++ b/src/PVE/API2/Firewall/VM.pm
@@ -77,13 +77,8 @@ __PACKAGE__->register_method({
 
        my $vmfw_conf = PVE::Firewall::load_vmfw_conf($vmid);
 
-       my $options = $vmfw_conf->{options} || {};
+       return PVE::Firewall::copy_opject_with_digest($vmfw_conf->{options});
 
-       my $digest = $vmfw_conf->{digest};
-
-       $options->{digest} = $digest;
-
-       return $options;
     }});
 
 1;

diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
index 93e53f5bed2374217515a857fc9770718268e6cc..ca57a2437e8b8f87c36224a1416c23d0c7bbddea 100644 (file)
--- a/src/PVE/Firewall.pm
+++ b/src/PVE/Firewall.pm
@@ -784,6 +784,50 @@ sub pve_fw_verify_protocol_spec {
 
 # helper function for API
 
+sub copy_opject_with_digest {
+    my ($object) = @_;
+
+    my $sha = Digest::SHA->new('sha1');
+
+    my $res = {};
+    foreach my $k (sort keys %$object) {
+       my $v = $object->{$k};
+       $res->{$k} = $v;
+       $sha->add($k, ':', $v, "\n");
+    }
+
+    my $digest = $sha->b64digest;
+
+    $res->{digest} = $digest;
+
+    return wantarray ? ($res, $digest) : $res;
+}
+
+sub copy_list_with_digest {
+    my ($list) = @_;
+
+    my $sha = Digest::SHA->new('sha1');
+
+    my $res = [];
+    foreach my $entry (@$list) {
+       my $data = {};
+       foreach my $k (sort keys %$entry) {
+           my $v = $entry->{$k};
+           $data->{$k} = $v;
+           $sha->add($k, ':', $v, "\n");
+       }
+       push @$res, $data;
+    }
+
+    my $digest = $sha->b64digest;
+
+    foreach my $entry (@$res) {
+       $entry->{digest} = $digest;
+    }
+
+    return wantarray ? ($res, $digest) : $res;
+}
+
 my $rule_properties = {
     pos => {
        description => "Update rule at position <pos>.",
@@ -841,23 +885,6 @@ my $rule_properties = {
     },
 };
 
-sub cleanup_fw_rule {
-    my ($rule, $digest, $pos) = @_;
-
-    my $r = {};
-
-    foreach my $k (keys %$rule) {
-       next if !$rule_properties->{$k};
-       my $v = $rule->{$k};
-       next if !defined($v);
-       $r->{$k} = $v;
-       $r->{digest} = $digest;
-       $r->{pos} = $pos;
-    }
-
-    return $r;
-}
-
 sub add_rule_properties {
     my ($properties) = @_;
 
@@ -1864,11 +1891,7 @@ sub parse_vm_fw_rules {
 
     my $section;
 
-    my $digest = Digest::SHA->new('sha1');
-
     while (defined(my $line = <$fh>)) {
-       $digest->add($line);
-
        next if $line =~ m/^#/;
        next if $line =~ m/^\s*$/;
 
@@ -1906,8 +1929,6 @@ sub parse_vm_fw_rules {
        push @{$res->{$section}}, $rule;
     }
 
-    $res->{digest} = $digest->b64digest;
-
     return $res;
 }
 
@@ -1918,11 +1939,7 @@ sub parse_host_fw_rules {
 
     my $section;
 
-    my $digest = Digest::SHA->new('sha1');
-
     while (defined(my $line = <$fh>)) {
-       $digest->add($line);
-
        next if $line =~ m/^#/;
        next if $line =~ m/^\s*$/;
 
@@ -1960,8 +1977,6 @@ sub parse_host_fw_rules {
        push @{$res->{$section}}, $rule;
     }
 
-    $res->{digest} = $digest->b64digest;
-
     return $res;
 }
 
@@ -1980,11 +1995,7 @@ sub parse_cluster_fw_rules {
        ipset_comments => {}, 
     };
 
-    my $digest = Digest::SHA->new('sha1');
-
     while (defined(my $line = <$fh>)) {
-       $digest->add($line);
-
        next if $line =~ m/^#/;
        next if $line =~ m/^\s*$/;
 
@@ -2072,7 +2083,6 @@ sub parse_cluster_fw_rules {
        }
     }
 
-    $res->{digest} = $digest->b64digest;
     return $res;
 }