Re: [pve-devel] [PATCH qemu-server 04/10] refactor map_storage to map_id

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On November 9, 2021 10:06 am, Fabian Ebner wrote:
> I feel like map_id should be moved to pve-common, so that it can be 
> re-used in other places (pve-container will need it too), and so that no 
> package boundary needs to be crossed if we ever need to adapt the 
> internals of the idmap.

yeah, likely some other parts as well (either to guest-common, common or 
storage, once we extend this mechanism to cover ZFS replication and 
container migration)

> 
> Am 05.11.21 um 14:03 schrieb Fabian Grünbichler:
>> since we are going to reuse the same mechanism/code for network bridge
>> mapping.
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>>   PVE/QemuMigrate.pm | 6 +++---
>>   PVE/QemuServer.pm  | 6 ++++--
>>   2 files changed, 7 insertions(+), 5 deletions(-)
>> 
>> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
>> index ae3eaf1..779f5ee 100644
>> --- a/PVE/QemuMigrate.pm
>> +++ b/PVE/QemuMigrate.pm
>> @@ -342,7 +342,7 @@ sub prepare {
>>   	my $targetsid = $sid;
>>   	# NOTE: we currently ignore shared source storages in mappings so skip here too for now
>>   	if (!$scfg->{shared}) {
>> -	    $targetsid = PVE::QemuServer::map_storage($self->{opts}->{storagemap}, $sid);
>> +	    $targetsid = PVE::QemuServer::map_id($self->{opts}->{storagemap}, $sid);
>>   	}
>>   
>>   	my $target_scfg = PVE::Storage::storage_check_enabled($storecfg, $targetsid, $self->{node});
>> @@ -408,7 +408,7 @@ sub scan_local_volumes {
>>   
>>   	    next if @{$dl->{$storeid}} == 0;
>>   
>> -	    my $targetsid = PVE::QemuServer::map_storage($self->{opts}->{storagemap}, $storeid);
>> +	    my $targetsid = PVE::QemuServer::map_id($self->{opts}->{storagemap}, $storeid);
>>   	    # check if storage is available on target node
>>   	    my $target_scfg = PVE::Storage::storage_check_enabled(
>>   		$storecfg,
>> @@ -479,7 +479,7 @@ sub scan_local_volumes {
>>   	    my $targetsid = $sid;
>>   	    # NOTE: we currently ignore shared source storages in mappings so skip here too for now
>>   	    if (!$scfg->{shared}) {
>> -		$targetsid = PVE::QemuServer::map_storage($self->{opts}->{storagemap}, $sid);
>> +		$targetsid = PVE::QemuServer::map_id($self->{opts}->{storagemap}, $sid);
>>   	    }
>>   
>>   	    PVE::Storage::storage_check_enabled($storecfg, $targetsid, $self->{node});
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index a0e1809..23e7b08 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -120,7 +120,9 @@ PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
>>   });
>>   
>>   
>> -sub map_storage {
>> +# maps source to target ID
>> +# currently used for targetstorage and targetbridge when migrating
>> +sub map_id {
>>       my ($map, $source) = @_;
>>   
>>       return $source if !defined($map);
>> @@ -5264,7 +5266,7 @@ sub vm_migrate_alloc_nbd_disks {
>>   	# volume is not available there, fall back to the default format.
>>   	# Otherwise use the same format as the original.
>>   	if (!$storagemap->{identity}) {
>> -	    $storeid = map_storage($storagemap, $storeid);
>> +	    $storeid = map_id($storagemap, $storeid);
>>   	    my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
>>   	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>>   	    my $fileFormat = qemu_img_format($scfg, $volname);
>> 
>