Re: [pve-devel] [PATCH qemu-server 3/3] api: create: implement extracting disks when needed for import-from

[Dominik Csapak <d.csapak@proxmox.com>]

On 4/18/24 11:41, Fiona Ebner wrote:
> Am 16.04.24 um 15:19 schrieb Dominik Csapak:
>> @@ -391,6 +392,13 @@ my sub create_disks : prototype($$$$$$$$$$) {
>>   
>>   		$needs_creation = $live_import;
>>   
>> +		if (PVE::Storage::copy_needs_extraction($source)) { # needs extraction beforehand
>> +		    print "extracting $source\n";
>> +		    $source = PVE::Storage::extract_disk_from_import_file($source, $vmid);
>> +		    print "finished extracting to $source\n";
>> +		    push @$delete_sources, $source;
>> +		}
>> +
> 
> This breaks import from an absolute path: copy_needs_extraction()
> expects to be called with a PVE-managed volid, so the above should be
> moved into the if below.

true, that will be fixed in the next iteration since we then get a
pve managed volid back after extraction
(see my answer to the cover letter)

> 
>>   		if (PVE::Storage::parse_volume_id($source, 1)) { # PVE-managed volume
>>   		    if ($live_import && $ds ne 'efidisk0') {
>>   			my $path = PVE::Storage::path($storecfg, $source)
>> @@ -514,13 +522,14 @@ my sub create_disks : prototype($$$$$$$$$$) {
>>   	    eval { PVE::Storage::vdisk_free($storecfg, $volid); };
>>   	    warn $@ if $@;
>>   	}
>> +	PVE::QemuServer::Helpers::cleanup_extracted_images($delete_sources);
>>   	die $err;
>>       }
>>   
>>       # don't return empty import mappings
>>       $live_import_mapping = undef if !%$live_import_mapping;
>>   
>> -    return ($vollist, $res, $live_import_mapping);
>> +    return ($vollist, $res, $live_import_mapping, $delete_sources);
>>   };
>>   
>>   my $check_cpu_model_access = sub {
> 
> The second caller of create_disks(), i.e. when updating an existing VM,
> is not updated to handle $delete_sources. (You can also do a disk
> import-from from an OVA for an existing VM).
> 
> When I tested that my suspicion is correct I didn't notice initially
> that the temporary dirs were hidden, should we really make them so hard
> to find?

see my recent answer to the cover letter, this shouldn't be an issue when
we put the extracted image into a valid image path on the storage


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