From: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Date: Wed, 29 Apr 2020 08:52:52 +0000 (+0200)
Subject: api: lock configs
X-Git-Url: https://git.proxmox.com/?p=pve-firewall.git;a=commitdiff_plain;h=a38849e6507fd567a3c7d7ff92f7b2bc93841126

api: lock configs

wherever we have a r-m-w cycle.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

diff --git a/src/PVE/API2/Firewall/Aliases.pm b/src/PVE/API2/Firewall/Aliases.pm
index 9ea6f70..33ac669 100644
--- a/src/PVE/API2/Firewall/Aliases.pm
+++ b/src/PVE/API2/Firewall/Aliases.pm
@@ -143,19 +143,23 @@ sub register_create_alias {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($fw_conf, $aliases) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
 
-	    my $name = lc($param->{name});
+		my ($fw_conf, $aliases) = $class->load_config($param);
 
-	    raise_param_exc({ name => "alias '$param->{name}' already exists" })
-		if defined($aliases->{$name});
+		my $name = lc($param->{name});
 
-	    my $data = { name => $param->{name}, cidr => $param->{cidr} };
-	    $data->{comment} = $param->{comment} if $param->{comment};
+		raise_param_exc({ name => "alias '$param->{name}' already exists" })
+		    if defined($aliases->{$name});
 
-	    $aliases->{$name} = $data;
+		my $data = { name => $param->{name}, cidr => $param->{cidr} };
+		$data->{comment} = $param->{comment} if $param->{comment};
 
-	    $class->save_aliases($param, $fw_conf, $aliases);
+		$aliases->{$name} = $data;
+
+		$class->save_aliases($param, $fw_conf, $aliases);
+	    });
 
 	    return undef;
 	}});
@@ -219,35 +223,39 @@ sub register_update_alias {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($fw_conf, $aliases) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
 
-	    my $list = &$aliases_to_list($aliases);
+		my ($fw_conf, $aliases) = $class->load_config($param);
 
-	    my (undef, $digest) = PVE::Firewall::copy_list_with_digest($list);
+		my $list = &$aliases_to_list($aliases);
 
-	    PVE::Tools::assert_if_modified($digest, $param->{digest});
+		my (undef, $digest) = PVE::Firewall::copy_list_with_digest($list);
 
-	    my $name = lc($param->{name});
+		PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-	    raise_param_exc({ name => "no such alias" }) if !$aliases->{$name};
+		my $name = lc($param->{name});
 
-	    my $data = { name => $param->{name}, cidr => $param->{cidr} };
-	    $data->{comment} = $param->{comment} if $param->{comment};
+		raise_param_exc({ name => "no such alias" }) if !$aliases->{$name};
 
-	    $aliases->{$name} = $data;
+		my $data = { name => $param->{name}, cidr => $param->{cidr} };
+		$data->{comment} = $param->{comment} if $param->{comment};
 
-	    my $rename = $param->{rename};
-	    $rename = lc($rename) if $rename;
+		$aliases->{$name} = $data;
 
-	    if ($rename && ($name ne $rename)) {
-		raise_param_exc({ name => "alias '$param->{rename}' already exists" })
-		    if defined($aliases->{$rename});
-		$aliases->{$name}->{name} = $param->{rename};
-		$aliases->{$rename} = $aliases->{$name};
-		delete $aliases->{$name};
-	    }
+		my $rename = $param->{rename};
+		$rename = lc($rename) if $rename;
 
-	    $class->save_aliases($param, $fw_conf, $aliases);
+		if ($rename && ($name ne $rename)) {
+		    raise_param_exc({ name => "alias '$param->{rename}' already exists" })
+			if defined($aliases->{$rename});
+		    $aliases->{$name}->{name} = $param->{rename};
+		    $aliases->{$rename} = $aliases->{$name};
+		    delete $aliases->{$name};
+		}
+
+		$class->save_aliases($param, $fw_conf, $aliases);
+	    });
 
 	    return undef;
 	}});
