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 A76431FF56B
	for <inbox@lore.proxmox.com>; Mon, 22 Apr 2024 13:10:22 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 1C7E8EEA0;
	Mon, 22 Apr 2024 13:10:25 +0200 (CEST)
Message-ID: <98b79c87-5721-431e-b6a7-f5280ff4d0f4@proxmox.com>
Date: Mon, 22 Apr 2024 13:09:51 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird Beta
To: Fiona Ebner <f.ebner@proxmox.com>,
 Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20240419094613.1427891-1-d.csapak@proxmox.com>
 <20240419094613.1427891-3-d.csapak@proxmox.com>
 <2c8a80ee-2eb1-42f1-8d34-c8851ddcbd9a@proxmox.com>
Content-Language: en-US
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <2c8a80ee-2eb1-42f1-8d34-c8851ddcbd9a@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.015 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
 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 storage v2 02/10] plugin: dir: implement
 import content type
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 4/22/24 13:00, Fiona Ebner wrote:
> Am 19.04.24 um 11:45 schrieb Dominik Csapak:
>> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
>> index 22a9729..39a8354 100644
>> --- a/src/PVE/Storage/Plugin.pm
>> +++ b/src/PVE/Storage/Plugin.pm
>> @@ -654,6 +654,10 @@ sub parse_volname {
>>   	return ('backup', $fn);
>>       } elsif ($volname =~ m!^snippets/([^/]+)$!) {
>>   	return ('snippets', $1);
>> +    } elsif ($volname =~ m!^import/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!) {
>> +	return ('import', $1);
> 
> Wouldn't it be nicer to return 'ovf' and 'ova' as the $file_format here
> and check for that at the call sites? Currently you rely on the presence
> or absence of $file_format in copy_needs_extraction() and
> get_import_metadata() and then re-match on the ova extension. Having the
> format right away would be a bit cleaner and more future-proof or is
> there a specific reason against doing it?

i explained it in either the cover letter or in some commit message, probably would have fit
in here too:

we currently only support raw/vmdk/qcow2/subvol here and it is intended only for image formats
IIUC. Also we would have to adapt the `verify_format` for that, since it will be
tested by that at least once. (not sure where exactly though, found out by testing)
and that would mean we could set it as 'default' format on the storage, which is not what we want...

> 
>> +    } elsif ($volname =~ m!^import/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+\.(raw|vmdk|qcow2))$!) {
>> +	return ('import', $1, undef, undef, undef, undef, $2);
>>       }
>>   
>>       die "unable to parse directory volume name '$volname'\n";



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