[pve-devel] [PATCH qemu-server 1/3] Update unused volumes in config when doing

[Fabian Ebner]

On 10/29/19 7:28 PM, Thomas Lamprecht wrote:
> On 10/28/19 10:57 AM, Fabian Ebner wrote:
>> When doing an online migration with --targetstorage unused disks get migrated
>> to the specified target storage as well.
>> With this patch we keep track of those volumes and update the VM config with
>> their new locations. Unused volumes of the VM previously not present in the
>> config are added as well.
>>
>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>> ---
>>   PVE/QemuMigrate.pm | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
>> index 65f39b6..0e9fdcf 100644
>> --- a/PVE/QemuMigrate.pm
>> +++ b/PVE/QemuMigrate.pm
>> @@ -465,6 +465,12 @@ sub sync_disks {
>>   	    } else {
>>   		next if $rep_volumes->{$volid};
>>   		push @{$self->{volumes}}, $volid;
>> +
>> +		if (defined($override_targetsid)) {
>> +		    my (undef, $targetvolname) = PVE::Storage::parse_volume_id($volid);
>> +		    push @{$self->{online_unused_volumes}}, "${targetsid}:${targetvolname}";
> sorry, but where do you check if this is a unused volume,
> vs. a used one?
>
> I mean here land all local volumes which are either !($self->{running} && $ref eq 'config')
> or $ref eq 'generated', or do I oversee something?
The check is implicit in the check for $override_targetsid,
which can only be defined if $self->{running}.
And if snapshots exist online migration is also not possible.
So the only possibilities are $ref eq 'storage' and undef
and I assumed that undef also means it's an unused volume.

I should've at least mentioned this and checking explicitly is
of course better and stable against future changes.
I'll send a v2, thanks!

>> +		}
>> +
>>   		my $opts = $self->{opts};
>>   		my $insecure = $opts->{migration_type} eq 'insecure';
>>   		my $with_snapshots = $local_volumes->{$volid}->{snapshots};
>> @@ -958,6 +964,16 @@ sub phase3_cleanup {
>>   	}
>>       }
>>   
>> +    if ($self->{online_unused_volumes}) {
>> +	foreach my $conf_key (keys %{$conf}) {
>> +	    delete $conf->{$conf_key} if ($conf_key =~ m/^unused\d+$/);
>> +	}
>> +	foreach my $targetvolid (@{$self->{online_unused_volumes}}) {
>> +	    PVE::QemuConfig->add_unused_volume($conf, $targetvolid);
>> +	}
>> +	PVE::QemuConfig->write_config($vmid, $conf);
>> +    }
>> +
>>       # transfer replication state before move config
>>       $self->transfer_replication_state() if $self->{replicated_volumes};
>>   
>>