public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Max R. Carrara" <m.carrara@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH storage 17/26] plugins: add vtype parameter to alloc_image
Date: Wed, 30 Jul 2025 16:05:27 +0200	[thread overview]
Message-ID: <DBPG3SQ8DVME.19KHT6M53EKQW@proxmox.com> (raw)
In-Reply-To: <DBPG035CJU02.3A6DEBYHNJ80C@proxmox.com>

On Wed Jul 30, 2025 at 4:00 PM CEST, Max R. Carrara wrote:
> On Tue Jul 29, 2025 at 1:15 PM CEST, Wolfgang Bumiller wrote:
> > Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> > ---
> >  src/PVE/Storage/BTRFSPlugin.pm       | 25 ++++++++----
> >  src/PVE/Storage/ESXiPlugin.pm        |  2 +-
> >  src/PVE/Storage/ISCSIDirectPlugin.pm |  2 +-
> >  src/PVE/Storage/ISCSIPlugin.pm       |  2 +-
> >  src/PVE/Storage/LVMPlugin.pm         | 35 ++++++++++++-----
> >  src/PVE/Storage/LvmThinPlugin.pm     | 19 +++++++--
> >  src/PVE/Storage/Plugin.pm            | 59 +++++++++++++++++-----------
> >  src/PVE/Storage/RBDPlugin.pm         | 27 +++++++++----
> >  src/PVE/Storage/ZFSPlugin.pm         | 17 ++++++--
> >  src/PVE/Storage/ZFSPoolPlugin.pm     | 35 +++++++++++------
> >  10 files changed, 154 insertions(+), 69 deletions(-)
> >
> > diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
> > index 585489c..0a42c88 100644
> > --- a/src/PVE/Storage/BTRFSPlugin.pm
> > +++ b/src/PVE/Storage/BTRFSPlugin.pm
> > @@ -13,6 +13,7 @@ use POSIX qw(EEXIST);
> >  
> >  use PVE::Tools qw(run_command dir_glob_foreach);
> >  
> > +use PVE::Storage::Common;
> >  use PVE::Storage::DirPlugin;
> >  
> >  use constant {
> > @@ -193,15 +194,16 @@ sub filesystem_path {
> >  
> >      my $path = $class->get_subdir($scfg, $vtype);
> >  
> > -    $path .= "/$vmid" if $vtype eq 'images';
> > +    my $is_volume_type = PVE::Storage::Common::is_volume_type($vtype);
> > +    $path .= "/$vmid" if $is_volume_type;
> >  
> > -    if ($vtype eq 'images' && defined($format) && $format eq 'raw') {
> > +    if ($is_volume_type && defined($format) && $format eq 'raw') {
> >          my $dir = raw_name_to_dir($name);
> >          if ($snapname) {
> >              $dir .= "\@$snapname";
> >          }
> >          $path .= "/$dir/disk.raw";
> > -    } elsif ($vtype eq 'images' && defined($format) && $format eq 'subvol') {
> > +    } elsif ($is_volume_type && defined($format) && $format eq 'subvol') {
> >          $path .= "/$name";
> >          if ($snapname) {
> >              $path .= "\@$snapname";
> > @@ -322,19 +324,28 @@ sub clone_image {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> >      if ($fmt ne 'raw' && $fmt ne 'subvol') {
> > -        return $class->SUPER::alloc_image($storeid, $scfg, $vmid, $fmt, $name, $size);
> > +        return $class->SUPER::alloc_image($storeid, $scfg, $vmid, $fmt, $name, $size, $vtype);
> >      }
> >  
> >      # From Plugin.pm:
> >  
> > -    my $imagedir = $class->get_subdir($scfg, 'images') . "/$vmid";
> > +    my $imagedir = $class->get_subdir($scfg, $vtype // 'images') . "/$vmid";
> >  
> >      mkpath $imagedir;
> >  
> > -    $name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 1) if !$name;
> > +    if ($name) {
> > +        if (defined($vtype)) {
> > +            die "illegal name '$name' - should be 'vol-vm-$vmid-*'\n"
> > +                if $vtype eq 'vm-vol' && $name !~ m/^vol-vm-$vmid-/;
> > +            die "illegal name '$name' - should be 'vol-ct-$vmid-*'\n"
> > +                if $vtype eq 'ct-vol' && $name !~ m/^vol-ct-$vmid-/;
> > +        }
> > +    } else {
> > +        $name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 1, $vtype);
> > +    }
> >  
> >      my (undef, $tmpfmt) = PVE::Storage::Plugin::parse_name_dir($name);
> >  
> > diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
> > index ea2f8f9..d10b156 100644
> > --- a/src/PVE/Storage/ESXiPlugin.pm
> > +++ b/src/PVE/Storage/ESXiPlugin.pm
> > @@ -480,7 +480,7 @@ sub path {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> >      die "creating images is not supported for $class\n";
> >  }
> > diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
> > index 98a3391..0fee541 100644
> > --- a/src/PVE/Storage/ISCSIDirectPlugin.pm
> > +++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
> > @@ -140,7 +140,7 @@ sub clone_image {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> >      die "can't allocate space in iscsi storage\n";
> >  }
> > diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
> > index 77a9173..5118d73 100644
> > --- a/src/PVE/Storage/ISCSIPlugin.pm
> > +++ b/src/PVE/Storage/ISCSIPlugin.pm
> > @@ -402,7 +402,7 @@ sub clone_image {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> >      die "can't allocate space in iscsi storage\n";
> >  }
> > diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
> > index 55c4578..af789ba 100644
> > --- a/src/PVE/Storage/LVMPlugin.pm
> > +++ b/src/PVE/Storage/LVMPlugin.pm
> > @@ -535,7 +535,7 @@ sub clone_image {
> >  }
> >  
> >  sub find_free_diskname {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix, $vtype) = @_;
> >  
> >      my $vg = $scfg->{vgname};
> >  
> > @@ -546,7 +546,7 @@ sub find_free_diskname {
> >      $add_fmt_suffix = $fmt && $fmt eq 'qcow2' ? 1 : undef;
> >  
> >      return PVE::Storage::Plugin::get_next_vm_diskname(
> > -        $disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix,
> > +        $disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix, $vtype,
> >      );
> >  }
> >  
> > @@ -628,8 +628,8 @@ my sub calculate_lvm_size {
> >      return $info->{'fully-allocated'} / 1024;
> >  }
> >  
> > -my sub alloc_lvm_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $backing_snap) = @_;
> > +my sub alloc_lvm_image : prototype($$$$$$$$$) {
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $backing_snap, $vtype) = @_;
> >  
> >      die "unsupported format '$fmt'" if $fmt ne 'raw' && $fmt ne 'qcow2';
> >  
> > @@ -637,6 +637,20 @@ my sub alloc_lvm_image {
> >          if $fmt eq 'qcow2'
> >          && !$scfg->{'snapshot-as-volume-chain'};
> >  
> > +    if ($name) {
> > +        if (defined($vtype) && $vtype eq 'vm-vol') {
> > +            die "illegal name '$name' - should be 'vol-vm-$vmid-*'\n"
> > +                if $name !~ m/^vol-vm-$vmid-/;
> > +        } elsif (defined($vtype) && $vtype eq 'ct-vol') {
> > +            die "illegal name '$name' - should be 'vol-ct-$vmid-*'\n"
> > +                if $name !~ m/^vol-ct-$vmid-/;
> > +        } else {
> > +            die "illegal name '$name'"
> > +                . " - should be 'vm-$vmid-*', 'vol-vm-$vmid-*' or 'vol-ct-$vmid-*'\n"
> > +                if $name !~ m/^(?:vol-vm|vol-ct|vm)-$vmid-/;
> > +        }
> > +    }
> > +
> >      $class->parse_volname($name);
> >  
> >      my $vgs = lvm_vgs();
> > @@ -672,20 +686,20 @@ my sub alloc_lvm_image {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> > -    $name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
> > +    $name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 0, $vtype)
> >          if !$name;
> >  
> > -    alloc_lvm_image($class, $storeid, $scfg, $vmid, $fmt, $name, $size);
> > +    alloc_lvm_image($class, $storeid, $scfg, $vmid, $fmt, $name, $size, undef, $vtype);
> >  
> >      return $name;
> >  }
> >  
> > -my sub alloc_snap_image {
> > +my sub alloc_snap_image : prototype($$$$$) {
> >      my ($class, $storeid, $scfg, $volname, $backing_snap) = @_;
> >  
> > -    my ($vmid, $format) = ($class->parse_volname($volname))[2, 6];
> > +    my ($vtype, $vmid, $format) = ($class->parse_volname($volname))[0, 2, 6];
> >      my $path = $class->path($scfg, $volname, $storeid, $backing_snap);
> >  
> >      #we need to use same size than the backing image qcow2 virtual-size
> > @@ -694,7 +708,8 @@ my sub alloc_snap_image {
> >  
> >      $size = $size / 1024; #we use kb in lvcreate
> >  
> > -    alloc_lvm_image($class, $storeid, $scfg, $vmid, $format, $volname, $size, $backing_snap);
> > +    alloc_lvm_image($class, $storeid, $scfg, $vmid, $format, $volname, $size, $backing_snap,
> > +        $vtype);
> >  }
> >  
> >  my sub free_snap_image {
> > diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm
> > index f6615d9..2071822 100644
> > --- a/src/PVE/Storage/LvmThinPlugin.pm
> > +++ b/src/PVE/Storage/LvmThinPlugin.pm
> > @@ -122,12 +122,23 @@ my $set_lv_autoactivation = sub {
> >  };
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> >      die "unsupported format '$fmt'" if $fmt ne 'raw';
> >  
> > -    die "illegal name '$name' - should be 'vm-$vmid-*'\n"
> > -        if $name && $name !~ m/^vm-$vmid-/;
> > +    if ($name) {
> > +        if (defined($vtype) && $vtype eq 'vm-vol') {
> > +            die "illegal name '$name' - should be 'vol-vm-$vmid-*'\n"
> > +                if $name !~ m/^vol-vm-$vmid-/;
> > +        } elsif (defined($vtype) && $vtype eq 'ct-vol') {
> > +            die "illegal name '$name' - should be 'vol-ct-$vmid-*'\n"
> > +                if $name !~ m/^vol-ct-$vmid-/;
> > +        } else {
> > +            die "illegal name '$name'"
> > +                . " - should be 'vm-$vmid-*', 'vol-vm-$vmid-*' or 'vol-ct-$vmid-*'\n"
> > +                if $name !~ m/^(?:vol-vm|vol-ct|vm)-$vmid-/;
> > +        }
> > +    }
>
> ^ This currently trips up when you try to make a snapshot on a VM disk
> following the new naming scheme:
>
> TASK ERROR: illegal name 'vm-200-state-foo' - should be 'vol-vm-200-*'
>
> Did some debugging and stacktrace-diving--turns out that
> `PVE::QemuConfig::__snapshot_save_vmstate()` passes the wrong name for
> the snapshot.
>
> Should we keep the old snapshot naming scheme for 'vm-$vmid-*' volumes
> or also use the new one from now on?
>
> With that being said, perhaps this could be a good opportunity to let
> `PVE::Storage::vdisk_alloc()` decide on the snapshot's name instead?
> As in, have `__snapshot_save_vmstate()` just pass on the plain name,
> that is "foo" instead of e.g. "vm-666-state-foo" since the $vmid is
> passed along anyway (and the vtype now is, too).
>
> NOTE: This also happens for directory storage too, and I'm assuming
> others as well. However, containers seem to be fine..?

I forgot to mention: VM disks with the legacy naming scheme work fine.
Just double checked for CTs--CT disks with both the legacy naming and
new naming scheme work fine (on lvm-thin).

>
> >  
> >      my $vgs = PVE::Storage::LVMPlugin::lvm_vgs();
> >  
> > @@ -135,7 +146,7 @@ sub alloc_image {
> >  
> >      die "no such volume group '$vg'\n" if !defined($vgs->{$vg});
> >  
> > -    $name = $class->find_free_diskname($storeid, $scfg, $vmid)
> > +    $name = $class->find_free_diskname($storeid, $scfg, $vmid, undef, 0, $vtype)
> >          if !$name;
> >  
> >      my $cmd = [
> > diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> > index 660045d..adaca47 100644
> > --- a/src/PVE/Storage/Plugin.pm
> > +++ b/src/PVE/Storage/Plugin.pm
> > @@ -851,7 +851,7 @@ sub filesystem_path {
> >  
> >      my $dir = $class->get_subdir($scfg, $vtype);
> >  
> > -    $dir .= "/$vmid" if $vtype eq 'images';
> > +    $dir .= "/$vmid" if PVE::Storage::Common::is_volume_type($vtype);
> >  
> >      my $path = "$dir/$name";
> >  
> > @@ -912,30 +912,34 @@ sub create_base {
> >  }
> >  
> >  my $get_vm_disk_number = sub {
> > -    my ($disk_name, $scfg, $vmid, $suffix) = @_;
> > -
> > -    my $disk_regex = qr/(vm|base)-$vmid-disk-(\d+)$suffix/;
> > -
> > -    my $type = $scfg->{type};
> > -    my $def = { %{ $defaultData->{plugindata}->{$type} } };
> > -
> > -    my $valid = $def->{format}->[0];
> > -    if ($valid->{subvol}) {
> > -        $disk_regex = qr/(vm|base|subvol|basevol)-$vmid-disk-(\d+)/;
> > -    }
> > -
> > -    if ($disk_name =~ m/$disk_regex/) {
> > -        return $2;
> > +    my ($disk_volid, $scfg, $vmid, $suffix) = @_;
> > +
> > +    # Dir storage have a vmid/ subdir, zfs/lvm/rbd/... do not.
> > +    # Also: LVM storages don't pass the `storeid:` prefix...
> > +    if ($disk_volid =~ m!^(?:[^:]+:)? (?:\d+/)? $DISK_NAME_REGEX $!xn) {
> > +        my $label = $+{label};
> > +        # Dir storages have a format suffix, zfs/lvm/rbd/... do not.
> > +        if ($label =~ /^disk-(\d+)(?:\..*)?$/) {
> > +            return int($1);
> > +        }
> >      }
> >  
> >      return undef;
> >  };
> >  
> > +# note: disk_list is an array of *volids*
> >  sub get_next_vm_diskname {
> > -    my ($disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix) = @_;
> > +    my ($disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix, $vtype) = @_;
> >  
> >      $fmt //= '';
> > -    my $prefix = ($fmt eq 'subvol') ? 'subvol' : 'vm';
> > +    my $prefix;
> > +    if (defined($vtype) && $vtype eq 'vm-vol') {
> > +        $prefix = 'vol-vm';
> > +    } elsif (defined($vtype) && $vtype eq 'ct-vol') {
> > +        $prefix = ($fmt eq 'subvol') ? 'subvol-ct' : 'vol-ct';
> > +    } else {
> > +        $prefix = ($fmt eq 'subvol') ? 'subvol' : 'vm';
> > +    }
> >      my $suffix = $add_fmt_suffix ? ".$fmt" : '';
> >  
> >      my $disk_ids = {};
> > @@ -954,13 +958,13 @@ sub get_next_vm_diskname {
> >  }
> >  
> >  sub find_free_diskname {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix, $vtype) = @_;
> >  
> > -    my $disks = $class->list_images($storeid, $scfg, $vmid);
> > +    my $disks = $class->list_images($storeid, $scfg, $vmid, undef, undef, $vtype);
> >  
> >      my $disk_list = [map { $_->{volid} } @$disks];
> >  
> > -    return get_next_vm_diskname($disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix);
> > +    return get_next_vm_diskname($disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix, $vtype);
> >  }
> >  
> >  sub clone_image {
> > @@ -1011,14 +1015,23 @@ sub clone_image {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> > -    my $imagedir = $class->get_subdir($scfg, 'images');
> > +    my $imagedir = $class->get_subdir($scfg, $vtype // 'images');
> >      $imagedir .= "/$vmid";
> >  
> >      mkpath $imagedir;
> >  
> > -    $name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 1) if !$name;
> > +    if ($name) {
> > +        if (defined($vtype)) {
> > +            die "illegal name '$name' - should be 'vol-vm-$vmid-*'\n"
> > +                if $vtype eq 'vm-vol' && $name !~ m/^vol-vm-$vmid-/;
> > +            die "illegal name '$name' - should be 'vol-ct-$vmid-*'\n"
> > +                if $vtype eq 'ct-vol' && $name !~ m/^vol-ct-$vmid-/;
> > +        }
> > +    } else {
> > +        $name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 1, $vtype);
> > +    }
> >  
> >      my (undef, $tmpfmt) = parse_name_dir($name);
> >  
> > diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
> > index 2ef1280..839ff73 100644
> > --- a/src/PVE/Storage/RBDPlugin.pm
> > +++ b/src/PVE/Storage/RBDPlugin.pm
> > @@ -620,7 +620,7 @@ sub qemu_blockdev_options {
> >  }
> >  
> >  sub find_free_diskname {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix, $vtype) = @_;
> >  
> >      my $cmd = $rbd_cmd->($scfg, $storeid, 'ls');
> >  
> > @@ -640,7 +640,9 @@ sub find_free_diskname {
> >  
> >      die $err if $err && $err !~ m/doesn't contain rbd images/;
> >  
> > -    return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, undef, $scfg);
> > +    return PVE::Storage::Plugin::get_next_vm_diskname(
> > +        $disk_list, $storeid, $vmid, undef, $scfg, 0, $vtype,
> > +    );
> >  }
> >  
> >  sub create_base {
> > @@ -730,12 +732,23 @@ sub clone_image {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > -
> > -    die "illegal name '$name' - should be 'vm-$vmid-*'\n"
> > -        if $name && $name !~ m/^vm-$vmid-/;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> > +
> > +    if ($name) {
> > +        if (defined($vtype) && $vtype eq 'vm-vol') {
> > +            die "illegal name '$name' - should be 'vol-vm-$vmid-*'\n"
> > +                if $name !~ m/^vol-vm-$vmid-/;
> > +        } elsif (defined($vtype) && $vtype eq 'ct-vol') {
> > +            die "illegal name '$name' - should be 'vol-ct-$vmid-*'\n"
> > +                if $name !~ m/^vol-ct-$vmid-/;
> > +        } else {
> > +            die "illegal name '$name'"
> > +                . " - should be 'vm-$vmid-*', 'vol-vm-$vmid-*' or 'vol-ct-$vmid-*'\n"
> > +                if $name !~ m/^(?:vol-vm|vol-ct|vm)-$vmid-/;
> > +        }
> > +    }
> >  
> > -    $name = $class->find_free_diskname($storeid, $scfg, $vmid) if !$name;
> > +    $name = $class->find_free_diskname($storeid, $scfg, $vmid, undef, 0, $vtype) if !$name;
> >  
> >      my @options = (
> >          '--image-format', 2, '--size', int(($size + 1023) / 1024),
> > diff --git a/src/PVE/Storage/ZFSPlugin.pm b/src/PVE/Storage/ZFSPlugin.pm
> > index a05f521..259e468 100644
> > --- a/src/PVE/Storage/ZFSPlugin.pm
> > +++ b/src/PVE/Storage/ZFSPlugin.pm
> > @@ -355,16 +355,25 @@ sub clone_image {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> >      die "unsupported format '$fmt'" if $fmt ne 'raw';
> >  
> > -    die "illegal name '$name' - should be 'vm-$vmid-*'\n"
> > -        if $name && $name !~ m/^vm-$vmid-/;
> > +    if ($name) {
> > +        if (defined($vtype) && $vtype eq 'vm-vol') {
> > +            die "illegal name '$name' - should be 'vol-vm-$vmid-*'\n"
> > +                if $name !~ m/^vol-vm-$vmid-/;
> > +        } elsif (defined($vtype) && $vtype eq 'ct-vol') {
> > +            die "non-subvolumes not allowed for containers\n";
> > +        } else {
> > +            die "illegal name '$name'" . " - should be 'vm-$vmid-*', 'vol-vm-$vmid-*'\n"
> > +                if $name !~ m/^(?:vol-vm|vm)-$vmid-/;
> > +        }
> > +    }
> >  
> >      my $volname = $name;
> >  
> > -    $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt) if !$volname;
> > +    $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 0, $vtype) if !$volname;
> >  
> >      $class->zfs_create_zvol($scfg, $volname, $size);
> >  
> > diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
> > index d65af69..f972718 100644
> > --- a/src/PVE/Storage/ZFSPoolPlugin.pm
> > +++ b/src/PVE/Storage/ZFSPoolPlugin.pm
> > @@ -271,29 +271,42 @@ sub zfs_wait_for_zvol_link {
> >  }
> >  
> >  sub alloc_image {
> > -    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
> > +    my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size, $vtype) = @_;
> >  
> >      my $volname = $name;
> >  
> >      if ($fmt eq 'raw') {
> > -
> > -        die "illegal name '$volname' - should be 'vm-$vmid-*'\n"
> > -            if $volname && $volname !~ m/^vm-$vmid-/;
> > -        $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
> > +        $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 0, $vtype)
> >              if !$volname;
> >  
> > +        if ($vtype) {
> > +            die "'raw' format not allowed for container volumes\n"
> > +                if $vtype ne 'vm-vol';
> > +
> > +            die "illegal name '$volname' - should be 'vol-vm-$vmid-*'\n"
> > +                if $volname !~ m/^vol-vm-$vmid-/;
> > +        } else {
> > +            die "illegal name '$volname' - should be 'vol-vm-$vmid-*' or 'vm-$vmid-*'\n"
> > +                if $volname !~ m/^(vol-)?vm-$vmid-/;
> > +        }
> > +
> >          $class->zfs_create_zvol($scfg, $volname, $size);
> >          $class->zfs_wait_for_zvol_link($scfg, $volname);
> >  
> >      } elsif ($fmt eq 'subvol') {
> > -
> > -        die "illegal name '$volname' - should be 'subvol-$vmid-*'\n"
> > -            if $volname && $volname !~ m/^subvol-$vmid-/;
> > -        $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
> > +        $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt, 0, $vtype)
> >              if !$volname;
> >  
> > -        die "illegal name '$volname' - should be 'subvol-$vmid-*'\n"
> > -            if $volname !~ m/^subvol-$vmid-/;
> > +        if ($vtype) {
> > +            die "'subvol' format not allowed for VM volumes\n"
> > +                if $vtype ne 'ct-vol';
> > +
> > +            die "illegal name '$volname' - should be 'subvol-ct-$vmid-*'\n"
> > +                if $volname !~ m/^subvol-ct-$vmid-/;
> > +        } else {
> > +            die "illegal name '$volname' - should be 'subvol-ct-$vmid-*' or 'subvol-$vmid-*'\n"
> > +                if $volname !~ m/^subvol(-ct)?-$vmid-/;
> > +        }
> >  
> >          $class->zfs_create_subvol($scfg, $volname, $size);
> >  
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



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


  reply	other threads:[~2025-07-30 14:04 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-29 11:15 [pve-devel] [RFC storage 00/26+10+3] unify vtype and content-type and Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 01/26] btrfs: remove unnecessary mkpath call Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 02/26] parse_volname: remove openvz 'rootdir' case Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 03/26] drop rootdir case in path_to_volume_id Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 04/26] escape dirs in path_to_volume_id regexes Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 05/26] tests: drop rootdir/ tests Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 06/26] common: use v5.36 Wolfgang Bumiller
2025-07-29 13:59   ` Fiona Ebner
2025-07-29 14:42     ` Thomas Lamprecht
2025-07-29 11:15 ` [pve-devel] [PATCH storage 07/26] common: add pve-storage-vtype standard option with new types Wolfgang Bumiller
2025-07-29 13:50   ` Fiona Ebner
2025-07-29 11:15 ` [pve-devel] [PATCH storage 08/26] prepare for vm-vol and ct-vol content and vtypes Wolfgang Bumiller
2025-07-30  8:38   ` Fabian Grünbichler
2025-08-08 12:01     ` Wolfgang Bumiller
2025-07-30  9:14   ` Fabian Grünbichler
2025-08-08 12:05     ` Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 09/26] plugins: add new content types to all plugindata Wolfgang Bumiller
2025-07-30  8:38   ` Fabian Grünbichler
2025-08-08 12:10     ` Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 10/26] plugins: update volname parsing for new naming convention Wolfgang Bumiller
2025-07-30  8:37   ` Fabian Grünbichler
2025-07-30  8:53     ` Wolfgang Bumiller
2025-07-30  8:58       ` Fabian Grünbichler
2025-07-29 11:15 ` [pve-devel] [PATCH storage 11/26] plugin: add vm/ct-vol to 'local' storage default content types Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 12/26] plugin: support new vtypes in activate_storage checks Wolfgang Bumiller
2025-07-30  8:36   ` Fabian Grünbichler
2025-08-08 13:16     ` Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 13/26] plugin, btrfs: update list_images and list_volumes Wolfgang Bumiller
2025-07-30  8:36   ` Fabian Grünbichler
2025-07-30  8:41     ` Fiona Ebner
2025-07-29 11:15 ` [pve-devel] [PATCH storage 14/26] plugins: update image/volume listing to support new types Wolfgang Bumiller
2025-07-30  8:36   ` Fabian Grünbichler
2025-07-29 11:15 ` [pve-devel] [PATCH storage 15/26] common: add is_volume_type and is_type_change_allowed helpers Wolfgang Bumiller
2025-07-30  9:01   ` Fabian Grünbichler
2025-07-29 11:15 ` [pve-devel] [PATCH storage 16/26] common: add volume_type_from_name convenience helper Wolfgang Bumiller
2025-07-30  8:36   ` Fabian Grünbichler
2025-07-30  9:09     ` Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 17/26] plugins: add vtype parameter to alloc_image Wolfgang Bumiller
2025-07-30  9:24   ` Fabian Grünbichler
2025-07-30 14:00   ` Max R. Carrara
2025-07-30 14:05     ` Max R. Carrara [this message]
2025-07-30 14:26       ` Fabian Grünbichler
2025-07-30 14:49         ` Max R. Carrara
2025-07-30 15:01           ` Fabian Grünbichler
2025-07-29 11:15 ` [pve-devel] [PATCH storage 18/26] plugins: update create_base methods Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 19/26] plugins: update clone_image methods Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 20/26] plugins: update rename_volumes methods Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 21/26] plugins: update volume_import methods Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 22/26] zfs: update 'path' method for new naming scheme Wolfgang Bumiller
2025-07-30  9:31   ` Fabian Grünbichler
2025-07-29 11:15 ` [pve-devel] [PATCH storage 23/26] pvesm: add vtype parameter to import command Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 24/26] api: add vtype parameter to create call Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH storage 25/26] update tests Wolfgang Bumiller
2025-07-29 16:33   ` Max R. Carrara
2025-07-29 11:15 ` [pve-devel] [PATCH storage 26/26] update ApiChangeLog Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH container 1/3] add vtype to vdisk_alloc and vdisk_clone calls Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH container 2/3] expect 'vm-vol' vtype in get_replicatable_volumes Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH container 3/3] add vtype to rename_volume call Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 01/10] add vtype to vdisk_alloc and vdisk_clone calls Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 02/10] add vtype parameter to rename_volume call Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 03/10] expect 'vm-vol' vtype in get_replicatable_volumes Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 04/10] expect 'vm-vol' vtype wherever 'images' was expected Wolfgang Bumiller
2025-07-30  8:40   ` Fabian Grünbichler
2025-07-30  9:17   ` Fiona Ebner
2025-07-30  9:33     ` Fiona Ebner
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 05/10] tests: update QmMock to support vtypes Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 06/10] tests: scripted: update tests to new vtypes and paths Wolfgang Bumiller
2025-07-29 14:13   ` Max R. Carrara
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 07/10] make tidy Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 08/10] tests: fixup restore test to use new volume naming scheme Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 09/10] tests: update remaining tests to new snapshot paths Wolfgang Bumiller
2025-07-29 11:15 ` [pve-devel] [PATCH qemu-server 10/10] tests: regenerate cfg2cmd files Wolfgang Bumiller
2025-07-29 14:19   ` Max R. Carrara
2025-07-29 15:34 ` [pve-devel] partially-applied: [RFC storage 00/26+10+3] unify vtype and content-type and Fiona Ebner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DBPG3SQ8DVME.19KHT6M53EKQW@proxmox.com \
    --to=m.carrara@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal