[pve-devel] [PATCH manager 1/3] Hold the guest migration lock when changing the replication config

[Fabian Ebner <f.ebner@proxmox.com>]

The guest migration lock is already held when running replications,
but it also makes sense to hold it when updating the replication
config itself. Otherwise, it can happen that the migration does
not know the de-facto state of replication.

For example:
1. migration starts
2. replication job is deleted
3. migration reads the replication config
4. migration runs the replication which causes the
   replicated disks to be removed, because the job
   is marked for removal
5. migration will continue without replication

Note that the migration doesn't actually fail, but it's probably
not the desired behavior either.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/ReplicationConfig.pm | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/ReplicationConfig.pm b/PVE/API2/ReplicationConfig.pm
index 2b4ecd10..e5262068 100644
--- a/PVE/API2/ReplicationConfig.pm
+++ b/PVE/API2/ReplicationConfig.pm
@@ -9,6 +9,7 @@ use PVE::JSONSchema qw(get_standard_option);
 use PVE::RPCEnvironment;
 use PVE::ReplicationConfig;
 use PVE::Cluster;
+use PVE::GuestHelpers;
 
 use PVE::RESTHandler;
 
@@ -144,7 +145,9 @@ __PACKAGE__->register_method ({
 	    $cfg->write();
 	};
 
-	PVE::ReplicationConfig::lock($code);
+	PVE::GuestHelpers::guest_migration_lock($guest, 10, sub {
+	    PVE::ReplicationConfig::lock($code);
+	});
 
 	return undef;
     }});
@@ -167,6 +170,7 @@ __PACKAGE__->register_method ({
 	my $id = extract_param($param, 'id');
 	my $digest = extract_param($param, 'digest');
 	my $delete = extract_param($param, 'delete');
+	my ($guest_id) = PVE::ReplicationConfig::parse_replication_job_id($id);
 
 	my $code = sub {
 	    my $cfg = PVE::ReplicationConfig->new();
@@ -199,7 +203,9 @@ __PACKAGE__->register_method ({
 	    $cfg->write();
 	};
 
-	PVE::ReplicationConfig::lock($code);
+	PVE::GuestHelpers::guest_migration_lock($guest_id, 10, sub {
+	    PVE::ReplicationConfig::lock($code);
+	});
 
 	return undef;
     }});
@@ -237,10 +243,12 @@ __PACKAGE__->register_method ({
 
 	my $rpcenv = PVE::RPCEnvironment::get();
 
+	my $id = extract_param($param, 'id');
+	my ($guest_id) = PVE::ReplicationConfig::parse_replication_job_id($id);
+
 	my $code = sub {
 	    my $cfg = PVE::ReplicationConfig->new();
 
-	    my $id = $param->{id};
 	    if ($param->{force}) {
 		raise_param_exc({ 'keep' => "conflicts with parameter 'force'" }) if $param->{keep};
 		delete $cfg->{ids}->{$id};
@@ -262,7 +270,9 @@ __PACKAGE__->register_method ({
 	    $cfg->write();
 	};
 
-	PVE::ReplicationConfig::lock($code);
+	PVE::GuestHelpers::guest_migration_lock($guest_id, 10, sub {
+	    PVE::ReplicationConfig::lock($code);
+	});
 
 	return undef;
     }});
-- 
2.20.1