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.eb...@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 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel