[pve-devel] [PATCH storage] check volume access: handle new 'ct-vol' and 'vm-vol' vtypes

[Fiona Ebner]

In the tests, legacy volids resulting in 'images' vtype will be
allowed for both 'ct-vol' and 'vm-vol'.

New test cases for new-form guest image volids are added too.

Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
---

Is based on top of Wolfgang's staff repo as well as [0].

[0]: https://lore.proxmox.com/pve-devel/20250730130506.96278-1-f.ebner@proxmox.com/T/#u

 src/PVE/Storage.pm                  | 31 ++++++++++++++++++---
 src/test/run_volume_access_tests.pl | 43 +++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 91a0278..e6dabc3 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -646,9 +646,32 @@ sub check_volume_access {
     if ($sid) {
         my ($vtype, undef, $ownervm) = parse_volname($cfg, $volid);
 
-        # Need to allow 'images' when expecting 'rootdir' too - not cleanly separated in plugins.
-        die "unable to use volume $volid - content type needs to be '$type'\n"
-            if defined($type) && $vtype ne $type && ($type ne 'rootdir' || $vtype ne 'images');
+        my $is_guest_image_vtype =
+            $vtype eq 'ct-vol' || $vtype eq 'vm-vol' || $vtype eq 'images' || $vtype eq 'rootdir';
+
+        if (defined($type)) {
+            my $msg = "unable to use volume $volid - volume type needs to be '$type'";
+
+            # TODO PVE 10.x die or remove completely, once all callers are adapted
+            $type = 'vm-vol' if $type eq 'images';
+            $type = 'ct-vol' if $type eq 'rootdir';
+
+            if ($is_guest_image_vtype) {
+                if ($type eq 'vm-vol') {
+                    die "$msg or 'images'\n" if $vtype ne $type && $vtype ne 'images';
+                } elsif ($type eq 'ct-vol') {
+                    # need to allow 'images' when expecting 'ct-vol' too for backwards-compat
+                    die "$msg or 'rootdir' or 'images'\n"
+                        if $vtype ne $type && $vtype ne 'rootdir' && $vtype ne 'images';
+                } else {
+                    # $type is not for guest-images, but $vtype is, disallow
+                    die "$msg\n";
+                }
+            } else {
+                # can check directly
+                die "$msg\n" if $vtype ne $type;
+            }
+        }
 
         return if $rpcenv->check($user, "/storage/$sid", ['Datastore.Allocate'], 1);
 
@@ -664,7 +687,7 @@ sub check_volume_access {
         } elsif ($vtype eq 'backup' && $ownervm) {
             $rpcenv->check($user, "/storage/$sid", ['Datastore.AllocateSpace']);
             $rpcenv->check($user, "/vms/$ownervm", ['VM.Backup']);
-        } elsif (($vtype eq 'images' || $vtype eq 'rootdir') && $ownervm) {
+        } elsif ($is_guest_image_vtype && $ownervm) {
             $rpcenv->check($user, "/storage/$sid", ['Datastore.Audit']);
             $rpcenv->check($user, "/vms/$ownervm", ['VM.Config.Disk']);
         } else {
diff --git a/src/test/run_volume_access_tests.pl b/src/test/run_volume_access_tests.pl
index ce9fc2e..9744b65 100755
--- a/src/test/run_volume_access_tests.pl
+++ b/src/test/run_volume_access_tests.pl
@@ -78,6 +78,28 @@ my @tests = (
             'backup' => 1,
         },
     },
+    {
+        volid => 'dir:100/vol-vm-100-disk-0.qcow2',
+        denied_users => {
+            'backup at pve' => 1,
+            'dsaudit at pve' => 1,
+        },
+        allowed_types => {
+            'images' => 1,
+            'vm-vol' => 1,
+        },
+    },
+    {
+        volid => 'dir:100/vol-ct-100-disk-0.raw',
+        denied_users => {
+            'backup at pve' => 1,
+            'dsaudit at pve' => 1,
+        },
+        allowed_types => {
+            'rootdir' => 1,
+            'ct-vol' => 1,
+        },
+    },
     {
         volid => 'dir:100/vm-100-disk-0.qcow2',
         denied_users => {
@@ -87,6 +109,8 @@ my @tests = (
         allowed_types => {
             'images' => 1,
             'rootdir' => 1,
+            'ct-vol' => 1,
+            'vm-vol' => 1,
         },
     },
     {
@@ -105,6 +129,17 @@ my @tests = (
             'iso' => 1,
         },
     },
+    {
+        volid => 'dir:111/subvol-ct-111-disk-0.subvol',
+        denied_users => {
+            'backup at pve' => 1,
+            'dsaudit at pve' => 1,
+        },
+        allowed_types => {
+            'rootdir' => 1,
+            'ct-vol' => 1,
+        },
+    },
     {
         volid => 'dir:111/subvol-111-disk-0.subvol',
         denied_users => {
@@ -114,6 +149,8 @@ my @tests = (
         allowed_types => {
             'images' => 1,
             'rootdir' => 1,
+            'ct-vol' => 1,
+            'vm-vol' => 1,
         },
     },
     # test different VM IDs
@@ -138,6 +175,8 @@ my @tests = (
         allowed_types => {
             'images' => 1,
             'rootdir' => 1,
+            'ct-vol' => 1,
+            'vm-vol' => 1,
         },
     },
     {
@@ -155,6 +194,8 @@ my @tests = (
         allowed_types => {
             'images' => 1,
             'rootdir' => 1,
+            'ct-vol' => 1,
+            'vm-vol' => 1,
         },
     },
     {
@@ -184,6 +225,8 @@ my @tests = (
         allowed_types => {
             'images' => 1,
             'rootdir' => 1,
+            'ct-vol' => 1,
+            'vm-vol' => 1,
         },
     },
     # test paths
-- 
2.47.2