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 [212.224.123.68])
by lore.proxmox.com (Postfix) with ESMTPS id 5D4E31FF16F
for <inbox@lore.proxmox.com>; Tue, 15 Apr 2025 15:52:55 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 417AF9B89;
Tue, 15 Apr 2025 15:51:34 +0200 (CEST)
From: Daniel Kral <d.kral@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Tue, 15 Apr 2025 15:50:22 +0200
Message-Id: <20250415135045.255272-5-d.kral@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250415135045.255272-1-d.kral@proxmox.com>
References: <20250415135045.255272-1-d.kral@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.014 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: [pve-devel] [PATCH storage v3 4/4] vdisk_alloc: add assertion for
volume's 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-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Require a caller of vdisk_alloc() to specify the volume's intended
content type and assert whether the specified content type may be stored
on the specified storage before allocating any volume.
The 'any' volume type is for backwards compatibility of external
callers, whose API would break if content type assertions would be
added.
The two affected pve-storage code paths of this change are the API
handler for the `pvesm alloc` endpoint, which fallbacks to 'images' or
'rootdir' depending which is supported for the storage and errors out if
neither is available, and the extract_disk_from_import_file subroutine,
which is assumed to always create a VM image for now, as its only caller
is for VM images currently.
This requires a versioned break for packages where vdisk_alloc() is
called from, which are currently qemu-server and pve-container.
Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
changes since v2:
* make $vtype a required parameter now
* add better documentation about the $vtype's possible values
* do only allow $vtype to be 'images', 'rootdir', 'import' and 'any'
* add the 'any' content type for backwards compatibility until PVE 9
* add commit message description about 'any' content type and changes
do existing code paths
* add fallback logic in `pvesm alloc` for retrieving a content type
src/PVE/API2/Storage/Content.pm | 13 ++++++++++++-
src/PVE/GuestImport.pm | 3 ++-
src/PVE/Storage.pm | 20 +++++++++++++++-----
src/test/run_test_zfspoolplugin.pl | 12 ++++++++----
4 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm
index 64ed56d..7cfb9c1 100644
--- a/src/PVE/API2/Storage/Content.pm
+++ b/src/PVE/API2/Storage/Content.pm
@@ -217,9 +217,20 @@ __PACKAGE__->register_method ({
}
my $cfg = PVE::Storage::config();
+ my $scfg = PVE::Storage::storage_config($cfg, $storeid);
+
+ # FIXME Replace fallback logic with scoped volume names
+ my $vtype;
+ $vtype = 'images' if defined($scfg->{content}->{images});
+ $vtype = 'rootdir' if defined($scfg->{content}->{rootdir});
+ if (!$vtype) {
+ raise_param_exc({
+ storage => "'$storeid' does neither support content type 'images' nor 'rootdir'"
+ });
+ }
my $volid = PVE::Storage::vdisk_alloc(
- $cfg, $storeid, $param->{vmid}, $param->{format}, $size, { name => $name });
+ $cfg, $storeid, $param->{vmid}, $param->{format}, $vtype, $size, { name => $name });
return $volid;
}});
diff --git a/src/PVE/GuestImport.pm b/src/PVE/GuestImport.pm
index 76ebc9d..a285043 100644
--- a/src/PVE/GuestImport.pm
+++ b/src/PVE/GuestImport.pm
@@ -69,7 +69,8 @@ sub extract_disk_from_import_file {
# create temporary 1M image that will get overwritten by the rename
# to reserve the filename and take care of locking
- $target_volid = PVE::Storage::vdisk_alloc($cfg, $target_storeid, $vmid, $inner_fmt, 1024);
+ $target_volid = PVE::Storage::vdisk_alloc(
+ $cfg, $target_storeid, $vmid, $inner_fmt, 'images', 1024);
$target_path = PVE::Storage::path($cfg, $target_volid);
print "renaming $source_path to $target_path\n";
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index ab4dcdd..f089777 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -1056,16 +1056,20 @@ sub unmap_volume {
return $plugin->unmap_volume($storeid, $scfg, $volname, $snapname);
}
-=head3 vdisk_alloc($cfg, $storeid, $vmid, $fmt, $size [, %opts])
+=head3 vdisk_alloc($cfg, $storeid, $vmid, $fmt, $vtype, $size [, %opts])
Allocates a volume on the storage with the identifier C<$storeid> (defined in
-the storage config C<$cfg>) for the VM with the id C<$vmid> with format C<$fmt>
-and a size of C<$size> kilobytes.
+the storage config C<$cfg>) for the VM with the id C<$vmid> with format C<$fmt>,
+volume type C<$vtype> and a size of C<$size> kilobytes.
The format C<$fmt> can be C<'raw'>, C<'qcow2'>, C<'subvol'> or C<'vmdk'>. If
C<$fmt> is left undefined, it will fallback on the default format of the storage
type of C<$storeid>.
+The volume type C<$vtype> can be C<'images'>, C<'rootdir'>, C<'import'>, or
+C<'any'>, where the last allows the allocation of any volume type. C<'any'> will
+be removed in PVE 9, so do not rely on this value for new code.
+
Optionally, the following key-value pairs are available for C<%opts>:
=over
@@ -1082,8 +1086,8 @@ Returns the identifier for the new volume in the format C<"$storeid:$volname">.
=cut
-sub vdisk_alloc : prototype($$$$$;%) {
- my ($cfg, $storeid, $vmid, $fmt, $size, $opts) = @_;
+sub vdisk_alloc : prototype($$$$$$;%) {
+ my ($cfg, $storeid, $vmid, $fmt, $vtype, $size, $opts) = @_;
my $name = $opts->{name};
@@ -1103,6 +1107,12 @@ sub vdisk_alloc : prototype($$$$$;%) {
activate_storage($cfg, $storeid);
+ die "no volume type specified\n" if !$vtype;
+ die "$vtype is not a valid volume type for vdisk_alloc\n"
+ if $vtype !~ m/(any|images|import|rootdir)/;
+
+ assert_content_type_available($cfg, $storeid, $vtype) if $vtype ne 'any';
+
my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
# lock shared storage
diff --git a/src/test/run_test_zfspoolplugin.pl b/src/test/run_test_zfspoolplugin.pl
index 7dd0a2b..c85ee2c 100755
--- a/src/test/run_test_zfspoolplugin.pl
+++ b/src/test/run_test_zfspoolplugin.pl
@@ -565,7 +565,8 @@ my $test13 = sub {
print "\nrun test13 \"vdisk_alloc\"\n";
eval {
- my $tmp_volid = PVE::Storage::vdisk_alloc($cfg, $storagename, "112", "raw", 1024 * 1024);
+ my $tmp_volid = PVE::Storage::vdisk_alloc(
+ $cfg, $storagename, "112", "raw", "images", 1024 * 1024);
if ($tmp_volid ne "$storagename:vm-112-disk-0") {
die "volname:$tmp_volid don't match\n";
@@ -589,7 +590,8 @@ my $test13 = sub {
}
eval {
- my $tmp_volid = PVE::Storage::vdisk_alloc($cfg, $storagename, "112", "raw", 2048 * 1024);
+ my $tmp_volid = PVE::Storage::vdisk_alloc(
+ $cfg, $storagename, "112", "raw", "images", 2048 * 1024);
if ($tmp_volid ne "$storagename:vm-112-disk-1") {
die "volname:$tmp_volid don't match\n";
@@ -613,7 +615,8 @@ my $test13 = sub {
}
eval {
- my $tmp_volid = PVE::Storage::vdisk_alloc($cfg, $storagename, "113", "subvol", 1024 * 1024);
+ my $tmp_volid = PVE::Storage::vdisk_alloc(
+ $cfg, $storagename, "113", "subvol", "images", 1024 * 1024);
if ($tmp_volid ne "$storagename:subvol-113-disk-0") {
die "volname:$tmp_volid don't match\n";
@@ -637,7 +640,8 @@ my $test13 = sub {
}
eval {
- my $tmp_volid = PVE::Storage::vdisk_alloc($cfg, $storagename, "113", "subvol", 2048 * 1024);
+ my $tmp_volid = PVE::Storage::vdisk_alloc(
+ $cfg, $storagename, "113", "subvol", "images", 2048 * 1024);
if ($tmp_volid ne "$storagename:subvol-113-disk-1") {
die "volname:$tmp_volid don't match\n";
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel