From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 70EC61FF15F
for <inbox@lore.proxmox.com>; Mon, 18 Nov 2024 18:24:53 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 9041A16142;
Mon, 18 Nov 2024 18:24:56 +0100 (CET)
Message-ID: <683efffa-4ef1-4295-bb13-6828026e1f6c@proxmox.com>
Date: Mon, 18 Nov 2024 18:24:52 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Dominik Csapak <d.csapak@proxmox.com>
References: <20241118152928.858590-1-d.csapak@proxmox.com>
<20241118152928.858590-16-d.csapak@proxmox.com>
From: Aaron Lauterer <a.lauterer@proxmox.com>
In-Reply-To: <20241118152928.858590-16-d.csapak@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.035 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH qemu-server v7 4/5] api: create: add
'import-working-storage' parameter
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
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.csapak@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