@@ -276,16 +284,20 @@ sub register_delete_alias {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($fw_conf, $aliases) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
 
-	    my $list = &$aliases_to_list($aliases);
-	    my (undef, $digest) = PVE::Firewall::copy_list_with_digest($list);
-	    PVE::Tools::assert_if_modified($digest, $param->{digest});
+		my ($fw_conf, $aliases) = $class->load_config($param);
 
-	    my $name = lc($param->{name});
-	    delete $aliases->{$name};
+		my $list = &$aliases_to_list($aliases);
+		my (undef, $digest) = PVE::Firewall::copy_list_with_digest($list);
+		PVE::Tools::assert_if_modified($digest, $param->{digest});
+
+		my $name = lc($param->{name});
+		delete $aliases->{$name};
 
-	    $class->save_aliases($param, $fw_conf, $aliases);
+		$class->save_aliases($param, $fw_conf, $aliases);
+	    });
 
 	    return undef;
 	}});
diff --git a/src/PVE/API2/Firewall/Cluster.pm b/src/PVE/API2/Firewall/Cluster.pm
index 0bf8285..c9c3e67 100644
--- a/src/PVE/API2/Firewall/Cluster.pm
+++ b/src/PVE/API2/Firewall/Cluster.pm
@@ -132,29 +132,31 @@ __PACKAGE__->register_method({
     code => sub {
 	my ($param) = @_;
 
-	my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
+	PVE::Firewall::lock_clusterfw_conf(10, sub {
+	    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});
+	    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'" })
-		    if !$option_properties->{$opt};
-		delete $cluster_conf->{options}->{$opt};
+	    if ($param->{delete}) {
+		foreach my $opt (PVE::Tools::split_list($param->{delete})) {
+		    raise_param_exc({ delete => "no such option '$opt'" })
+			if !$option_properties->{$opt};
+		    delete $cluster_conf->{options}->{$opt};
+		}
 	    }
-	}
 
-	if (defined($param->{enable}) && ($param->{enable} > 1)) {
-	    $param->{enable} = time();
-	}
+	    if (defined($param->{enable}) && ($param->{enable} > 1)) {
+		$param->{enable} = time();
+	    }
 
-	foreach my $k (keys %$option_properties) {
-	    next if !defined($param->{$k});
-	    $cluster_conf->{options}->{$k} = $param->{$k};
-	}
+	    foreach my $k (keys %$option_properties) {
+		next if !defined($param->{$k});
+		$cluster_conf->{options}->{$k} = $param->{$k};
+	    }
 
-	PVE::Firewall::save_clusterfw_conf($cluster_conf);
+	    PVE::Firewall::save_clusterfw_conf($cluster_conf);
+	});
 
 	# instant firewall update when using double (anti-lockout) API call
 	# -> not waiting for a firewall update at the first (timestamp enable) set
diff --git a/src/PVE/API2/Firewall/Groups.pm b/src/PVE/API2/Firewall/Groups.pm
index ec0d016..558ba8e 100644
--- a/src/PVE/API2/Firewall/Groups.pm
+++ b/src/PVE/API2/Firewall/Groups.pm
@@ -91,37 +91,39 @@ __PACKAGE__->register_method({
     code => sub {
 	my ($param) = @_;
 
-	my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
+	PVE::Firewall::lock_clusterfw_conf(10, sub {
+	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
 
-	if ($param->{rename}) {
-	    my (undef, $digest) = &$get_security_group_list($cluster_conf);
-	    PVE::Tools::assert_if_modified($digest, $param->{digest});
+	    if ($param->{rename}) {
+		my (undef, $digest) = &$get_security_group_list($cluster_conf);
+		PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-	    raise_param_exc({ group => "Security group '$param->{rename}' does not exist" })
-		if !$cluster_conf->{groups}->{$param->{rename}};
+		raise_param_exc({ group => "Security group '$param->{rename}' does not exist" })
+		    if !$cluster_conf->{groups}->{$param->{rename}};
 
-	    # prevent overwriting an existing group
-	    raise_param_exc({ group => "Security group '$param->{group}' does already exist" })
-		if $cluster_conf->{groups}->{$param->{group}} &&
-		$param->{group} ne $param->{rename};
+		# prevent overwriting an existing group
+		raise_param_exc({ group => "Security group '$param->{group}' does already exist" })
+		    if $cluster_conf->{groups}->{$param->{group}} &&
+		    $param->{group} ne $param->{rename};
 
-	    my $data = delete $cluster_conf->{groups}->{$param->{rename}};
-	    $cluster_conf->{groups}->{$param->{group}} = $data;
-	    if (my $comment = delete $cluster_conf->{group_comments}->{$param->{rename}}) {
-		$cluster_conf->{group_comments}->{$param->{group}} = $comment;
-	    }
-	    $cluster_conf->{group_comments}->{$param->{group}} = $param->{comment} if defined($param->{comment});
-	} else {
-	    foreach my $name (keys %{$cluster_conf->{groups}}) {
-		raise_param_exc({ group => "Security group '$name' already exists" })
-		    if $name eq $param->{group};
-	    }
+		my $data = delete $cluster_conf->{groups}->{$param->{rename}};
+		$cluster_conf->{groups}->{$param->{group}} = $data;
+		if (my $comment = delete $cluster_conf->{group_comments}->{$param->{rename}}) {
+		    $cluster_conf->{group_comments}->{$param->{group}} = $comment;
+		}
+		$cluster_conf->{group_comments}->{$param->{group}} = $param->{comment} if defined($param->{comment});
+	    } else {
+		foreach my $name (keys %{$cluster_conf->{groups}}) {
+		    raise_param_exc({ group => "Security group '$name' already exists" })
+			if $name eq $param->{group};
+		}
 
-	    $cluster_conf->{groups}->{$param->{group}} = [];
-	    $cluster_conf->{group_comments}->{$param->{group}} = $param->{comment} if defined($param->{comment});
-	}
+		$cluster_conf->{groups}->{$param->{group}} = [];
+		$cluster_conf->{group_comments}->{$param->{group}} = $param->{comment} if defined($param->{comment});
+	    }
 
-	PVE::Firewall::save_clusterfw_conf($cluster_conf);
+	    PVE::Firewall::save_clusterfw_conf($cluster_conf);
+	});
 
 	return undef;
     }});
diff --git a/src/PVE/API2/Firewall/Host.pm b/src/PVE/API2/Firewall/Host.pm
index 2303494..b66ca55 100644
--- a/src/PVE/API2/Firewall/Host.pm
+++ b/src/PVE/API2/Firewall/Host.pm
@@ -118,30 +118,32 @@ __PACKAGE__->register_method({
     code => sub {
 	my ($param) = @_;
 
-	my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
-	my $hostfw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf);
-
-	my (undef, $digest) = PVE::Firewall::copy_opject_with_digest($hostfw_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'" })
-		    if !$option_properties->{$opt};
-		delete $hostfw_conf->{options}->{$opt};
+	PVE::Firewall::lock_hostfw_conf(10, sub {
+	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
+	    my $hostfw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf);
+
+	    my (undef, $digest) = PVE::Firewall::copy_opject_with_digest($hostfw_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'" })
+			if !$option_properties->{$opt};
+		    delete $hostfw_conf->{options}->{$opt};
+		}
 	    }
-	}
 
-	if (defined($param->{enable})) {
-	    $param->{enable} = $param->{enable} ? 1 : 0;
-	}
+	    if (defined($param->{enable})) {
+		$param->{enable} = $param->{enable} ? 1 : 0;
+	    }
 
-	foreach my $k (keys %$option_properties) {
-	    next if !defined($param->{$k});
-	    $hostfw_conf->{options}->{$k} = $param->{$k};
-	}
+	    foreach my $k (keys %$option_properties) {
+		next if !defined($param->{$k});
+		$hostfw_conf->{options}->{$k} = $param->{$k};
+	    }
 
-	PVE::Firewall::save_hostfw_conf($hostfw_conf);
+	    PVE::Firewall::save_hostfw_conf($hostfw_conf);
+	});
 
 	return undef;
     }});
diff --git a/src/PVE/API2/Firewall/IPSet.pm b/src/PVE/API2/Firewall/IPSet.pm
index 72e7524..913dd86 100644
--- a/src/PVE/API2/Firewall/IPSet.pm
+++ b/src/PVE/API2/Firewall/IPSet.pm
@@ -148,12 +148,17 @@ sub register_delete_ipset {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
+
+		my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
 
-	    die "IPSet '$param->{name}' is not empty\n"
-		if scalar(@$ipset);
+		die "IPSet '$param->{name}' is not empty\n"
+		    if scalar(@$ipset);
 
-	    $class->save_ipset($param, $fw_conf, undef);
+		$class->save_ipset($param, $fw_conf, undef);
+
+	    });
 
 	    return undef;
 	}});
