lore.proxmox.com seems to show the diff wrong...
On 2024-11-18 18:24, Aaron Lauterer wrote:
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.",
Lets try it once more, hopefully the line endings will be as I want
them, even on lore.proxmox.com
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.",
Also, I was able to run into this by uploading the OVA to a CephFS.
+ 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
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
