[pve-devel] [PATCH v2 qemu-server] fix #1013 : migrate : sync_disk : --targetstorage with offline disk

[Thomas Lamprecht]

On 12/10/18 5:28 PM, Alexandre Derumier wrote:
> targetsid was not used, for disk unused (offline copy)
> ---
>  PVE/QemuMigrate.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index e9e9075..0bae076 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -446,6 +446,7 @@ sub sync_disks {
>  
>  	foreach my $volid (keys %$local_volumes) {
>  	    my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
> +	    my $targetsid = $self->{opts}->{targetstorage} ? $self->{opts}->{targetstorage} : $sid;

Hmm, I took a closer look at this and found out that it now breaks the case where
no targetstorage is passed, previously we falled back to the same storage as the
volume had on the source node, to mark this we do (a few lines above your change):

if ($self->{running} && !$sharedvm && !$self->{opts}->{targetstorage}) {
    $self->{opts}->{targetstorage} = 1; #use same sid for remote local
}

with your changes the '1' is used a literal target storage, which then naturally
fails... E.e., this fails:

# qm migrate 101 pve2 --online --with-local-disks

but this works:
# qm migrate 101 pve2 --online --with-local-disks --targetstorage local-lvm


>  	    if ($self->{running} && $self->{opts}->{targetstorage} && $local_volumes->{$volid}->{ref} eq 'config') {
>  		push @{$self->{online_local_volumes}}, $volid;
>  	    } else {
> @@ -453,7 +454,7 @@ sub sync_disks {
>  		push @{$self->{volumes}}, $volid;
>  		my $insecure = $self->{opts}->{migration_type} eq 'insecure';
>  		my $with_snapshots = $local_volumes->{$volid}->{snapshots};
> -		PVE::Storage::storage_migrate($self->{storecfg}, $volid, $self->{ssh_info}, $sid,
> +		PVE::Storage::storage_migrate($self->{storecfg}, $volid, $self->{ssh_info}, $targetsid,
>  					      undef, undef, undef, undef, $insecure, $with_snapshots);
>  	    }
>  	}
>