@@ -184,30 +189,35 @@ sub register_create_ip {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
 
-	    my $cidr = $param->{cidr};
+		my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
 
-	    foreach my $entry (@$ipset) {
-		raise_param_exc({ cidr => "address '$cidr' already exists" })
-		    if $entry->{cidr} eq $cidr;
-	    }
+		my $cidr = $param->{cidr};
+
+		foreach my $entry (@$ipset) {
+		    raise_param_exc({ cidr => "address '$cidr' already exists" })
+			if $entry->{cidr} eq $cidr;
+		}
+
+		raise_param_exc({ cidr => "a zero prefix is not allowed in ipset entries" })
+		    if $cidr =~ m!/0+$!;
 
-	    raise_param_exc({ cidr => "a zero prefix is not allowed in ipset entries" })
-		if $cidr =~ m!/0+$!;
+		# make sure alias exists (if $cidr is an alias)
+		PVE::Firewall::resolve_alias($cluster_conf, $fw_conf, $cidr)
+		    if $cidr =~ m/^${PVE::Firewall::ip_alias_pattern}$/;
 
-	    # make sure alias exists (if $cidr is an alias)
-	    PVE::Firewall::resolve_alias($cluster_conf, $fw_conf, $cidr)
-		if $cidr =~ m/^${PVE::Firewall::ip_alias_pattern}$/;
+		my $data = { cidr => $cidr };
 
-	    my $data = { cidr => $cidr };
+		$data->{nomatch} = 1 if $param->{nomatch};
+		$data->{comment} = $param->{comment} if $param->{comment};
 
-	    $data->{nomatch} = 1 if $param->{nomatch};
-	    $data->{comment} = $param->{comment} if $param->{comment};
+		unshift @$ipset, $data;
 
-	    unshift @$ipset, $data;
+		$class->save_ipset($param, $fw_conf, $ipset);
 
-	    $class->save_ipset($param, $fw_conf, $ipset);
+	    });
 
 	    return undef;
 	}});
@@ -276,19 +286,27 @@ sub register_update_ip {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
+	    my $found = $class->lock_config($param, sub {
+		my ($param) = @_;
+
+		my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
 
-	    my (undef, $digest) = PVE::Firewall::copy_list_with_digest($ipset);
-	    PVE::Tools::assert_if_modified($digest, $param->{digest});
+		my (undef, $digest) = PVE::Firewall::copy_list_with_digest($ipset);
+		PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-	    foreach my $entry (@$ipset) {
-		if($entry->{cidr} eq $param->{cidr}) {
-		    $entry->{nomatch} = $param->{nomatch};
-		    $entry->{comment} = $param->{comment};
-		    $class->save_ipset($param, $fw_conf, $ipset);
-		    return;
+		foreach my $entry (@$ipset) {
+		    if($entry->{cidr} eq $param->{cidr}) {
+			$entry->{nomatch} = $param->{nomatch};
+			$entry->{comment} = $param->{comment};
+			$class->save_ipset($param, $fw_conf, $ipset);
+			return 1;
+		    }
 		}
-	    }
+
+		return 0;
+	    });
+
+	    return if $found;
 
 	    raise_param_exc({ cidr => "no such IP/Network" });
 	}});
@@ -318,18 +336,22 @@ sub register_delete_ip {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
 
-	    my (undef, $digest) = PVE::Firewall::copy_list_with_digest($ipset);
-	    PVE::Tools::assert_if_modified($digest, $param->{digest});
+		my ($cluster_conf, $fw_conf, $ipset) = $class->load_config($param);
 
-	    my $new = [];
+		my (undef, $digest) = PVE::Firewall::copy_list_with_digest($ipset);
+		PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-	    foreach my $entry (@$ipset) {
-		push @$new, $entry if $entry->{cidr} ne $param->{cidr};
-	    }
+		my $new = [];
 
-	    $class->save_ipset($param, $fw_conf, $new);
+		foreach my $entry (@$ipset) {
+		    push @$new, $entry if $entry->{cidr} ne $param->{cidr};
+		}
+
+		$class->save_ipset($param, $fw_conf, $new);
+	    });
 
 	    return undef;
 	}});
