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 D03BB6C388 for ; Fri, 29 Jan 2021 16:11:53 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2AC5A112F9 for ; Fri, 29 Jan 2021 16:11:53 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (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 EF7121120E for ; Fri, 29 Jan 2021 16:11:47 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id B86EC43B65 for ; Fri, 29 Jan 2021 16:11:47 +0100 (CET) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Fri, 29 Jan 2021 16:11:38 +0100 Message-Id: <20210129151143.10014-9-f.ebner@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210129151143.10014-1-f.ebner@proxmox.com> References: <20210129151143.10014-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.004 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [qemumigrate.pm] Subject: [pve-devel] [PATCH v2 qemu-server 08/13] migration: simplify removal of local volumes and get rid of self->{volumes} 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: Fri, 29 Jan 2021 15:11:53 -0000 This also changes the behavior to remove the local copies of offline migrated volumes only after the migration has finished successfully (this is relevant for mixed settings, e.g. online migration with unused/vmstate disks). local_volumes contains both, the volumes previously in $self->{volumes} and the volumes in $self->{online_local_volumes}, and hence is the place to look for which volumes we need to remove. Of course, replicated volumes still need to be skipped. Signed-off-by: Fabian Ebner --- Changes from v1: * changed the order of this and next patch to avoid temporary breakage in an edge case if only the old #8 would be applied: if we died in phase3_cleanup on the qemu_drive_mirror_monitor call unused disks could get lost, because they were already cleaned up in phase3. Now this patch (old #9) comes first, making sure the cleanup of local disks happens in phase3_cleanup after successful migration. See also the test in patch #13 PVE/QemuMigrate.pm | 51 +++++++++++++++------------------------------- 1 file changed, 16 insertions(+), 35 deletions(-) diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm index b10638a..5d92028 100644 --- a/PVE/QemuMigrate.pm +++ b/PVE/QemuMigrate.pm @@ -365,7 +365,6 @@ sub scan_local_volumes { # local volumes which have been copied # and their old_id => new_id pairs - $self->{volumes} = []; $self->{volume_map} = {}; $self->{local_volumes} = {}; @@ -599,14 +598,10 @@ sub scan_local_volumes { my $ref = $local_volumes->{$volid}->{ref}; if ($self->{running} && $ref eq 'config') { push @{$self->{online_local_volumes}}, $volid; - } elsif ($ref eq 'generated') { - die "can't live migrate VM with local cloudinit disk. use a shared storage instead\n" if $self->{running}; - # skip all generated volumes but queue them for deletion in phase3_cleanup - push @{$self->{volumes}}, $volid; - next; + } elsif ($self->{running} && $ref eq 'generated') { + die "can't live migrate VM with local cloudinit disk. use a shared storage instead\n"; } else { next if $self->{replicated_volumes}->{$volid}; - push @{$self->{volumes}}, $volid; $local_volumes->{$volid}->{migration_mode} = 'offline'; } } @@ -764,8 +759,10 @@ sub phase1_cleanup { $self->log('err', $err); } - if ($self->{volumes}) { - foreach my $volid (@{$self->{volumes}}) { + my @volids = $self->filter_local_volumes('offline'); + if (scalar(@volids)) { + foreach my $volid (@volids) { + next if defined($self->{replicated_volumes}->{$volid}); $self->log('err', "found stale volume copy '$volid' on node '$self->{node}'"); # fixme: try to remove ? } @@ -1198,18 +1195,7 @@ sub phase2_cleanup { sub phase3 { my ($self, $vmid) = @_; - my $volids = $self->{volumes}; - return if $self->{phase2errors}; - - # destroy local copies - foreach my $volid (@$volids) { - eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); }; - if (my $err = $@) { - $self->log('err', "removing local copy of '$volid' failed - $err"); - $self->{errors} = 1; - last if $err =~ /^interrupted by signal$/; - } - } + return; } sub phase3_cleanup { @@ -1334,22 +1320,17 @@ sub phase3_cleanup { $self->{errors} = 1; } - if($self->{storage_migration}) { - # destroy local copies - my $volids = $self->{online_local_volumes}; - - foreach my $volid (@$volids) { - # keep replicated volumes! - next if $self->{replicated_volumes}->{$volid}; + # destroy local copies + foreach my $volid (keys %{$self->{local_volumes}}) { + # keep replicated volumes! + next if $self->{replicated_volumes}->{$volid}; - eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); }; - if (my $err = $@) { - $self->log('err', "removing local copy of '$volid' failed - $err"); - $self->{errors} = 1; - last if $err =~ /^interrupted by signal$/; - } + eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); }; + if (my $err = $@) { + $self->log('err', "removing local copy of '$volid' failed - $err"); + $self->{errors} = 1; + last if $err =~ /^interrupted by signal$/; } - } # clear migrate lock -- 2.20.1