[PATCH qemu-server] cloud-init: activate volume early enough to fix regression with qcow2 on LVM

[PATCH qemu-server] cloud-init: activate volume early enough to fix regression with qcow2 on LVM

[Fiona Ebner]

Since pve-storage commit 05032c5 ("fix #7811: storage: lvm: reject
allocation on format and volume name mismatch") and its follow-ups,
cloud-init disks are named with a '.qcow2' extension on LVM storages
with snapshot-as-volume-chain enabled.

This causes a regression, because volume_size_info() fails and returns
undef when the qcow2 volume is not active. When the size cannot be
determined, commit_cloudinit_disk() function assumes that the disk
does not yet exist and tries to allocate new disk with the same name,
which fails.

 [I] root@pve9a1 ~# qm start 100
 failed to stat '/dev/lvm/vm-100-cloudinit.qcow2'
   Rounding up size to full physical extent 8.00 MiB
 lvcreate 'lvm/vm-100-cloudinit.qcow2' error:   Logical Volume
  "vm-100-cloudinit.qcow2" already exists in volume group "lvm"

Fix the issue by checking the existence/size with vdisk_list(), which
also works for deactivated volumes.

Activating the volume earlier is an alternative, but would need to be
inside an eval block that silently ignores failure, since the volume
might not exist yet, which should not be logged. And in case where the
qcow2 on LVM volume does exist, but activation fails, there would be
an attempt to allocate a new volume with the same name, which also
seems less than ideal. So that approach is a bit hacky.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/Cloudinit.pm | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer/Cloudinit.pm b/src/PVE/QemuServer/Cloudinit.pm
index c1311da8..5af6b608 100644
--- a/src/PVE/QemuServer/Cloudinit.pm
+++ b/src/PVE/QemuServer/Cloudinit.pm
@@ -39,7 +39,18 @@ sub commit_cloudinit_disk {
     my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
     my $format = checked_volume_format($storecfg, $drive->{file});
 
-    my $size = eval { PVE::Storage::volume_size_info($storecfg, $drive->{file}) };
+    my $volumes = PVE::Storage::vdisk_list($storecfg, $storeid, $vmid, [$drive->{file}], 'images');
+    if (scalar($volumes->{$storeid}->@*) > 1) {
+        # just to be sure, since this is the first caller with $vollist outside of the storage tests
+        print "bug: storage plugin for '$storeid' does not honor \$vollist for list_images()\n";
+    }
+    my $size;
+    for my $volume_info ($volumes->{$storeid}->@*) {
+        next if $volume_info->{volid} ne $drive->{file};
+        $size = $volume_info->{size} // $volume_info->{'approximate-size'};
+        last;
+    }
+
     if (!defined($size) || $size <= 0) {
         $volname =~ m/(vm-$vmid-cloudinit(.\Q$format\E)?)/;
         my $name = $1;
-- 
2.47.3

superseded: [PATCH qemu-server] cloud-init: activate volume early enough to fix regression with qcow2 on LVM

[Fiona Ebner]

superseded-by:
https://lore.proxmox.com/pve-devel/20260812111316.85049-1-f.ebner@proxmox.com/T/

Had forgotten to update the title, since I went for the alternative
approach at first.

Am 12.08.26 um 1:10 PM schrieb Fiona Ebner:
> Since pve-storage commit 05032c5 ("fix #7811: storage: lvm: reject
> allocation on format and volume name mismatch") and its follow-ups,
> cloud-init disks are named with a '.qcow2' extension on LVM storages
> with snapshot-as-volume-chain enabled.
> 
> This causes a regression, because volume_size_info() fails and returns
> undef when the qcow2 volume is not active. When the size cannot be
> determined, commit_cloudinit_disk() function assumes that the disk
> does not yet exist and tries to allocate new disk with the same name,
> which fails.
> 
>  [I] root@pve9a1 ~# qm start 100
>  failed to stat '/dev/lvm/vm-100-cloudinit.qcow2'
>    Rounding up size to full physical extent 8.00 MiB
>  lvcreate 'lvm/vm-100-cloudinit.qcow2' error:   Logical Volume
>   "vm-100-cloudinit.qcow2" already exists in volume group "lvm"
> 
> Fix the issue by checking the existence/size with vdisk_list(), which
> also works for deactivated volumes.
> 
> Activating the volume earlier is an alternative, but would need to be
> inside an eval block that silently ignores failure, since the volume
> might not exist yet, which should not be logged. And in case where the
> qcow2 on LVM volume does exist, but activation fails, there would be
> an attempt to allocate a new volume with the same name, which also
> seems less than ideal. So that approach is a bit hacky.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/QemuServer/Cloudinit.pm | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/QemuServer/Cloudinit.pm b/src/PVE/QemuServer/Cloudinit.pm
> index c1311da8..5af6b608 100644
> --- a/src/PVE/QemuServer/Cloudinit.pm
> +++ b/src/PVE/QemuServer/Cloudinit.pm
> @@ -39,7 +39,18 @@ sub commit_cloudinit_disk {
>      my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>      my $format = checked_volume_format($storecfg, $drive->{file});
>  
> -    my $size = eval { PVE::Storage::volume_size_info($storecfg, $drive->{file}) };
> +    my $volumes = PVE::Storage::vdisk_list($storecfg, $storeid, $vmid, [$drive->{file}], 'images');
> +    if (scalar($volumes->{$storeid}->@*) > 1) {
> +        # just to be sure, since this is the first caller with $vollist outside of the storage tests
> +        print "bug: storage plugin for '$storeid' does not honor \$vollist for list_images()\n";
> +    }
> +    my $size;
> +    for my $volume_info ($volumes->{$storeid}->@*) {
> +        next if $volume_info->{volid} ne $drive->{file};
> +        $size = $volume_info->{size} // $volume_info->{'approximate-size'};
> +        last;
> +    }
> +
>      if (!defined($size) || $size <= 0) {
>          $volname =~ m/(vm-$vmid-cloudinit(.\Q$format\E)?)/;
>          my $name = $1;