Re: [pve-devel] [PATCH qemu-server v7 4/5] api: create: add 'import-working-storage' parameter

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

I now looked into your diff.

Am 18.11.24 um 18:24 schrieb Aaron Lauterer:
> On  2024-11-18  16:29, Dominik Csapak wrote:
>> +		    if (!$extraction_scfg->{content}->{images} || !$extraction_scfg->{path}) {
> 
> I think the if condition here is grouped wrong.
> 
> As it is, once if one of the items (content->images or path) is falsy, 
> we trigger it, but it should not be the case, if the other is actually true.

how so? It must be both, file based and allow the import content type?

> Consider the following diff, basically grouping the content->images || 
> path condition, and only if the result is falsy, we want to raise the error.
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 8db443d..545fe31 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -179,7 +179,7 @@ my $check_storage_access = sub {
>                          $scfg;
>                      my $extraction_param = defined($extraction_storage) ? 'import-working-storage' : $ds;
> 
> -                   if (!$extraction_scfg->{content}->{images} || !$extraction_scfg->{path}) {
> +                   if (!($extraction_scfg->{content}->{images} || $extraction_scfg->{path})) {

Besides above, the error would now be wrong too, it states:

"storage selected for extraction does not support 'images' content type or is not file based"

(not (support 'images' content type)) or is (not (file based))

Exactly what Dominik used.

The fix here would add the 'images' content-type as supported one for
CephFS.


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