On 2/20/25 15:23, Fiona Ebner wrote:
Am 11.02.25 um 17:08 schrieb Daniel Kral:
Asserts whether the target storage supports storing cloudinit images,
i.e. VM images, before creating a cloudinit image on the target storage.
Without the check in place, a cloudinit image can be created on the
storage, which does not support VM images, but won't be able to start
since any attached volume must be stored on a supported storage.
Signed-off-by: Daniel Kral <d.k...@proxmox.com>
---
changes since v1:
- new bug fix! (was indirectly fixed in rfc at commit_cloudinit_image)
PVE/API2/Qemu.pm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 2a2d971e..9fadf3e5 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -142,12 +142,13 @@ my $check_storage_access = sub {
my $volid = $drive->{file};
my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
+ my $is_cloudinit = defined($volname) && $volname eq 'cloudinit';
- if (!$volid || ($volid eq 'none' || (defined($volname) && $volname eq 'cloudinit'))) {
+ if (!$volid || $volid eq 'none') {
# nothing to check
} elsif ($isCDROM && ($volid eq 'cdrom')) {
$rpcenv->check($authuser, "/", ['Sys.Console']);
- } elsif (!$isCDROM && ($volid =~ $PVE::QemuServer::Drive::NEW_DISK_RE))
{
+ } elsif (!$isCDROM && ($volid =~ $PVE::QemuServer::Drive::NEW_DISK_RE
|| $is_cloudinit)) {
A cloudinit drive should be a CD-ROM. Can we even reach here?
Partly, but you're correct that was bad testing on my part, sorry.
As it turns out, it is reachable for some cloudinit images...But it's
not obvious why and I'll fix this and make the why clearer in the v3's
patch message:
`PVE::QemuServer::drive_is_cdrom` checks whether the given drive has the
property key-value pair "media=cdrom". But we neither add that to the
drive string in the pve-manager (which only sends, e.g.
"ide2": "local-lvm:cloudinit,format=qcow2"
for any format) and in the rarer case someone allocates a cloudinit
image via `qm set`, they are likely to not append "media=cdrom"
themselves like:
"qm set -ide2 local-lvm:cloudinit,media=cdrom"
Therefore, the check here doesn't detect a cloudinit image as a cdrom as
long as the "media=cdrom" is not set. But you're correct, with this
patch applied the check would just be skipped like before if someone
explicitly provides this setting.
If it doesn't add too much confusion here and we decide to merge the
cloudinit && new_disk branch here (see my reply for #5), I suggest to
make the `drive_is_cdrom` exclude cloudinit images here in v3 (so that
$isCDROM is only 1 when media=cdrom except for cloudinit drives).
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
