Am 18.04.24 um 11:48 schrieb Dominik Csapak:
> 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)
>
Sorry, I don't understand. The breakage is for import from an absolute
path, because copy_needs_extraction() cannot be called on an absolute
path. Why does it matter whether extraction returns a managed volid or not?
>>
>>> 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
>
But we should still attempt cleanup and not just ignore the
$delete_sources for the second caller.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
