[PATCH v2 qemu-server] cloud-init: query volume size via vdisk_list() to fix regression with qcow2 on LVM

[PATCH v2 qemu-server] cloud-init: query volume size via vdisk_list() 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.

 # 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>
---

Changes in v2:
* don't include full shell prompt in commit message
* fix commit title

 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

Re: [PATCH v2 qemu-server] cloud-init: query volume size via vdisk_list() to fix regression with qcow2 on LVM

[Dominik Csapak]

generally works for the issue that is described, but see my comment
inline

On 8/12/26 1:13 PM, Fiona Ebner wrote:
> 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.
> 
>   # 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>
> ---
> 
> Changes in v2:
> * don't include full shell prompt in commit message
> * fix commit title
> 
>   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');

for lvm this looks alright, but for other storages this is now a
relatively large performance impact?

vdisk_list calls list_images of the plugin, and for e.g. nfs/dir/etc.
this calls 'file_size_info' for *all* found volumes (since that
does not filter early for vollist)

on nfs this could maybe be problematic with many qcow2 files?

also, this is not eval'd, so any plugin that dies during list_images
(e.g. because of permissions, or some other error condition)

blocks the vm start now..

IMHO this should be in an eval and the storage plugins
should filter early on vollist to avoid unnecessary work.

alternatively we could drop the vollist and just give the $vmid
which filters early in most (?) storages.

a third thing i noticed is that vdisk_list actives the storage, which
volume_size_info does not (AFAICS). This is probably correct
anyway but worth noting in the commit message IMO

> +    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;

Re: [PATCH v2 qemu-server] cloud-init: query volume size via vdisk_list() to fix regression with qcow2 on LVM

[Fiona Ebner]

Am 17.08.26 um 8:59 AM schrieb Dominik Csapak:
> generally works for the issue that is described, but see my comment
> inline
> 
> On 8/12/26 1:13 PM, Fiona Ebner wrote:
>> 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.
>>
>>   # 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>
>> ---
>>
>> Changes in v2:
>> * don't include full shell prompt in commit message
>> * fix commit title
>>
>>   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');
> 
> for lvm this looks alright, but for other storages this is now a
> relatively large performance impact?
> 
> vdisk_list calls list_images of the plugin, and for e.g. nfs/dir/etc.
> this calls 'file_size_info' for *all* found volumes (since that
> does not filter early for vollist)

Good catch! This is a bit unfortunate. The reason is that the volid
looks different if it's a linked clone.

> 
> on nfs this could maybe be problematic with many qcow2 files?
> 
> also, this is not eval'd, so any plugin that dies during list_images
> (e.g. because of permissions, or some other error condition)
> 
> blocks the vm start now..
> 
> IMHO this should be in an eval and the storage plugins
> should filter early on vollist to avoid unnecessary work.
> 

I'm not fully convinced it should be eval'd, because if listing the
volume you are interested in fails, that's a good reason to fail. But
since some plugins don't properly just query the actual volume yet, I'll
just add it to be sure.

> alternatively we could drop the vollist and just give the $vmid
> which filters early in most (?) storages.

I do already specify the $vmid as well ;) But why drop vollist? If the
plugin respects it, it's better to have it. If the plugin doesn't, it
doesn't hurt.

> 
> a third thing i noticed is that vdisk_list actives the storage, which
> volume_size_info does not (AFAICS). This is probably correct
> anyway but worth noting in the commit message IMO

I can mention it.

> 
>> +    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;
> 

Re: [PATCH v2 qemu-server] cloud-init: query volume size via vdisk_list() to fix regression with qcow2 on LVM

[Fiona Ebner]

Am 17.08.26 um 10:11 AM schrieb Fiona Ebner:
> Am 17.08.26 um 8:59 AM schrieb Dominik Csapak:
>>
>> a third thing i noticed is that vdisk_list actives the storage, which
>> volume_size_info does not (AFAICS). This is probably correct
>> anyway but worth noting in the commit message IMO
> 
> I can mention it.

And it actually fixes another bug, because the storage is not actually
activated anywhere before (for a VM with just the cloudinit disk on the
NFS storage):
[I] root@pve9a1 ~# qm stop 100; umount /mnt/pve/nfs; and qm start 100
failed to stat '/mnt/pve/nfs/images/100/vm-100-cloudinit.raw'
disk image '/mnt/pve/nfs/images/100/vm-100-cloudinit.raw' already exists

Re: [PATCH v2 qemu-server] cloud-init: query volume size via vdisk_list() to fix regression with qcow2 on LVM

[Dominik Csapak]