@@ -611,37 +633,41 @@ sub register_create {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($cluster_conf, $fw_conf) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
 
-	    if ($param->{rename}) {
-		my (undef, $digest) = &$get_ipset_list($fw_conf);
-		PVE::Tools::assert_if_modified($digest, $param->{digest});
+		my ($cluster_conf, $fw_conf) = $class->load_config($param);
 
-		raise_param_exc({ name => "IPSet '$param->{rename}' does not exist" })
-		    if !$fw_conf->{ipset}->{$param->{rename}};
+		if ($param->{rename}) {
+		    my (undef, $digest) = &$get_ipset_list($fw_conf);
+		    PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-		# prevent overwriting existing ipset
-		raise_param_exc({ name => "IPSet '$param->{name}' does already exist"})
-		    if $fw_conf->{ipset}->{$param->{name}} &&
-		    $param->{name} ne $param->{rename};
+		    raise_param_exc({ name => "IPSet '$param->{rename}' does not exist" })
+			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 {
-		foreach my $name (keys %{$fw_conf->{ipset}}) {
-		    raise_param_exc({ name => "IPSet '$name' already exists" })
-			if $name eq $param->{name};
-		}
+		    # prevent overwriting existing ipset
+		    raise_param_exc({ name => "IPSet '$param->{name}' does already exist"})
+			if $fw_conf->{ipset}->{$param->{name}} &&
+			$param->{name} ne $param->{rename};
 
-		$fw_conf->{ipset}->{$param->{name}} = [];
-		$fw_conf->{ipset_comments}->{$param->{name}} = $param->{comment} if defined($param->{comment});
-	    }
+		    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 {
+		    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});
+		}
 
-	    $class->save_config($param, $fw_conf);
+		$class->save_config($param, $fw_conf);
+	    });
 
 	    return undef;
 	}});
diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm
index 1fde596..1e76e99 100644
--- a/src/PVE/API2/Firewall/Rules.pm
+++ b/src/PVE/API2/Firewall/Rules.pm
@@ -225,18 +225,22 @@ sub register_create_rule {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
+
+		my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
 
-	    my $rule = {};
+		my $rule = {};
 
-	    PVE::Firewall::copy_rule_data($rule, $param);
-	    PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env());
+		PVE::Firewall::copy_rule_data($rule, $param);
+		PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env());
 
-	    $rule->{enable} = 0 if !defined($param->{enable});
+		$rule->{enable} = 0 if !defined($param->{enable});
 
-	    unshift @$rules, $rule;
+		unshift @$rules, $rule;
 
-	    $class->save_rules($param, $fw_conf, $rules);
+		$class->save_rules($param, $fw_conf, $rules);
+	    });
 
 	    return undef;
 	}});
@@ -282,36 +286,40 @@ sub register_update_rule {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
+
+		my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
 
-	    my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
-	    PVE::Tools::assert_if_modified($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);
+		die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
 
-	    my $rule = $rules->[$param->{pos}];
+		my $rule = $rules->[$param->{pos}];
 
-	    my $moveto = $param->{moveto};
-	    if (defined($moveto) && $moveto != $param->{pos}) {
-		my $newrules = [];
-		for (my $i = 0; $i < scalar(@$rules); $i++) {
-		    next if $i == $param->{pos};
-		    if ($i == $moveto) {
-			push @$newrules, $rule;
+		my $moveto = $param->{moveto};
+		if (defined($moveto) && $moveto != $param->{pos}) {
+		    my $newrules = [];
+		    for (my $i = 0; $i < scalar(@$rules); $i++) {
+			next if $i == $param->{pos};
+			if ($i == $moveto) {
+			    push @$newrules, $rule;
+			}
+			push @$newrules, $rules->[$i];
 		    }
-		    push @$newrules, $rules->[$i];
-		}
-		push @$newrules, $rule if $moveto >= scalar(@$rules);
-		$rules = $newrules;
-	    } else {
-		PVE::Firewall::copy_rule_data($rule, $param);
+		    push @$newrules, $rule if $moveto >= scalar(@$rules);
+		    $rules = $newrules;
+		} else {
+		    PVE::Firewall::copy_rule_data($rule, $param);
 
-		PVE::Firewall::delete_rule_properties($rule, $param->{'delete'}) if $param->{'delete'};
+		    PVE::Firewall::delete_rule_properties($rule, $param->{'delete'}) if $param->{'delete'};
 
-		PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env());
-	    }
+		    PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env());
+		}
 
