[pve-devel] [PATCH storage 3/9] plugin: dir: handle ova files for import

[Dominik Csapak]

On 4/17/24 15:52, Fabian Grünbichler wrote:
> On April 17, 2024 3:10 pm, Dominik Csapak wrote:
>> On 4/17/24 14:45, Fabian Grünbichler wrote:
>>> On April 16, 2024 3:18 pm, Dominik Csapak wrote:
>>>> +sub cleanup_extracted_image {
>>>
>>> same for this?
>>>
>>>> +    my ($source) = @_;
>>>> +
>>>> +    if ($source =~ m|^(/.+/\.tmp_[0-9]+_[0-9]+)/[^/]+$|) {
>>>> +	my $tmpdir = $1;
>>>> +
>>>> +	unlink $source or $! == ENOENT or die "removing image $source failed: $!\n";
>>>> +	rmdir $tmpdir or $! == ENOENT or die "removing tmpdir $tmpdir failed: $!\n";
>>>> +    } else {
>>>> +	die "invalid extraced image path '$source'\n";
>>>
>>> nit: typo
>>>
>>> these are also not discoverable if the error handling in qemu-server
>>> failed for some reason.. might be a source of unwanted space
>>> consumption..
>>
>> any suggestions for better handling that cleanup?
>> we could put it at the beginning of each cleanup step, that should
>> at least make sure we cleaned up the temporary images
> 
> we could extract them into images/XXX/vm-XXX-disk-.. directly (or
> rename/move them there after extraction), that way at least they could
> be cleaned up via the storage API or rescan + delete (and via a regular
> vdisk_free in qemu-server, instead of requiring a special helper).
> 
> other than that, I don't think we have an easy way of
> - exposing them in list & free_image
> - while ensuring nobody deletes them while the import is still going on
>    (the target VM ownership checks ensure that at least via the UI if we
>    make it an owned volume)
> 
> it would also allow skipping the conversion if the storage+format
> already match the target spec as well..

mhmm that could work, but what if the storage does not have
the 'images' content type enabled? should we simply fail then?