From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 45E1E76791 for ; Tue, 19 Oct 2021 09:55:48 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3907A29C75 for ; Tue, 19 Oct 2021 09:55:17 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 7DA0F298F9 for ; Tue, 19 Oct 2021 09:55:09 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id B000F468D2 for ; Tue, 19 Oct 2021 09:55:04 +0200 (CEST) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Tue, 19 Oct 2021 09:54:55 +0200 Message-Id: <20211019075459.14328-7-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211019075459.14328-1-f.ebner@proxmox.com> References: <20211019075459.14328-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.267 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH guest-common 3/4] replication: find common snapshot: use additional information X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Oct 2021 07:55:48 -0000 which is now available from the storage back-end. The benefits are: 1. Ability to detect different snapshots even if they have the same name. Rather hard to reach, but for example with: Snapshots A -> B -> C -> __replicationXYZ Remove B, rollback to C (causes __replicationXYZ to be removed), create a new snapshot B. Previously, B was selected as replication base, but it didn't match on source and target. Now, C is correctly selected. 2. Smaller delta in some cases by not prefering replication snapshots over config snapshots, but using the most recent possible one from both types of snapshots. 3. Less code complexity for snapshot selection. If the remote side is old, it's not possible to detect mismatch of distinct snapshots with the same name, but the timestamps from the local side can still be used. Still limit to our snapshots (config and replication), because we don't have guarantees for other snapshots (could be deleted in the meantime, name might not fit import/export regex, etc.). Signed-off-by: Fabian Ebner --- src/PVE/Replication.pm | 55 ++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/src/PVE/Replication.pm b/src/PVE/Replication.pm index 4d4f62f..051dfd9 100644 --- a/src/PVE/Replication.pm +++ b/src/PVE/Replication.pm @@ -31,6 +31,7 @@ sub find_common_replication_snapshot { my ($ssh_info, $jobid, $vmid, $storecfg, $volumes, $storeid_list, $last_sync, $guest_conf, $logfunc) = @_; my $parent_snapname = $guest_conf->{parent}; + my $conf_snapshots = $guest_conf->{snapshots}; my $last_sync_snapname = PVE::ReplicationState::replication_snapshot_name($jobid, $last_sync); @@ -57,24 +58,35 @@ sub find_common_replication_snapshot { my $base_snapshots = {}; foreach my $volid (@$volumes) { - if (defined($last_snapshots->{$volid}) && defined($remote_snapshots->{$volid})) { - if ($last_snapshots->{$volid}->{$last_sync_snapname} && - $remote_snapshots->{$volid}->{$last_sync_snapname}) { + my $local_info = $last_snapshots->{$volid}; + my $remote_info = $remote_snapshots->{$volid}; + + if (defined($local_info) && defined($remote_info)) { + my $common_snapshot = sub { + my ($snap) = @_; + + return 0 if !$local_info->{$snap} || !$remote_info->{$snap}; + + # Check for ID if remote side supports it already. + return $local_info->{$snap}->{id} eq $remote_info->{$snap}->{id} + if ref($remote_info->{$snap}) eq 'HASH'; + + return 1; + }; + + if ($common_snapshot->($last_sync_snapname)) { $base_snapshots->{$volid} = $last_sync_snapname; - } elsif (defined($parent_snapname) && - ($last_snapshots->{$volid}->{$parent_snapname} && - $remote_snapshots->{$volid}->{$parent_snapname})) { + } elsif (defined($parent_snapname) && $common_snapshot->($parent_snapname)) { $base_snapshots->{$volid} = $parent_snapname; } else { - # First, try all replication snapshots. my $most_recent = [0, undef]; - for my $remote_snap (keys $remote_snapshots->{$volid}->%*) { - next if !defined($last_snapshots->{$volid}->{$remote_snap}); + for my $snapshot (keys $local_info->%*) { + next if !$common_snapshot->($snapshot); + next if !$conf_snapshots->{$snapshot} && !is_replication_snapshot($snapshot); - my $timestamp = ($remote_snap =~ /__replicate_\Q$vmid\E-(?:\d+)_(\d+)_/)[0]; - next if !$timestamp; + my $timestamp = $local_info->{$snapshot}->{timestamp}; - $most_recent = [$timestamp, $remote_snap] if $timestamp > $most_recent->[0]; + $most_recent = [$timestamp, $snapshot] if $timestamp > $most_recent->[0]; } if ($most_recent->[1]) { @@ -82,25 +94,6 @@ sub find_common_replication_snapshot { next; } - # Then, try config snapshots ($parent_snapname was already tested for above). - my $snapname = $parent_snapname // ''; - - # Be robust against loop, just in case. - # https://forum.proxmox.com/threads/snapshot-not-working.69511/post-312281 - my $max_count = scalar(keys $guest_conf->{snapshots}->%*); - for (my $count = 0; $count < $max_count; $count++) { - last if defined($base_snapshots->{$volid}); - - my $snap_conf = $guest_conf->{snapshots}->{$snapname} || last; - $snapname = $snap_conf->{parent} // last; - - if ($last_snapshots->{$volid}->{$snapname} && - $remote_snapshots->{$volid}->{$snapname}) - { - $base_snapshots->{$volid} = $snapname; - } - } - # The volume exists on the remote side, so trying a full sync won't work. # Die early with a clean error. die "No common base to restore the job state\n". -- 2.30.2