From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id C7FDC1FF13F for ; Thu, 23 Apr 2026 15:58:09 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8E3871A219; Thu, 23 Apr 2026 15:58:09 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 23 Apr 2026 15:57:31 +0200 Message-Id: Subject: Re: [PATCH-SERIES qemu-server v3 00/18] fix #7066: api: allow live snapshot (remove) of qcow2 TPM drive with snapshot-as-volume-chain From: "Lukas Sichert" To: "Fiona Ebner" , References: <20260423093753.43199-1-f.ebner@proxmox.com> In-Reply-To: <20260423093753.43199-1-f.ebner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776952562395 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.788 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [qsd.pm,volumechain.pm,drive.pm,qmpclient.pm,monitor.pm,qemuserver.pm,qemumigrate.pm,blockjob.pm,qemumigratemock.pm,qemu.pm,blockdev.pm] Message-ID-Hash: JBUJX3DYVRB4VVEU5PKAGAFXXAWR5DFZ X-Message-ID-Hash: JBUJX3DYVRB4VVEU5PKAGAFXXAWR5DFZ X-MailFrom: l.sichert@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Everything works as expected. My setup for testing: I configured one PVE machine as an iSCSI target server and exported a 400 GB disk as a writable LUN. On a second PVE machine, in the GUI I added the target LUN as iSCSI storage, created a volume group (VG) on top of it, and then added a LVM storage based on that VG. For testing, I used Windows Server 2022 Evaluation and Windows 11. Before the patch: -taking live snapshots was not possible -removing any snapshot live was not possible After the patch: -taking live snapshots is possible -removing live snapshots is possible, except for the top-most snapshot Unrelated to the patch, works with and without it: -rollbacks power off the VM and are therefore possible -offline snapshots are still possible I will go through the code to give a review soon. Tested-by: Lukas Sichert On 2026-04-23 11:35, Fiona Ebner wrote: > Changes in v3: > * Rebase on latest master (adapt to change from new perltidy version). > > Changes in v2: > * Rebase on latest master (many modified functions moved to > VolumeChain.pm). > * get_node_name_below_throttle(): also die after else branch if no > top node can be found. > > The QMP commands for snapshot operations can just be passed to the > qemu-storage-daemon for the FUSE-exported TPM state and it will handle > it just the same as the main QEMU instance. > > There is a single limitation remaining. That limitation is removing > the top-most snapshot live when the current image is exported via > FUSE, because exporting unshares the 'resize' permission, which would > be required by both 'block-commit' and 'block-stream', for example: > >> QEMU storage daemon 100 qsd command 'block-commit' failed - Permission >> conflict on node '#block017': permissions 'resize' are both required >> by node 'drive-tpmstate0' (uses node '#block017' as 'file' child) and >> unshared by commit job 'commit-drive-tpmstate0' (uses node '#block017' >> as 'main node' child). > > Most of the work is preparing the code interacting with the block > layer to support different QMP peers. > > The first two commits are independent from the goal of the series, but > tangentially related. > > Since this touches quite a bit of code, more testing would be very > appreciated! > > qemu-server: > > Fiona Ebner (18): > block job: fix variable name in documentation > qmp client: add default timeouts for more block commands > drive: introduce drive_uses_qsd_fuse() helper > monitor: add vm_qmp_peer() helper > monitor: add qsd_peer() helper > blockdev: rename variable in get_node_name_below_throttle() for > readability > blockdev: switch get_node_name_below_throttle() to use QMP peer > blockdev: switch detach() to use QMP peer > blockdev: switch blockdev_replace() to use QMP peer > blockdev: switch blockdev_external_snapshot() to use QMP peer > block job: switch qemu_handle_concluded_blockjob() to use QMP peer > block job: switch qemu_blockjobs_cancel() to use QMP peer > block job: switch qemu_drive_mirror_monitor() to use QMP peer > blockdev: switch blockdev_delete() to use QMP peer > blockdev: switch blockdev_stream() to use QMP peer > blockdev: switch blockdev_commit() to use QMP peer > snapshot: support live snapshot (remove) of qcow2 TPM drive on storage > with snapshot-as-volume-chain > fix #7066: api: allow live snapshot (remove) of qcow2 TPM drive with > snapshot-as-volume-chain > > src/PVE/API2/Qemu.pm | 45 ++++++-------- > src/PVE/QMPClient.pm | 7 +++ > src/PVE/QemuMigrate.pm | 6 +- > src/PVE/QemuServer.pm | 65 +++++++++++--------- > src/PVE/QemuServer/BlockJob.pm | 62 +++++++++++-------- > src/PVE/QemuServer/Blockdev.pm | 59 +++++++++--------- > src/PVE/QemuServer/Drive.pm | 20 +++++++ > src/PVE/QemuServer/Monitor.pm | 19 +++++- > src/PVE/QemuServer/QSD.pm | 11 ++-- > src/PVE/QemuServer/VolumeChain.pm | 73 ++++++++++++++--------- > src/PVE/VZDump/QemuServer.pm | 6 +- > src/test/MigrationTest/QemuMigrateMock.pm | 2 +- > 12 files changed, 222 insertions(+), 153 deletions(-) > > > Summary over all repositories: > 12 files changed, 222 insertions(+), 153 deletions(-) > > --=20 > Generated by git-murpp 0.5.0