Re: [pve-devel] [PATCH qemu-server 18/31] blockdev: add change_medium() helper - DERUMIER, Alexandre via pve-devel

From: "DERUMIER, Alexandre via pve-devel" <pve-devel@lists.proxmox.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Cc: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
Subject: Re: [pve-devel] [PATCH qemu-server 18/31] blockdev: add change_medium() helper
Date: Mon, 30 Jun 2025 14:29:34 +0000	[thread overview]
Message-ID: <mailman.749.1751293783.395.pve-devel@lists.proxmox.com> (raw)
In-Reply-To: <20250627155737.162083-19-f.ebner@proxmox.com>

[-- Attachment #1: Type: message/rfc822, Size: 19995 bytes --]

From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server 18/31] blockdev: add change_medium() helper
Date: Mon, 30 Jun 2025 14:29:34 +0000
Message-ID: <cd933fed020383019705045025d38c509042c267.camel@groupe-cyllene.com>

After a cloudinit regenerate, or if I swap a cdrom image to a new cdrom
image,

the old format && file blockdev are not removed,  and the new blockdevs
have autogenerated nodenames


info blockdev -n

#block143: /var/lib/vz/images/107/vm-107-cloudinit.qcow2 (qcow2)
    Cache mode:       writeback

#block003: /var/lib/vz/images/107/vm-107-cloudinit.qcow2 (file)
    Cache mode:       writeback


fc4da005b8264191a9923ea266be591: /var/lib/vz/images/107/vm-107-
cloudinit.qcow2 (qcow2, read-only)
    Cache mode:       writeback

ec4da005b8264191a9923ea266be591: /var/lib/vz/images/107/vm-107-
cloudinit.qcow2 (file, read-only)
    Cache mode:       writeback


or


Type 'help' for help.
# info block -n
#block594: /var/lib/vz/template/iso/new.iso (raw)
    Cache mode:       writeback

#block439: /var/lib/vz/template/iso/new.iso (file)
    Cache mode:       writeback



fb01e5f0d77f17daef4df2ea5a1d0cd: /var/lib/vz/template/iso/old.iso (raw,
read-only)
    Cache mode:       writeback

eb01e5f0d77f17daef4df2ea5a1d0cd: /var/lib/vz/template/iso/old.iso
(file, read-only)
    Cache mode:       writeback


-------- Message initial --------
De: Fiona Ebner <f.ebner@proxmox.com>
Répondre à: Proxmox VE development discussion <pve-
devel@lists.proxmox.com>
À: pve-devel@lists.proxmox.com
Objet: [pve-devel] [PATCH qemu-server 18/31] blockdev: add
change_medium() helper
Date: 27/06/2025 17:57:14

There is a slight change in behavior for cloud-init disks, when the
file for the new cloud-init disk is 'none'. Previously, the current
drive would not be ejected, now it is. Not sure if that edge case can
even happen in practice and it is more correct, becuase the config was
already updated.

Co-developed-by: Alexandre Derumier <alexandre.derumier@groupe-
cyllene.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm          | 40 ++++++----------------------------
 src/PVE/QemuServer/Blockdev.pm | 18 +++++++++++++++
 2 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 3f135fcb..6e44132e 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5170,30 +5170,15 @@ sub vmconfig_update_disk {
             }
 
         } else { # cdrom
+            eval { PVE::QemuServer::Blockdev::change_medium($storecfg,
$vmid, $opt, $drive); };
+            my $err = $@;
 
-            if ($drive->{file} eq 'none') {
-                mon_cmd($vmid, "eject", force => JSON::true, id =>
"$opt");
-                if (drive_is_cloudinit($old_drive)) {
-                    vmconfig_register_unused_drive($storecfg, $vmid,
$conf, $old_drive);
-                }
-            } else {
-                my ($path, $format) =
-                   
PVE::QemuServer::Drive::get_path_and_format($storecfg, $drive);
-
-                # force eject if locked
-                mon_cmd($vmid, "eject", force => JSON::true, id =>
"$opt");
-
-                if ($path) {
-                    mon_cmd(
-                        $vmid,
-                        "blockdev-change-medium",
-                        id => "$opt",
-                        filename => "$path",
-                        format => "$format",
-                    );
-                }
+            if ($drive->{file} eq 'none' &&
drive_is_cloudinit($old_drive)) {
+                vmconfig_register_unused_drive($storecfg, $vmid,
$conf, $old_drive);
             }
 
+            die $err if $err;
+
             return 1;
         }
     }
@@ -5230,18 +5215,7 @@ sub vmconfig_update_cloudinit_drive {
     my $running = PVE::QemuServer::check_running($vmid);
 
     if ($running) {
-        my ($path, $format) =
-            PVE::QemuServer::Drive::get_path_and_format($storecfg,
$cloudinit_drive);
-        if ($path) {
-            mon_cmd($vmid, "eject", force => JSON::true, id =>
"$cloudinit_ds");
-            mon_cmd(
-                $vmid,
-                "blockdev-change-medium",
-                id => "$cloudinit_ds",
-                filename => "$path",
-                format => "$format",
-            );
-        }
+        PVE::QemuServer::Blockdev::change_medium($storecfg, $vmid,
$cloudinit_ds, $cloudinit_drive);
     }
 }
 
diff --git a/src/PVE/QemuServer/Blockdev.pm
b/src/PVE/QemuServer/Blockdev.pm
index 73cb7ae5..8ef17a3b 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -510,4 +510,22 @@ sub resize {
     );
 }
 
+sub change_medium {
+    my ($storecfg, $vmid, $qdev_id, $drive) = @_;
+
+    # force eject if locked
+    mon_cmd($vmid, "eject", force => JSON::true, id => "$qdev_id");
+
+    my ($path, $format) =
PVE::QemuServer::Drive::get_path_and_format($storecfg, $drive);
+
+    if ($path) { # no path for 'none'
+        mon_cmd(
+            $vmid, "blockdev-change-medium",
+            id => "$qdev_id",
+            filename => "$path",
+            format => "$format",
+        );
+    }
+}
+
 1;

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
