Re: [PATCH storage] fix #7811: storage: lvm: reject allocation on format and volume name mismatch

["Max R. Carrara" <m.carrara@proxmox.com>]

On Thu Jul 16, 2026 at 12:32 PM CEST, Elias Huhsovitz wrote:
> When users allocate a new volume via the API and request a specific
> format (like 'qcow2'), but provide a volume name without the
> corresponding extension (like 'vm-100-disk-0'), the generic API
> validation misses the inconsistency. This occurs because standard LVM
> raw volumes do not use file extensions.
>
> The LVMPlugin ignored this mismatch. It created a raw
> logical volume and logged a misleading "formatting as qcow2" message.
> This resulted in a confusing state where the storage lists the volume
> as 'raw', but the underlying block device contains a 'qcow2' image.
>
> To fix this, add a new file-private subroutine, `verify_volname_format`,
> to the LVM plugin. This subroutine compares the requested format with
> the format implied by the volume name. If they differ, the plugin
> rejects the allocation and provides a hint to correct the
> volume name.

Gave this a spin on my local development VM and can confirm that the
check prevents allocating new qcow2 disks if the filename doesn't end
with .qcow2.

In particular, this fails:

    pvesh create /nodes/localhost/storage/lvm-thick/content \
    	--filename vm-116-disk-1 \
	--size 4G \
	--vmid 116 \
	--format qcow2

But this succeeds:

    pvesh create /nodes/localhost/storage/lvm-thick/content \
    	--filename vm-116-disk-1.qcow2 \
	--size 4G \
	--vmid 116 \
	--format qcow2

Just for completeness' sake, I also tested this with a regular `dir`
storage to see if the behavior is the same now -- and indeed it is!
So, what this this commit implements matches the expected behavior.

The code is also neat; very nice that you're using `my sub` for the
helper you're adding. The only thing that's missing is a `make tidy`
run, but honestly, since that only changes one line (see below), I think
it can just be run when applying this IMO.

Therefore, consider:

Reviewed-by: Max R. Carrara <m.carrara@proxmox.com>
Tested-by: Max R. Carrara <m.carrara@proxmox.com>

>
> Signed-off-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
> ---
>  src/PVE/Storage/LVMPlugin.pm | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
> index a313ecc..3be833e 100644
> --- a/src/PVE/Storage/LVMPlugin.pm
> +++ b/src/PVE/Storage/LVMPlugin.pm
> @@ -738,12 +738,36 @@ my sub alloc_lvm_image {
>
>  }
>
> +my %LVM_FORMAT_EXTENSIONS = (
> +    raw   => '',
              ^ this guy here gets un-indented by `make tidy`.

> +    qcow2 => 'qcow2',
> +);
> +
> +my sub verify_volname_format {
> +    my ($class, $name, $fmt) = @_;
> +
> +    my $expected_ext = $LVM_FORMAT_EXTENSIONS{$fmt};
> +    return if !defined($expected_ext);
> +
> +    my (undef, undef, undef, undef, undef, undef, $parsed_fmt) = $class->parse_volname($name);
> +
> +    return if $fmt eq $parsed_fmt;
> +
> +    my $base_name = $name =~ s/\.[^.]+$//r;
> +    my $suggested_name = $expected_ext ? "$base_name.$expected_ext" : $base_name;
> +
> +    die "volume name '$name' does not match requested format '$fmt' "
> +        . "(did you mean '$suggested_name'?)\n";
> +}
> +
>  sub alloc_image {
>      my ($class, $storeid, $scfg, $vmid, $fmt, $name, $size) = @_;
>
>      $name = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
>          if !$name;
>
> +    verify_volname_format($class, $name, $fmt);
> +
>      alloc_lvm_image($class, $storeid, $scfg, $vmid, $fmt, $name, $size);
>
>      return $name;