Re: [PATCH qemu-server v3 16/18] blockdev: switch blockdev_commit() to use QMP peer

["Lukas Sichert" <l.sichert@proxmox.com>]

On 2026-04-24 13:56, Fiona Ebner <f.ebner@proxmox.com> wrote:

> Am 24.04.26 um 1:47 PM schrieb Lukas Sichert:
>>> diff --git a/src/PVE/QemuServer/VolumeChain.pm b/src/PVE/QemuServer/VolumeChain.pm
>>> index dad39479..91e78495 100644
>>> --- a/src/PVE/QemuServer/VolumeChain.pm
>>> +++ b/src/PVE/QemuServer/VolumeChain.pm
>>> @@ -192,7 +192,7 @@ sub blockdev_replace {
>>>  }
>>>  
>>>  sub blockdev_commit {
>>> -    my ($storecfg, $vmid, $machine_version, $deviceid, $drive, $src_snap, $target_snap) = @_;
>>> +    my ($storecfg, $qmp_peer, $machine_version, $deviceid, $drive, $src_snap, $target_snap) = @_;
>>>  
>>>      my $volid = $drive->{file};
>>>      my $target_was_read_only;
>>> @@ -229,7 +229,7 @@ sub blockdev_commit {
>>>          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]);
>>> +        qmp_cmd($qmp_peer, 'blockdev-reopen', options => [$target_fmt_blockdev]);
>>>          # For the guest, the drive is still read-only, because the top throttle node is.
>>>      }
>>>  
>>> @@ -241,7 +241,7 @@ sub blockdev_commit {
>>>          $opts->{'base-node'} = $target_fmt_blockdev->{'node-name'};
>>>          $opts->{'top-node'} = $src_fmt_blockdev->{'node-name'};
>>>  
>>> -        mon_cmd($vmid, "block-commit", %$opts);
>>> +        qmp_cmd($qmp_peer, "block-commit", %$opts);
>>>          $jobs->{$job_id} = {};
>>>  
>>>          # If the 'current' state is committed to its backing snapshot, the job will not complete
>>> @@ -255,17 +255,10 @@ sub blockdev_commit {
>>>          # 'block-commit' will complete automatically.
>>>          my $complete = $src_snap && $src_snap ne 'current' ? 'auto' : 'complete';
>>>  
>>> -        PVE::QemuServer::BlockJob::monitor(
>>> -            vm_qmp_peer($vmid), undef, $jobs, $complete, 0, 'commit',
>>> -        );
>>> +        PVE::QemuServer::BlockJob::monitor($qmp_peer, undef, $jobs, $complete, 0, 'commit');
>> Similar to [1], but not as obvious. 'vm_qmp_peer' just got added in this
>> series.
>>>  
>>>          blockdev_delete(
>>> -            $storecfg,
>>> -            vm_qmp_peer($vmid),
>>> -            $drive,
>>> -            $src_file_blockdev,
>>> -            $src_fmt_blockdev,
>>> -            $src_snap,
>>> +            $storecfg, $qmp_peer, $drive, $src_file_blockdev, $src_fmt_blockdev, $src_snap,
>>>          );
>>>      };
>> Same as above.
>
> But patch 09/18 "blockdev: switch blockdev_replace() to use QMP peer"
> already adds an import for vm_qmp_peer and patches 13/18 "block job:
> switch qemu_drive_mirror_monitor() to use QMP peer" and 14/18 "blockdev:
> switch blockdev_delete() to use QMP peer" switched the functions to
> support having a QMP peer already, so what is the issue?
You are right. I had assumed it was possible to switch from '$vmid'
directly to 'qmp_peer', which is not really applicable. Sorry for the
noise.