all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server v2 3/5] fix #6675: template backup: fix regression with IDE/SATA and blockdev
Date: Tue, 12 Aug 2025 16:37:41 +0200	[thread overview]
Message-ID: <20250812143900.138723-4-f.ebner@proxmox.com> (raw)
In-Reply-To: <20250812143900.138723-1-f.ebner@proxmox.com>

With ide-hd, the inserted block node needs to be marked as writable
too, but -blockdev will complain if it's marked as writable but the
actual backing device is read-only (e.g. read-only base LV).

IDE/SATA do not support being configured as read-only, the most
similar is using ide-cd instead of ide-hd, with most of the code and
configuration shared in QEMU.

Since a template is never actually started, the front-end device is
never accessed. The backup only accesses the inserted block node, so
it does not matter for the backup if the type is 'ide-cd' instead.

The same issue did not manifest for '-drive', because the '-snapshot'
option is used for template backups. The '-snapshot' option does not
affect '-blockdev', from 'man kvm':

> snapshot is incompatible with -blockdev

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm                      | 13 ++++++++++---
 src/test/cfg2cmd/simple1-template.conf.cmd |  4 ++--
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 67db9bc3..d3f0c589 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1272,6 +1272,15 @@ sub print_drivedevice_full {
 
         my $device_type = ($drive->{media} && $drive->{media} eq 'cdrom') ? "cd" : "hd";
 
+        # With ide-hd, the inserted block node needs to be marked as writable too, but -blockdev
+        # will complain if it's marked as writable but the actual backing device is read-only (e.g.
+        # read-only base LV). IDE/SATA do not support being configured as read-only, the most
+        # similar is using ide-cd instead of ide-hd, with most of the code and configuration shared
+        # in QEMU. Since a template is never actually started, the front-end device is never
+        # accessed. The backup only accesses the inserted block node, so it does not matter for the
+        # backup if the type is 'ide-cd' instead.
+        $device_type = 'cd' if $conf->{template};
+
         $device = "ide-$device_type";
         if ($drive->{interface} eq 'ide') {
             $device .= ",bus=ide.$controller,unit=$unit";
@@ -3752,9 +3761,7 @@ sub config_to_command {
 
                     my $extra_blockdev_options = {};
                     $extra_blockdev_options->{'live-restore'} = $live_restore if $live_restore;
-                    # extra protection for templates, but SATA and IDE don't support it..
-                    $extra_blockdev_options->{'read-only'} = 1
-                        if drive_is_read_only($conf, $drive);
+                    $extra_blockdev_options->{'read-only'} = 1 if $is_template;
 
                     my $blockdev = PVE::QemuServer::Blockdev::generate_drive_blockdev(
                         $storecfg, $drive, $machine_version, $extra_blockdev_options,
diff --git a/src/test/cfg2cmd/simple1-template.conf.cmd b/src/test/cfg2cmd/simple1-template.conf.cmd
index df51a4a5..369b4de9 100644
--- a/src/test/cfg2cmd/simple1-template.conf.cmd
+++ b/src/test/cfg2cmd/simple1-template.conf.cmd
@@ -30,7 +30,7 @@
   -blockdev '{"detect-zeroes":"unmap","discard":"unmap","driver":"throttle","file":{"cache":{"direct":true,"no-flush":false},"detect-zeroes":"unmap","discard":"unmap","driver":"qcow2","file":{"aio":"io_uring","cache":{"direct":true,"no-flush":false},"detect-zeroes":"unmap","discard":"unmap","driver":"file","filename":"/var/lib/vz/images/8006/base-8006-disk-1.qcow2","node-name":"e1085774206ae4a6b6bf8426ff08f16","read-only":true},"node-name":"f1085774206ae4a6b6bf8426ff08f16","read-only":true},"node-name":"drive-scsi0","read-only":true,"throttle-group":"throttle-drive-scsi0"}' \
   -device 'scsi-hd,bus=scsihw0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,device_id=drive-scsi0,write-cache=on' \
   -device 'ahci,id=ahci0,multifunction=on,bus=pci.0,addr=0x7' \
-  -blockdev '{"detect-zeroes":"unmap","discard":"unmap","driver":"throttle","file":{"cache":{"direct":true,"no-flush":false},"detect-zeroes":"unmap","discard":"unmap","driver":"qcow2","file":{"aio":"io_uring","cache":{"direct":true,"no-flush":false},"detect-zeroes":"unmap","discard":"unmap","driver":"file","filename":"/var/lib/vz/images/8006/base-8006-disk-0.qcow2","node-name":"eab334c2e07734480f33dd80d89871b","read-only":false},"node-name":"fab334c2e07734480f33dd80d89871b","read-only":false},"node-name":"drive-sata0","read-only":false,"throttle-group":"throttle-drive-sata0"}' \
-  -device 'ide-hd,bus=ahci0.0,drive=drive-sata0,id=sata0,write-cache=on' \
+  -blockdev '{"detect-zeroes":"unmap","discard":"unmap","driver":"throttle","file":{"cache":{"direct":true,"no-flush":false},"detect-zeroes":"unmap","discard":"unmap","driver":"qcow2","file":{"aio":"io_uring","cache":{"direct":true,"no-flush":false},"detect-zeroes":"unmap","discard":"unmap","driver":"file","filename":"/var/lib/vz/images/8006/base-8006-disk-0.qcow2","node-name":"eab334c2e07734480f33dd80d89871b","read-only":true},"node-name":"fab334c2e07734480f33dd80d89871b","read-only":true},"node-name":"drive-sata0","read-only":true,"throttle-group":"throttle-drive-sata0"}' \
+  -device 'ide-cd,bus=ahci0.0,drive=drive-sata0,id=sata0,write-cache=on' \
   -machine 'accel=tcg,smm=off,type=pc+pve0' \
   -snapshot
-- 
2.47.2



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


  parent reply	other threads:[~2025-08-12 14:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-12 14:37 [pve-devel] [PATCH-SERIES qemu-server v2 0/5] " Fiona Ebner
2025-08-12 14:37 ` [pve-devel] [PATCH qemu-server v2 1/5] ovmf: pass along whether the VM is a template Fiona Ebner
2025-08-13  7:37   ` Fabian Grünbichler
2025-08-12 14:37 ` [pve-devel] [PATCH qemu-server v2 2/5] code cleanup: cfg2cmd: check if configuration is for template centrally Fiona Ebner
2025-08-12 14:37 ` Fiona Ebner [this message]
2025-08-12 14:37 ` [pve-devel] [PATCH qemu-server v2 4/5] code cleanup: drive: get rid of outdated drive_is_read_only() helper Fiona Ebner
2025-08-12 14:37 ` [pve-devel] [PATCH qemu-server v2 5/5] cfg2cmd: add reminder comments to remove template handling for -drive Fiona Ebner
2025-08-13  7:34 ` [pve-devel] applied-series: [PATCH-SERIES qemu-server v2 0/5] fix #6675: template backup: fix regression with IDE/SATA and blockdev Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250812143900.138723-4-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal