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

Fabian Ebner f.ebner at proxmox.com
Thu Jul 30 13:29:13 CEST 2020


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 at proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner at 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






More information about the pve-devel mailing list