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 6C12F1FF165 for ; Thu, 31 Jul 2025 12:48:50 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 40CB3388AA; Thu, 31 Jul 2025 12:50:14 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Thu, 31 Jul 2025 12:48:45 +0200 Message-ID: <20250731104935.53039-1-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.2 MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1753958967993 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.026 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 Subject: [pve-devel] [PATCH qemu-server 1/2] fix #6580: blockdev: commit: re-open target format node as writable if necessary X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Removing the first snapshot in a snapshot-as-volume-chain is done via block-commit for performance reasons, rather than stream, because the snapshot volume, being the base, is usually larger than the delta since the snapshot. When a drive has the 'ro' flag set in the virtual machine configuration, all three nodes in the throttle->fmt->file chain are opened with the read-only flag set and thus the format node could not serve as the target for the stream operation. Fix this, by temporarily re-opening the format node as writable. Note that from the guest perspective, nothing changes, because the read-only flag for the top throttle node is preserved. Signed-off-by: Fiona Ebner --- src/PVE/QemuServer/Blockdev.pm | 66 +++++++++++++++++++++++----------- 1 file changed, 45 insertions(+), 21 deletions(-) diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm index 08bc48dd..750b5f05 100644 --- a/src/PVE/QemuServer/Blockdev.pm +++ b/src/PVE/QemuServer/Blockdev.pm @@ -991,6 +991,7 @@ sub blockdev_commit { my ($storecfg, $vmid, $machine_version, $deviceid, $drive, $src_snap, $target_snap) = @_; my $volid = $drive->{file}; + my $target_was_read_only; print "block-commit $src_snap to base:$target_snap\n"; @@ -1020,29 +1021,52 @@ sub blockdev_commit { { 'snapshot-name' => $src_snap }, ); - my $job_id = "commit-$deviceid"; - my $jobs = {}; - my $opts = { 'job-id' => $job_id, device => $deviceid }; - - $opts->{'base-node'} = $target_fmt_blockdev->{'node-name'}; - $opts->{'top-node'} = $src_fmt_blockdev->{'node-name'}; - - mon_cmd($vmid, "block-commit", %$opts); - $jobs->{$job_id} = {}; - - # if we commit the current, the blockjob need to be in 'complete' mode - my $complete = $src_snap && $src_snap ne 'current' ? 'auto' : 'complete'; - - eval { - PVE::QemuServer::BlockJob::qemu_drive_mirror_monitor( - $vmid, undef, $jobs, $complete, 0, 'commit', - ); - }; - if ($@) { - die "Failed to complete block commit: $@\n"; + if ($target_was_read_only = $target_fmt_blockdev->{'read-only'}) { + print "reopening internal read-only block node for '$target_snap' as writable\n"; + $target_fmt_blockdev->{'read-only'} = JSON::false; + $target_file_blockdev->{'read-only'} = JSON::false; + mon_cmd($vmid, 'blockdev-reopen', options => [$target_fmt_blockdev]); + # For the guest, the drive is still read-only, because the top throttle node is. } - blockdev_delete($storecfg, $vmid, $drive, $src_file_blockdev, $src_fmt_blockdev, $src_snap); + eval { + my $job_id = "commit-$deviceid"; + my $jobs = {}; + my $opts = { 'job-id' => $job_id, device => $deviceid }; + + $opts->{'base-node'} = $target_fmt_blockdev->{'node-name'}; + $opts->{'top-node'} = $src_fmt_blockdev->{'node-name'}; + + mon_cmd($vmid, "block-commit", %$opts); + $jobs->{$job_id} = {}; + + # if we commit the current, the blockjob need to be in 'complete' mode + my $complete = $src_snap && $src_snap ne 'current' ? 'auto' : 'complete'; + + eval { + PVE::QemuServer::BlockJob::qemu_drive_mirror_monitor( + $vmid, undef, $jobs, $complete, 0, 'commit', + ); + }; + if ($@) { + die "Failed to complete block commit: $@\n"; + } + + blockdev_delete($storecfg, $vmid, $drive, $src_file_blockdev, $src_fmt_blockdev, $src_snap); + }; + my $err = $@; + + if ($target_was_read_only) { + # Even when restoring the read-only flag on the format and file nodes fails, the top + # throttle node still has it, ensuring it is read-only for the guest. + print "re-applying read-only flag for internal block node for '$target_snap'\n"; + $target_fmt_blockdev->{'read-only'} = JSON::true; + $target_file_blockdev->{'read-only'} = JSON::true; + eval { mon_cmd($vmid, 'blockdev-reopen', options => [$target_fmt_blockdev]); }; + print "failed to re-apply read-only flag - $@\n" if $@; + } + + die $err if $err; } sub blockdev_stream { -- 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel