On  2024-11-18  16:29, Dominik Csapak wrote:
this is to override the target extraction storage for the option disk
extraction for 'import-from'. This way if the storage does not
supports the content type 'images', one can give an alternative  one.

Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
---
changes from v6:
* rename 'import-extraction-storage' to 'import-working-storage'
* rework permission checks (single branch)

i opted to not make the target the first default, since i did not want
to introduce such a change this late in the patch/review cycle and
AFAICS quite a few code changes would have been necessary for that
(we can still change that later too)

  PVE/API2/Qemu.pm | 48 +++++++++++++++++++++++++++++++++++++++---------
  1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index c947f09c..701558a7 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -132,7 +132,7 @@ my $check_drive_param = sub {
  };
my $check_storage_access = sub {
-   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
+   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage, 
$extraction_storage) = @_;
$foreach_volume_with_alloc->($settings, sub {
        my ($ds, $drive) = @_;
@@ -174,9 +174,18 @@ my $check_storage_access = sub {
                    if $vtype ne 'images' && $vtype ne 'import';
if (PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt)) {
-                   raise_param_exc({ $ds => "$src_image is not on an storage with 
'images' content type."})
-                       if !$scfg->{content}->{images};
-                   $rpcenv->check($authuser, "/storage/$storeid", 
['Datastore.AllocateSpace']);
+                   my $extraction_scfg = defined($extraction_storage) ?
+                       PVE::Storage::storage_config($storecfg, 
$extraction_storage) :
+                       $scfg;
+                   my $extraction_param = defined($extraction_storage) ? 
'import-working-storage' : $ds;
+
+                   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.

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})) {
                        raise_param_exc({
$extraction_param => "storage selected for extraction does not support" ." 'images' content type or is not file based.",



+                       raise_param_exc({
+                           $extraction_param => "storage selected for extraction 
does not support"
+                               ." 'images' content type or is not file based.",
+                       });
+                   }
+                   $rpcenv->check($authuser, "/storage/" . 
($extraction_storage // $storeid), ['Datastore.AllocateSpace']);
                }
            }
@@ -349,7 +358,7 @@ my sub prohibit_tpm_version_change {


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

Reply via email to