On 8/17/26 10:11 AM, Fiona Ebner wrote:
> Am 17.08.26 um 8:59 AM schrieb Dominik Csapak:
>> generally works for the issue that is described, but see my comment
>> inline
>>
>> On 8/12/26 1:13 PM, Fiona Ebner wrote:
>>> 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.
>>>
>>>    # 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>
>>> ---
>>>
>>> Changes in v2:
>>> * don't include full shell prompt in commit message
>>> * fix commit title
>>>
>>>    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');
>>
>> for lvm this looks alright, but for other storages this is now a
>> relatively large performance impact?
>>
>> vdisk_list calls list_images of the plugin, and for e.g. nfs/dir/etc.
>> this calls 'file_size_info' for *all* found volumes (since that
>> does not filter early for vollist)
> 
> Good catch! This is a bit unfortunate. The reason is that the volid
> looks different if it's a linked clone.
> 
>>
>> on nfs this could maybe be problematic with many qcow2 files?
>>
>> also, this is not eval'd, so any plugin that dies during list_images
>> (e.g. because of permissions, or some other error condition)
>>
>> blocks the vm start now..
>>
>> IMHO this should be in an eval and the storage plugins
>> should filter early on vollist to avoid unnecessary work.
>>
> 
> I'm not fully convinced it should be eval'd, because if listing the
> volume you are interested in fails, that's a good reason to fail. But
> since some plugins don't properly just query the actual volume yet, I'll
> just add it to be sure.
> 
>> alternatively we could drop the vollist and just give the $vmid
>> which filters early in most (?) storages.
> 
> I do already specify the $vmid as well ;) But why drop vollist? If the
> plugin respects it, it's better to have it. If the plugin doesn't, it
> doesn't hurt.

yes but e.g. the dir storage explicitly does not use vmid for skipping
early if vollist is given:

```
next if !$vollist && defined($vmid) && ($owner ne $vmid);
```

> 
>>
>> a third thing i noticed is that vdisk_list actives the storage, which
>> volume_size_info does not (AFAICS). This is probably correct
>> anyway but worth noting in the commit message IMO
> 
> I can mention it.
> 
>>
>>> +    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;
>>
> 

Re: [PATCH v2 qemu-server] cloud-init: query volume size via vdisk_list() to fix regression with qcow2 on LVM

[Fiona Ebner]

Am 17.08.26 um 10:27 AM schrieb Dominik Csapak:
> On 8/17/26 10:11 AM, Fiona Ebner wrote:
>> Am 17.08.26 um 8:59 AM schrieb Dominik Csapak:
>>> generally works for the issue that is described, but see my comment
>>> inline
>>>
>>> On 8/12/26 1:13 PM, Fiona Ebner wrote:
>>>> 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.
>>>>
>>>>    # 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>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> * don't include full shell prompt in commit message
>>>> * fix commit title
>>>>
>>>>    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');
>>>
>>> for lvm this looks alright, but for other storages this is now a
>>> relatively large performance impact?
>>>
>>> vdisk_list calls list_images of the plugin, and for e.g. nfs/dir/etc.
>>> this calls 'file_size_info' for *all* found volumes (since that
>>> does not filter early for vollist)
>>
>> Good catch! This is a bit unfortunate. The reason is that the volid
>> looks different if it's a linked clone.
>>
>>>
>>> on nfs this could maybe be problematic with many qcow2 files?
>>>
>>> also, this is not eval'd, so any plugin that dies during list_images
>>> (e.g. because of permissions, or some other error condition)
>>>
>>> blocks the vm start now..
>>>
>>> IMHO this should be in an eval and the storage plugins
>>> should filter early on vollist to avoid unnecessary work.
>>>
>>
>> I'm not fully convinced it should be eval'd, because if listing the
>> volume you are interested in fails, that's a good reason to fail. But
>> since some plugins don't properly just query the actual volume yet, I'll
>> just add it to be sure.
>>
>>> alternatively we could drop the vollist and just give the $vmid
>>> which filters early in most (?) storages.
>>
>> I do already specify the $vmid as well ;) But why drop vollist? If the
>> plugin respects it, it's better to have it. If the plugin doesn't, it
>> doesn't hurt.
> 
> yes but e.g. the dir storage explicitly does not use vmid for skipping
> early if vollist is given:
> 
> ```
> next if !$vollist && defined($vmid) && ($owner ne $vmid);
> ```
> 

Right. The semantics in our implementations are that if $vollist is
specified, then $vmid is completely ignored.

I feel like we should have a more tailored volume_exists() or
volume_info() at some point, which avoids the complicated semantics and
overhead of vdisk_list(). We already had a need in the past: there is
rbd_volume_exists(), but unfortunately, no plugin method.

superseded: [PATCH v2 qemu-server] cloud-init: query volume size via vdisk_list() to fix regression with qcow2 on LVM

[Fiona Ebner]

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