[pve-devel] [PATCH storage 08/26] prepare for vm-vol and ct-vol content and vtypes

Fabian Grünbichler f.gruenbichler at proxmox.com
Wed Jul 30 10:38:19 CEST 2025


On July 29, 2025 1:15 pm, Wolfgang Bumiller wrote:
> Prepares the stoplevel PVE::Storage API updates as well as adding the
> new vtype subdirs to the base plugin's vtype subdir hash.
> 
> The new types are "vm-vol" and "ct-vol". They represent VM and
> container volumes, respectively. The "images" and "rootdir" types are
> considered legacy/deprecated, as the "rootdir" type was not properly
> used, and container volumes were technically of type "images", with
> the "rootdir" case "hacked in" by checking the existing VMs.
> 
> To more easily transition, the "images" type is now also a "supertype"
> of "vm-vol", and the "rootdir" type a "supertype" of "ct-vol".
> 
> - `get_images_dir()` is replaced by `get_vm_volume_dir()`
> - `get_private_dir()` is dropped as it is an openvz leftover
> - `get_ct_volume_dir()` is added its stead
> 
> We now also unify the vtypes and content types. As such,
> `content-dirs` can now include separate dirs for `vm-vol` and
> `ct-vol`.
> This is now also taken into account in `path_to_volume_id()` which
> tries to match file system paths to a storage and content type.
> 
> The following subs also get a $vtype parameter:
> - `vdisk_alloc()`
> - `vdisk_clone()`
> - `volume_import()`
> 
> `volume_list()`'s `$content` parameter allows `vm-vol` and
> `ct-vol` as type.
> 
> New helpers are added:
> - is_content_type_covered($content_hash, $content_type)
>   Check if the contents listed in the hash(set) allows the provided
>   $content_type.
>   Meaning the content type is in the set *or* as supertype is in the
>   set (see above).
> - storage_check_type_allowed($cfg, $storeid, $type [, $noerr])
>   Access to the above in our usual public storag API signature.
> 
> Finally: the content type completion also gets the new content types.
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> ---
>  src/PVE/GuestImport.pm    |   2 +-
>  src/PVE/Storage.pm        | 113 ++++++++++++++++++++++++++++++--------
>  src/PVE/Storage/Plugin.pm |   4 +-
>  3 files changed, 95 insertions(+), 24 deletions(-)
> 
> diff --git a/src/PVE/GuestImport.pm b/src/PVE/GuestImport.pm
> index 3d59dcd..ec2f09d 100644
> --- a/src/PVE/GuestImport.pm
> +++ b/src/PVE/GuestImport.pm
> @@ -40,7 +40,7 @@ sub extract_disk_from_import_file {
>  
>      my $ova_path = PVE::Storage::path($cfg, $archive_volid);
>  
> -    my $tmpdir = PVE::Storage::get_image_dir($cfg, $target_storeid, $vmid);
> +    my $tmpdir = PVE::Storage::get_vm_volume_dir($cfg, $target_storeid, $vmid);
>      my $pid = $$;
>      $tmpdir .= "/tmp_${pid}_${vmid}";
>      mkpath $tmpdir;
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index da53beb..91a0278 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -547,24 +547,24 @@ sub volume_snapshot_info {
>      return $plugin->volume_snapshot_info($scfg, $storeid, $volname);
>  }
>  
> -sub get_image_dir {
> +sub get_vm_volume_dir {
>      my ($cfg, $storeid, $vmid) = @_;
>  
>      my $scfg = storage_config($cfg, $storeid);
>      my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>  
> -    my $path = $plugin->get_subdir($scfg, 'images');
> +    my $path = $plugin->get_subdir($scfg, 'vm-vol');
>  
>      return $vmid ? "$path/$vmid" : $path;
>  }

this one is only used by GuestImport above AFAICT, should we use this
opportunity and give it a better name to make it clear that this should
not be used in general? we use it there to get a base path for creating
a tmpdir inside for extracting the OVA, and might want to switch to some
other subdir in the future..

>  
> -sub get_private_dir {
> +sub get_ct_volume_dir {
>      my ($cfg, $storeid, $vmid) = @_;
>  
>      my $scfg = storage_config($cfg, $storeid);
>      my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>  
> -    my $path = $plugin->get_subdir($scfg, 'rootdir');
> +    my $path = $plugin->get_subdir($scfg, 'ct-vol');
>  
>      return $vmid ? "$path/$vmid" : $path;
>  }

this one is not used anywhere, should we drop it?

> @@ -737,22 +737,26 @@ sub path_to_volume_id {
>          next if !$scfg->{path};
>          my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>          my $imagedir = $plugin->get_subdir($scfg, 'images');
> +        my $vmdir = $plugin->get_subdir($scfg, 'vm-vol');
> +        my $ctdir = $plugin->get_subdir($scfg, 'ct-vol');
>          my $isodir = $plugin->get_subdir($scfg, 'iso');
>          my $tmpldir = $plugin->get_subdir($scfg, 'vztmpl');
>          my $backupdir = $plugin->get_subdir($scfg, 'backup');
>          my $snippetsdir = $plugin->get_subdir($scfg, 'snippets');
>          my $importdir = $plugin->get_subdir($scfg, 'import');
>  
> -        if ($path =~ m!^\Q$imagedir\E/(\d+)/([^/\s]+)$!) {
> -            my $vmid = $1;
> -            my $name = $2;
> +        if ($path =~ m!^(\Q$imagedir\E|\Q$vmdir\E|\Q$ctdir\E)/(\d+)/([^/\s]+)$!) {
> +            my $subdir = $1;
> +            my $vmid = $2;
> +            my $name = $3;
>  
>              my $vollist = $plugin->list_images($sid, $scfg, $vmid);
>              foreach my $info (@$vollist) {
>                  my ($storeid, $volname) = parse_volume_id($info->{volid});
> +                my ($vtype) = parse_volname($cfg, $info->{volid});
>                  my $volpath = $plugin->path($scfg, $volname, $storeid);
>                  if ($volpath eq $path) {
> -                    return ('images', $info->{volid});
> +                    return ($vtype, $info->{volid});
>                  }
>              }
>          } elsif ($path =~ m!^\Q$isodir\E/([^/]+$ISO_EXT_RE_0)$!) {
> @@ -822,7 +826,7 @@ sub qemu_blockdev_options {
>  
>      my ($vtype) = $plugin->parse_volname($volname);
>      die "cannot use volume of type '$vtype' as a QEMU blockdevice\n"
> -        if $vtype ne 'images' && $vtype ne 'iso' && $vtype ne 'import';
> +        if $vtype ne 'vm-vol' && $vtype ne 'images' && $vtype ne 'iso' && $vtype ne 'import';
>  
>      my $blockdev =
>          $plugin->qemu_blockdev_options($scfg, $storeid, $volname, $machine_version, $options);
> @@ -884,6 +888,7 @@ my $volume_import_prepare = sub {
>      my $with_snapshots = $opts->{with_snapshots} ? 1 : 0;
>      my $migration_snapshot = $opts->{migration_snapshot} ? 1 : 0;
>      my $allow_rename = $opts->{allow_rename} ? 1 : 0;
> +    my $vtype = $opts->{vtype};
>  
>      my $recv = ['pvesm', 'import', $volid, $format, $path, '-with-snapshots', $with_snapshots];
>      if (defined($snapshot)) {
> @@ -892,6 +897,9 @@ my $volume_import_prepare = sub {
>      if ($migration_snapshot) {
>          push @$recv, '-delete-snapshot', $snapshot;
>      }
> +    if ($vtype) {
> +        push @$recv, '-vtype', $vtype;
> +    }

should we skip this if `$vtype` is `images`, to improve backwards
compat? otherwise this would break migrating back from new to old nodes,
right?

>      push @$recv, '-allow-rename', $allow_rename;
>  
>      if (defined($base_snapshot)) {
> @@ -1099,7 +1107,7 @@ sub storage_migrate {
>  }
>  
>  sub vdisk_clone {
> -    my ($cfg, $volid, $vmid, $snap) = @_;
> +    my ($cfg, $volid, $vmid, $snap, $vtype) = @_;
>  
>      my ($storeid, $volname) = parse_volume_id($volid);
>  
> @@ -1115,7 +1123,7 @@ sub vdisk_clone {
>          $scfg->{shared},
>          undef,
>          sub {
> -            my $volname = $plugin->clone_image($scfg, $storeid, $volname, $vmid, $snap);
> +            my $volname = $plugin->clone_image($scfg, $storeid, $volname, $vmid, $snap, $vtype);
>              return "$storeid:$volname";
>          },
>      );
> @@ -1168,8 +1176,52 @@ sub unmap_volume {
>      return $plugin->unmap_volume($storeid, $scfg, $volname, $snapname);
>  }
>  
> +=head3 is_content_type_covered($content_hash, $content_type)
> +
> +Check if the C<$content_hash> allows content of type C<$content_type>.
> +
> +Note that the legacy types C<images> and C<rootdir> will allow content of type C<vm-vol> and
> +C<ct-vol> respectively, but not the other way round.
> +
> +For anything else this just checks whether C<$content_hash->{$content_type}> is set.
> +
> +=cut
> +
> +my sub is_content_type_covered : prototype($$) {
> +    my ($content_hash, $content_type) = @_;
> +
> +    return
> +        $content_hash->{$content_type}
> +        || ($content_type eq 'vm-vol' && $content_hash->{images})
> +        || ($content_type eq 'ct-vol' && $content_hash->{rootdir});
> +}
> +
> +=head3 storage_check_type_allowed($cfg, $storeid, $type [, $noerr])

should this be called by qemu-server? see comments there ;)

> +
> +Check if a storage allows content of type C<$type>.
> +
> +Note that the legacy types C<images> and C<rootdir> will allow content of type C<vm-vol> and
> +C<ct-vol> respectively, but not the other way round.
> +
> +If C<$noerr> is true, returns true if the type is allowed, false otherwise.
> +If C<$noerr> is false, dies if the type is not allowed, returns true otherwise.
> +
> +=cut
> +
> +sub storage_check_type_allowed : prototype($$$;$) {
> +    my ($cfg, $storeid, $content_type, $noerr) = @_;
> +
> +    my $scfg = storage_config($cfg, $storeid);
> +
> +    return !!1 if is_content_type_covered($scfg->{content}, $content_type);
> +    return !!0 if $noerr;
> +    die "storage '$storeid' doees not allow content of type '$content_type'\n";

typo 'doees'

> +}
> +
>  sub vdisk_alloc {
> -    my ($cfg, $storeid, $vmid, $fmt, $name, $size) = @_;
> +    my ($cfg, $storeid, $vmid, $fmt, $name, $size, $vtype) = @_;
> +
> +    die "vdisk_alloc without vtype not allowed anymore\n" if !defined($vtype);
>  
>      die "no storage ID specified\n" if !$storeid;
>  
> @@ -1187,6 +1239,8 @@ sub vdisk_alloc {
>  
>      activate_storage($cfg, $storeid);
>  
> +    storage_check_type_allowed($cfg, $storeid, $vtype);
> +
>      # lock shared storage
>      return $plugin->cluster_lock_storage(
>          $storeid,
> @@ -1195,7 +1249,7 @@ sub vdisk_alloc {
>          sub {
>              my $old_umask = umask(umask | 0037);
>              my $volname =
> -                eval { $plugin->alloc_image($storeid, $scfg, $vmid, $fmt, $name, $size) };
> +                eval { $plugin->alloc_image($storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) };
>              my $err = $@;
>              umask $old_umask;
>              die $err if $err;
> @@ -1263,8 +1317,10 @@ sub vdisk_list {
>              next if $storeid && $storeid ne $sid;
>              next if !storage_check_enabled($cfg, $sid, undef, 1);
>              my $content = $ids->{$sid}->{content};
> -            next if defined($ctype) && !$content->{$ctype};
> -            next if !($content->{rootdir} || $content->{images});
> +            next if defined($ctype) && !is_content_type_covered($content, $ctype);
> +            next
> +                if !(is_content_type_covered($content, 'vm-vol')
> +                    || is_content_type_covered($content, 'ct-vol'));
>              push @$storage_list, $sid;
>          }
>      }
> @@ -1278,7 +1334,7 @@ sub vdisk_list {
>  
>          my $scfg = $ids->{$sid};
>          my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
> -        $res->{$sid} = $plugin->list_images($sid, $scfg, $vmid, $vollist, $cache);
> +        $res->{$sid} = $plugin->list_images($sid, $scfg, $vmid, $vollist, $cache, $ctype);
>          @{ $res->{$sid} } = sort { lc($a->{volid}) cmp lc($b->{volid}) } @{ $res->{$sid} }
>              if $res->{$sid};
>      }
> @@ -1318,13 +1374,15 @@ sub template_list {
>  sub volume_list {
>      my ($cfg, $storeid, $vmid, $content) = @_;
>  
> +    # We don't need to extend this by 'vm-vol' and 'ct-vol' as they are included by 'images' and
> +    # 'rootdir'.
>      my @ctypes = qw(rootdir images vztmpl iso backup snippets import);

I am not sure that's a good approach - we want to sunset 'images' and
'rootdir' at some point, and we don't allow users to opt-into not
allowing the legacy types if we do this?

>  
>      my $cts = $content ? [$content] : [@ctypes];
>  
>      my $scfg = PVE::Storage::storage_config($cfg, $storeid);
>  
> -    $cts = [grep { defined($scfg->{content}->{$_}) } @$cts];
> +    $cts = [grep { is_content_type_covered($scfg->{content}, $_) } @$cts];
>  
>      my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>  
> @@ -2121,8 +2179,18 @@ sub volume_export : prototype($$$$$$$) {
>      );
>  }
>  
> -sub volume_import : prototype($$$$$$$$) {
> -    my ($cfg, $fh, $volid, $format, $snapshot, $base_snapshot, $with_snapshots, $allow_rename) = @_;
> +sub volume_import : prototype($$$$$$$$$) {
> +    my (
> +        $cfg,
> +        $fh,
> +        $volid,
> +        $format,
> +        $snapshot,
> +        $base_snapshot,
> +        $with_snapshots,
> +        $allow_rename,
> +        $import_vtype,
> +    ) = @_;
>  
>      my ($storeid, $volname) = parse_volume_id($volid, 1);
>      die "cannot import into volume '$volid'\n" if !$storeid;
> @@ -2138,6 +2206,7 @@ sub volume_import : prototype($$$$$$$$) {
>          $base_snapshot,
>          $with_snapshots,
>          $allow_rename,
> +        $import_vtype,
>      ) // $volid;
>  }
>  
> @@ -2288,7 +2357,7 @@ sub complete_storage_enabled {
>  sub complete_content_type {
>      my ($cmdname, $pname, $cvalue) = @_;
>  
> -    return [qw(rootdir images vztmpl iso backup snippets)];
> +    return [qw(vm-vol ct-vol rootdir images vztmpl iso backup snippets)];
>  }
>  
>  sub complete_volume {
> @@ -2326,7 +2395,7 @@ sub complete_volume {
>  }
>  
>  sub rename_volume {
> -    my ($cfg, $source_volid, $target_vmid, $target_volname) = @_;
> +    my ($cfg, $source_volid, $target_vmid, $target_volname, $vtype) = @_;
>  
>      die "no source volid provided\n" if !$source_volid;
>      die "no target VMID or target volname provided\n" if !$target_vmid && !$target_volname;
> @@ -2346,7 +2415,7 @@ sub rename_volume {
>          undef,
>          sub {
>              return $plugin->rename_volume(
> -                $scfg, $storeid, $source_volname, $target_vmid, $target_volname,
> +                $scfg, $storeid, $source_volname, $target_vmid, $target_volname, $vtype,
>              );
>          },
>      );
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 0107976..ae9d673 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -771,7 +771,9 @@ sub parse_volname {
>  
>  my $vtype_subdirs = {
>      images => 'images',
> -    rootdir => 'private',
> +    rootdir => 'images',
> +    'vm-vol' => 'vms',
> +    'ct-vol' => 'cts',
>      iso => 'template/iso',
>      vztmpl => 'template/cache',
>      backup => 'dump',
> -- 
> 2.47.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




More information about the pve-devel mailing list