[pve-devel] [PATCH qemu-server 1/2] fix #6580: blockdev: commit: re-open target format node as writable if necessary
Fiona Ebner
f.ebner at proxmox.com
Thu Jul 31 12:48:45 CEST 2025
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 <f.ebner at proxmox.com>
---
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
More information about the pve-devel
mailing list