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

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

[Elias Huhsovitz]

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.

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   => '',
+    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;
-- 
2.47.3

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

[Max R. Carrara]

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;