-	    $class->save_rules($param, $fw_conf, $rules);
+		$class->save_rules($param, $fw_conf, $rules);
+	    });
 
 	    return undef;
 	}});
@@ -344,16 +352,20 @@ sub register_delete_rule {
 	code => sub {
 	    my ($param) = @_;
 
-	    my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
+	    $class->lock_config($param, sub {
+		my ($param) = @_;
 
-	    my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
-	    PVE::Tools::assert_if_modified($digest, $param->{digest});
+		my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param);
 
-	    die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
+		my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules);
+		PVE::Tools::assert_if_modified($digest, $param->{digest});
 
-	    splice(@$rules, $param->{pos}, 1);
+		die "no rule at position $param->{pos}\n" if $param->{pos} >= scalar(@$rules);
 
-	    $class->save_rules($param, $fw_conf, $rules);
+		splice(@$rules, $param->{pos}, 1);
+
+		$class->save_rules($param, $fw_conf, $rules);
+	    });
 
 	    return undef;
 	}});
@@ -433,12 +445,16 @@ __PACKAGE__->register_method({
     code => sub {
 	my ($param) = @_;
 
-	my (undef, $cluster_conf, $rules) = __PACKAGE__->load_config($param);
+	__PACKAGE__->lock_config($param, sub {
+	    my ($param) = @_;
+
+	    my (undef, $cluster_conf, $rules) = __PACKAGE__->load_config($param);
 
-	die "Security group '$param->{group}' is not empty\n"
-	    if scalar(@$rules);
+	    die "Security group '$param->{group}' is not empty\n"
+		if scalar(@$rules);
 
-	__PACKAGE__->save_rules($param, $cluster_conf, undef);
+	    __PACKAGE__->save_rules($param, $cluster_conf, undef);
+	});
 
 	return undef;
     }});
diff --git a/src/PVE/API2/Firewall/VM.pm b/src/PVE/API2/Firewall/VM.pm
index 5b9b6dd..48b8c5f 100644
--- a/src/PVE/API2/Firewall/VM.pm
+++ b/src/PVE/API2/Firewall/VM.pm
@@ -121,31 +121,32 @@ sub register_handlers {
 	code => sub {
 	    my ($param) = @_;
 
-
-	    my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
-	    my $vmfw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, $rule_env, $param->{vmid});
-
-	    my (undef, $digest) = PVE::Firewall::copy_opject_with_digest($vmfw_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'" })
-			if !$option_properties->{$opt};
-		    delete $vmfw_conf->{options}->{$opt};
+	    PVE::Firewall::lock_vmfw_conf($param->{vmid}, 10, sub {
+		my $cluster_conf = PVE::Firewall::load_clusterfw_conf();
+		my $vmfw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, $rule_env, $param->{vmid});
+
+		my (undef, $digest) = PVE::Firewall::copy_opject_with_digest($vmfw_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'" })
+			    if !$option_properties->{$opt};
+			delete $vmfw_conf->{options}->{$opt};
+		    }
 		}
-	    }
 
-	    if (defined($param->{enable})) {
-		$param->{enable} = $param->{enable} ? 1 : 0;
-	    }
+		if (defined($param->{enable})) {
+		    $param->{enable} = $param->{enable} ? 1 : 0;
+		}
 
-	    foreach my $k (keys %$option_properties) {
-		next if !defined($param->{$k});
-		$vmfw_conf->{options}->{$k} = $param->{$k};
-	    }
+		foreach my $k (keys %$option_properties) {
+		    next if !defined($param->{$k});
+		    $vmfw_conf->{options}->{$k} = $param->{$k};
+		}
 
-	    PVE::Firewall::save_vmfw_conf($param->{vmid}, $vmfw_conf);
+		PVE::Firewall::save_vmfw_conf($param->{vmid}, $vmfw_conf);
+	    });
 
 	    return undef;
 	}});