On April 8, 2020 11:25 am, Fabian Ebner wrote:
> Call cleanup_remotedisks in phase1_cleanup as well, because that's where
> we end if sync_disks fails and some disks might already have been
> transfered successfully.
> 
> Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
> ---
>  PVE/QemuMigrate.pm | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 0a6277d..68eb083 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -577,16 +577,32 @@ sub sync_disks {
>  sub cleanup_remotedisks {
>      my ($self) = @_;
>  
> +    my $cleanup_volids = {};
> +
> +    foreach my $volid (values %{$self->{volume_map}}) {
> +     $cleanup_volids->{$volid} = 1;
> +    }
> +
>      foreach my $target_drive (keys %{$self->{target_drive}}) {
>       my $drivestr = $self->{target_drive}->{$target_drive}->{drivestr};
>       next if !defined($drivestr);
>  
>       my $drive = PVE::QemuServer::parse_drive($target_drive, $drivestr);
> +     my $volid = $drive->{file};
> +     next if !defined($volid);
>  
> -     # don't clean up replicated disks!
> -     next if defined($self->{replicated_volumes}->{$drive->{file}});
> +     if (defined($self->{replicated_volumes}->{$volid})) {
> +         # don't clean up replicated disks!
> +         delete $cleanup_volids->{$volid};
> +     } else {
> +         # volume_map might not yet contain all of the live-migrating volumes
> +         # if there's an error while starting the drive-mirror jobs
> +         $cleanup_volids->{$volid} = 1;

see patch #14 ;)

it might also be interesting to look at unifying all or some of the 
volume tracking keys in $self:

$self->{volumes} for local volumes, except replicated ones, except if 
live-migrating and referenced in current config
$self->{online_local_volumes} source volids that should be NBD migrated
$self->{replicated_volumes} for replicated volumes
$self->{volume_map} for mappings of source to target volids
$self->{target_drive} returned info from target node for NBD migrated volumes
... ?

really shows quite well what the main issue is for QemuMigrate.pm

> +     }
> +    }
>  
> -     my ($storeid, $volname) = PVE::Storage::parse_volume_id($drive->{file});
> +    foreach my $volid (keys %{$cleanup_volids}) {
> +     my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid);
>  
>       my $cmd = [@{$self->{rem_ssh}}, 'pvesm', 'free', "$storeid:$volname"];
>  
> @@ -638,11 +654,9 @@ sub phase1_cleanup {
>       $self->log('err', $err);
>      }
>  
> -    if ($self->{volumes}) {
> -     foreach my $volid (@{$self->{volumes}}) {
> -         $self->log('err', "found stale volume copy '$volid' on node 
> '$self->{node}'");
> -         # fixme: try to remove ?
> -     }
> +    eval { $self->cleanup_remotedisks() };
> +    if (my $err = $@) {
> +     $self->log('err', $err);
>      }
>  
>      eval { $self->cleanup_bitmaps() };
> -- 
> 2.20.1

_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to