[PATCH-SERIES qemu-server 0/8] block job: monitor: improve completion handling and style

[PATCH-SERIES qemu-server 0/8] block job: monitor: improve completion handling and style

[Fiona Ebner]

Avoid issuing duplicate completes and printing duplicate log messages
when a job does not complete right away. This also avoids an
unnecessary failure that could arise with bad timing.

Avoid sleeping for a full second each time while waiting for
completion to benefit migration. This partially solves #7854.

Further, factor out the status printing from monitor(), clean up its
signature and properly document it.

qemu-server:

Fiona Ebner (8):
  block job: monitor: avoid duplicate completion
  block job: monitor: only print message that jobs are ready once
  partially fix #7854: block job: monitor: handle completion faster to
    benefit migration
  block job: monitor: factor out job status printing
  block job: monitor: make operation argument non-optional
  block job: monitor: rename 'op' argument to 'operation'
  block job: monitor: pass options as a hash
  block job: monitor: properly document function

 src/PVE/QemuMigrate.pm                    |   5 +-
 src/PVE/QemuServer.pm                     |  14 +-
 src/PVE/QemuServer/BlockJob.pm            | 168 +++++++++++++++-------
 src/PVE/QemuServer/VolumeChain.pm         |   4 +-
 src/test/MigrationTest/QemuMigrateMock.pm |   2 +-
 5 files changed, 131 insertions(+), 62 deletions(-)


Summary over all repositories:
  5 files changed, 131 insertions(+), 62 deletions(-)

-- 
Generated by git-murpp 0.5.0

[PATCH qemu-server 1/8] block job: monitor: avoid duplicate completion

[Fiona Ebner]

If the QMP command for completion (i.e. complete or cancel) is
successful, the job transitions to the next state in QEMU. So a
duplicate completion command won't usually do anything. A job might
take some time to actually complete, so at the moment, completion
handling may happen multiple times. This produces confusing duplicate
log lines and can actually lead to unnecessary failure with bad
timing, if the job transitions to state 'concluded' after between
querying the status and issuing the next complete:

> VM 100 qmp command 'job-complete' failed - \
> Job 'mirror-scsi0' in state 'concluded' cannot \
> accept command verb 'complete'

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/BlockJob.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 921f046c..254d7906 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -196,6 +196,8 @@ sub monitor {
                 } else {
 
                     for my $job_id (sort keys %$jobs) {
+                        next if $jobs->{$job_id}->{complete};
+
                         # try to switch the disk if source and destination are on the same guest
                         print "$job_id: Completing block job...\n";
 
-- 
2.47.3

[PATCH qemu-server 2/8] block job: monitor: only print message that jobs are ready once

[Fiona Ebner]

After issuing the QMP command for completion (i.e. complete or
cancel), a job might still take a while to actually complete. Avoid
printing the message that jobs are ready more than once.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/BlockJob.pm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 254d7906..0a51a65b 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -93,6 +93,7 @@ sub monitor {
     $op //= "mirror";
 
     eval {
+        my $ready_message_printed;
         my $err_complete = 0;
 
         my $starttime = time();
@@ -160,7 +161,10 @@ sub monitor {
             last if scalar(keys %$jobs) == 0;
 
             if ($readycounter == scalar(keys %$jobs)) {
-                print "all '$op' jobs are ready\n";
+                if (!$ready_message_printed) {
+                    print "all '$op' jobs are ready\n";
+                    $ready_message_printed = 1;
+                }
 
                 # do the complete later (or has already been done)
                 last if $completion eq 'skip' || $completion eq 'auto';
-- 
2.47.3

[PATCH qemu-server 3/8] partially fix #7854: block job: monitor: handle completion faster to benefit migration

[Fiona Ebner]

Currently, the block job monitor always sleeps a full second between
iterations. This causes a longer guest downtime for migration that
does offline storage migration. Optimize this, by skipping sleep
directly after issuing the completion commands and by sleeping only
for 0.1 seconds when all jobs are ready. Except when a QMP completion
command fails with 'cannot be completed', sleep for a full second
again.

>From some quick testing with a VM with four disks, the time window
between completing state migration and issuing resume on the target
side shrank from ~2.5 to 1.6 seconds.

A further optimization will be to avoid spawning qm via SSH for the
nbdstop command, and instead re-using the tunnel. This requires some
kind of capability check though, to know if the target side already
supports it.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/BlockJob.pm | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 0a51a65b..3c8de3e4 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -5,6 +5,7 @@ use warnings;
 
 use JSON;
 use Storable qw(dclone);
+use Time::HiRes qw(usleep);
 
 use PVE::Format qw(render_duration render_bytes);
 use PVE::RESTEnvironment qw(log_warn);
@@ -110,6 +111,7 @@ sub monitor {
             }
 
             my $readycounter = 0;
+            my $wait_time = 1_000_000; # microseconds
 
             for my $job_id (sort keys %$jobs) {
                 my $job = $running_jobs->{$job_id};
@@ -161,6 +163,8 @@ sub monitor {
             last if scalar(keys %$jobs) == 0;
 
             if ($readycounter == scalar(keys %$jobs)) {
+                $wait_time = 100_000; # everything is ready, query more often
+
                 if (!$ready_message_printed) {
                     print "all '$op' jobs are ready\n";
                     $ready_message_printed = 1;
@@ -202,6 +206,8 @@ sub monitor {
                     for my $job_id (sort keys %$jobs) {
                         next if $jobs->{$job_id}->{complete};
 
+                        my $got_completion_error;
+
                         # try to switch the disk if source and destination are on the same guest
                         print "$job_id: Completing block job...\n";
 
@@ -225,6 +231,9 @@ sub monitor {
                         if ($err && $err =~ m/cannot be completed/) {
                             print "$job_id: block job cannot be completed, trying again.\n";
                             $err_complete++;
+
+                            $wait_time = 1_000_000; # give it some time
+                            $got_completion_error = 1;
                         } elsif ($err) {
                             die "$job_id: block job cannot be completed - $err\n";
                         } else {
@@ -233,11 +242,14 @@ sub monitor {
 
                             print "$job_id: Completed successfully.\n";
                             $jobs->{$job_id}->{complete} = 1;
+
+                            # query right away, except if there was a completion error
+                            $wait_time = 0 if !$got_completion_error;
                         }
                     }
                 }
             }
-            sleep 1;
+            usleep($wait_time) if $wait_time;
         }
     };
     my $err = $@;
-- 
2.47.3

[PATCH qemu-server 4/8] block job: monitor: factor out job status printing

[Fiona Ebner]

The validity/presence of $job->{ready} does not depend on $job->{len}
in any way, so does not need to be part of the if block.

No functional change intended.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/BlockJob.pm | 57 +++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 3c8de3e4..14d814ee 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -79,6 +79,35 @@ sub qemu_blockjobs_cancel {
     }
 }
 
+my sub print_job_status {
+    my ($job_id, $job, $duration) = @_;
+
+    if (my $total = $job->{len}) {
+        my $transferred = $job->{offset} || 0;
+        my $remaining = $total - $transferred;
+        my $percent = sprintf "%.2f", ($transferred * 100 / $total);
+
+        my $total_h = render_bytes($total, 1);
+        my $transferred_h = render_bytes($transferred, 1);
+
+        my $status = sprintf(
+            "transferred $transferred_h of $total_h ($percent%%) in %s",
+            render_duration($duration),
+        );
+
+        if ($job->{ready}) {
+            if ($job->{busy}) {
+                $status .= ", still busy"; # shouldn't even happen? but mirror is weird
+            } else {
+                $status .= ", ready";
+            }
+        }
+        print "$job_id: $status\n";
+    }
+
+    return;
+}
+
 # $completion can be either
 # 'complete': wait until all jobs are ready, job-complete them (default)
 # 'cancel': wait until all jobs are ready, block-job-cancel them
@@ -130,32 +159,10 @@ sub monitor {
                     qemu_handle_concluded_blockjob($qmp_peer, $job_id, $job, $jobs->{$job_id});
                 }
 
-                my $busy = $job->{busy};
-                my $ready = $job->{ready};
-                if (my $total = $job->{len}) {
-                    my $transferred = $job->{offset} || 0;
-                    my $remaining = $total - $transferred;
-                    my $percent = sprintf "%.2f", ($transferred * 100 / $total);
-
-                    my $duration = $ctime - $starttime;
-                    my $total_h = render_bytes($total, 1);
-                    my $transferred_h = render_bytes($transferred, 1);
-
-                    my $status = sprintf(
-                        "transferred $transferred_h of $total_h ($percent%%) in %s",
-                        render_duration($duration),
-                    );
-
-                    if ($ready) {
-                        if ($busy) {
-                            $status .= ", still busy"; # shouldn't even happen? but mirror is weird
-                        } else {
-                            $status .= ", ready";
-                        }
-                    }
-                    print "$job_id: $status\n" if !$jobs->{$job_id}->{ready};
-                    $jobs->{$job_id}->{ready} = $ready;
+                if (!$jobs->{$job_id}->{ready}) {
+                    print_job_status($job_id, $job, $ctime - $starttime);
                 }
+                $jobs->{$job_id}->{ready} = $job->{ready};
 
                 $readycounter++ if $job->{ready};
             }
-- 
2.47.3

[PATCH qemu-server 5/8] block job: monitor: make operation argument non-optional

[Fiona Ebner]

There is nothing special about mirror other than being the first block
job handled by the monitor() function.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuMigrate.pm         | 7 ++++++-
 src/PVE/QemuServer.pm          | 2 +-
 src/PVE/QemuServer/BlockJob.pm | 3 +--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 8da6f15d..604eae82 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -1581,7 +1581,12 @@ sub phase2 {
         # thus, this command changes to it to blockjob complete (see qapi docs)
         eval {
             PVE::QemuServer::BlockJob::monitor(
-                vm_qmp_peer($vmid), undef, $self->{storage_migration_jobs}, 'cancel',
+                vm_qmp_peer($vmid),
+                undef,
+                $self->{storage_migration_jobs},
+                'cancel',
+                undef,
+                'mirror',
             );
         };
         if (my $err = $@) {
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 9aec7f9c..1be89be9 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -8010,7 +8010,7 @@ sub clone_disk {
             # previous drive-mirrors
             if (($completion && $completion eq 'complete') && (scalar(keys %$jobs) > 0)) {
                 PVE::QemuServer::BlockJob::monitor(
-                    vm_qmp_peer($vmid), $newvmid, $jobs, $completion, $qga,
+                    vm_qmp_peer($vmid), $newvmid, $jobs, $completion, $qga, 'mirror',
                 );
             }
             goto no_data_clone;
diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 14d814ee..3b105c37 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -120,7 +120,6 @@ sub monitor {
         if $vmiddst && $qmp_peer->{type} ne 'qmp';
 
     $completion //= 'complete';
-    $op //= "mirror";
 
     eval {
         my $ready_message_printed;
@@ -341,7 +340,7 @@ sub qemu_drive_mirror {
         die "mirroring error: $err\n";
     }
 
-    monitor(vm_qmp_peer($vmid), $vmiddst, $jobs, $completion, $qga);
+    monitor(vm_qmp_peer($vmid), $vmiddst, $jobs, $completion, $qga, 'mirror');
 }
 
 # Callers should version guard this (only available with a binary >= QEMU 8.2)
-- 
2.47.3

[PATCH qemu-server 6/8] block job: monitor: rename 'op' argument to 'operation'

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/BlockJob.pm | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 3b105c37..d4998370 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -114,7 +114,7 @@ my sub print_job_status {
 # 'skip': wait until all jobs are ready, return with block jobs in ready state
 # 'auto': wait until all jobs disappear, only use for jobs which complete automatically
 sub monitor {
-    my ($qmp_peer, $vmiddst, $jobs, $completion, $qga, $op) = @_;
+    my ($qmp_peer, $vmiddst, $jobs, $completion, $qga, $operation) = @_;
 
     die "drive mirror: different destination is only supported when peer is main QEMU instance\n"
         if $vmiddst && $qmp_peer->{type} ne 'qmp';
@@ -127,14 +127,14 @@ sub monitor {
 
         my $starttime = time();
         while (1) {
-            die "block job ('$op') timed out\n" if $err_complete > 300;
+            die "block job ('$operation') timed out\n" if $err_complete > 300;
 
             my $stats = qmp_cmd($qmp_peer, "query-block-jobs");
             my $ctime = time();
 
             my $running_jobs = {};
             for my $stat (@$stats) {
-                next if $stat->{type} ne $op;
+                next if $stat->{type} ne $operation;
                 $running_jobs->{ $stat->{device} } = $stat;
             }
 
@@ -147,12 +147,12 @@ sub monitor {
                 my $vanished = !defined($job);
                 my $complete = defined($jobs->{$job_id}->{complete}) && $vanished;
                 if ($complete || ($vanished && $completion eq 'auto')) {
-                    print "$job_id: $op-job finished\n";
+                    print "${job_id}: ${operation}-job finished\n";
                     delete $jobs->{$job_id};
                     next;
                 }
 
-                die "$job_id: '$op' has been cancelled\n" if !defined($job);
+                die "$job_id: '$operation' has been cancelled\n" if !defined($job);
 
                 if ($job && $job->{status} eq 'concluded') {
                     qemu_handle_concluded_blockjob($qmp_peer, $job_id, $job, $jobs->{$job_id});
@@ -172,7 +172,7 @@ sub monitor {
                 $wait_time = 100_000; # everything is ready, query more often
 
                 if (!$ready_message_printed) {
-                    print "all '$op' jobs are ready\n";
+                    print "all '$operation' jobs are ready\n";
                     $ready_message_printed = 1;
                 }
 
@@ -262,7 +262,7 @@ sub monitor {
 
     if ($err) {
         eval { qemu_blockjobs_cancel($qmp_peer, $jobs) };
-        die "block job ($op) error: $err";
+        die "block job ($operation) error: $err";
     }
 }
 
-- 
2.47.3

[PATCH qemu-server 7/8] block job: monitor: pass options as a hash

[Fiona Ebner]

No functional change intended.

While the value of $completion is optional, all callers pass it as a
parameter and most do pass a value. Don't include it in the options
hash.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuMigrate.pm                    | 10 ++--------
 src/PVE/QemuServer.pm                     | 14 +++++++-------
 src/PVE/QemuServer/BlockJob.pm            | 22 +++++++++++-----------
 src/PVE/QemuServer/VolumeChain.pm         |  4 ++--
 src/test/MigrationTest/QemuMigrateMock.pm |  2 +-
 5 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 604eae82..dcf26a1c 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -1580,14 +1580,8 @@ sub phase2 {
         # to avoid it trying to re-establish it. We are in blockjob ready state,
         # thus, this command changes to it to blockjob complete (see qapi docs)
         eval {
-            PVE::QemuServer::BlockJob::monitor(
-                vm_qmp_peer($vmid),
-                undef,
-                $self->{storage_migration_jobs},
-                'cancel',
-                undef,
-                'mirror',
-            );
+            my $jobs = $self->{storage_migration_jobs};
+            PVE::QemuServer::BlockJob::monitor(vm_qmp_peer($vmid), $jobs, 'cancel', 'mirror');
         };
         if (my $err = $@) {
             die "Failed to complete storage migration: $err\n";
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 1be89be9..aad0e323 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -7367,9 +7367,7 @@ sub pbs_live_restore {
         }
 
         mon_cmd($vmid, 'cont');
-        PVE::QemuServer::BlockJob::monitor(
-            vm_qmp_peer($vmid), undef, $jobs, 'auto', 0, 'stream',
-        );
+        PVE::QemuServer::BlockJob::monitor(vm_qmp_peer($vmid), $jobs, 'auto', 'stream');
 
         print "restore-drive jobs finished successfully, removing all tracking block devices"
             . " to disconnect from Proxmox Backup Server\n";
@@ -7489,9 +7487,7 @@ sub live_import_from_files {
         }
 
         mon_cmd($vmid, 'cont');
-        PVE::QemuServer::BlockJob::monitor(
-            vm_qmp_peer($vmid), undef, $jobs, 'auto', 0, 'stream',
-        );
+        PVE::QemuServer::BlockJob::monitor(vm_qmp_peer($vmid), $jobs, 'auto', 'stream');
 
         print "restore-drive jobs finished successfully, removing all tracking block devices\n";
 
@@ -8010,7 +8006,11 @@ sub clone_disk {
             # previous drive-mirrors
             if (($completion && $completion eq 'complete') && (scalar(keys %$jobs) > 0)) {
                 PVE::QemuServer::BlockJob::monitor(
-                    vm_qmp_peer($vmid), $newvmid, $jobs, $completion, $qga, 'mirror',
+                    vm_qmp_peer($vmid),
+                    $jobs,
+                    $completion,
+                    'mirror',
+                    { 'dest-vmid' => $newvmid, qga => $qga },
                 );
             }
             goto no_data_clone;
diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index d4998370..5fbc311e 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -114,7 +114,9 @@ my sub print_job_status {
 # 'skip': wait until all jobs are ready, return with block jobs in ready state
 # 'auto': wait until all jobs disappear, only use for jobs which complete automatically
 sub monitor {
-    my ($qmp_peer, $vmiddst, $jobs, $completion, $qga, $operation) = @_;
+    my ($qmp_peer, $jobs, $completion, $operation, $options) = @_;
+
+    my $vmiddst = $options->{'dest-vmid'};
 
     die "drive mirror: different destination is only supported when peer is main QEMU instance\n"
         if $vmiddst && $qmp_peer->{type} ne 'qmp';
@@ -182,7 +184,7 @@ sub monitor {
                 if ($qmp_peer->{type} eq 'qmp' && $vmiddst && $vmiddst != $qmp_peer->{id}) {
                     my $vmid = $qmp_peer->{id};
                     my $should_fsfreeze =
-                        PVE::QemuServer::Agent::guest_fs_freeze_applicable($qga, $vmid);
+                        PVE::QemuServer::Agent::guest_fs_freeze_applicable($options->{qga}, $vmid);
                     if ($should_fsfreeze) {
                         print "issuing guest agent 'guest-fsfreeze-freeze' command\n";
                         eval { PVE::QemuServer::Agent::guest_fs_freeze($vmid); };
@@ -340,7 +342,8 @@ sub qemu_drive_mirror {
         die "mirroring error: $err\n";
     }
 
-    monitor(vm_qmp_peer($vmid), $vmiddst, $jobs, $completion, $qga, 'mirror');
+    my $monitor_options = { 'dest-vmid' => $vmiddst, qga => $qga };
+    monitor(vm_qmp_peer($vmid), $jobs, $completion, 'mirror', $monitor_options);
 }
 
 # Callers should version guard this (only available with a binary >= QEMU 8.2)
@@ -537,14 +540,11 @@ sub blockdev_mirror {
         log_warn("unable to delete blockdev '$target_node_name' - $@");
         die "error starting blockdev mirrror - $err";
     }
-    monitor(
-        vm_qmp_peer($vmid),
-        $dest->{vmid},
-        $jobs,
-        $completion,
-        $options->{'guest-agent'},
-        'mirror',
-    );
+    my $monitor_options = {
+        'dest-vmid' => $dest->{vmid},
+        qga => $options->{'guest-agent'},
+    };
+    monitor(vm_qmp_peer($vmid), $jobs, $completion, 'mirror', $monitor_options);
 }
 
 sub mirror {
diff --git a/src/PVE/QemuServer/VolumeChain.pm b/src/PVE/QemuServer/VolumeChain.pm
index 1572450b..a67ec247 100644
--- a/src/PVE/QemuServer/VolumeChain.pm
+++ b/src/PVE/QemuServer/VolumeChain.pm
@@ -255,7 +255,7 @@ sub blockdev_commit {
         # 'block-commit' will complete automatically.
         my $complete = $src_snap && $src_snap ne 'current' ? 'auto' : 'complete';
 
-        PVE::QemuServer::BlockJob::monitor($qmp_peer, undef, $jobs, $complete, 0, 'commit');
+        PVE::QemuServer::BlockJob::monitor($qmp_peer, $jobs, $complete, 'commit');
 
         blockdev_delete(
             $storecfg, $qmp_peer, $drive, $src_file_blockdev, $src_fmt_blockdev, $src_snap,
@@ -337,7 +337,7 @@ sub blockdev_stream {
     qmp_cmd($qmp_peer, 'block-stream', %$options);
     $jobs->{$job_id} = {};
 
-    PVE::QemuServer::BlockJob::monitor($qmp_peer, undef, $jobs, 'auto', 0, 'stream');
+    PVE::QemuServer::BlockJob::monitor($qmp_peer, $jobs, 'auto', 'stream');
 
     blockdev_delete(
         $storecfg, $qmp_peer, $drive, $snap_file_blockdev, $snap_fmt_blockdev, $snap,
diff --git a/src/test/MigrationTest/QemuMigrateMock.pm b/src/test/MigrationTest/QemuMigrateMock.pm
index 3a483817..079d1a0f 100644
--- a/src/test/MigrationTest/QemuMigrateMock.pm
+++ b/src/test/MigrationTest/QemuMigrateMock.pm
@@ -173,7 +173,7 @@ $qemu_server_blockjob_module->mock(
         common_mirror_mock($source->{vmid}, $drive_id);
     },
     monitor => sub {
-        my ($qmp_peer, $vmiddst, $jobs, $completion, $qga, $op) = @_;
+        my ($qmp_peer, $jobs, $completion, $operation, $options) = @_;
 
         if (
             $fail_config->{block_job_monitor}
-- 
2.47.3

[PATCH qemu-server 8/8] block job: monitor: properly document function

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/BlockJob.pm | 56 +++++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 5fbc311e..2f4bf3cc 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -108,11 +108,57 @@ my sub print_job_status {
     return;
 }
 
-# $completion can be either
-# 'complete': wait until all jobs are ready, job-complete them (default)
-# 'cancel': wait until all jobs are ready, block-job-cancel them
-# 'skip': wait until all jobs are ready, return with block jobs in ready state
-# 'auto': wait until all jobs disappear, only use for jobs which complete automatically
+=head3 monitor
+
+Monitor the given jobs C<$jobs> and handle them according to the mode C<$completion>.
+
+=over
+
+=item C<$qmp_peer>: The QMP peer which is executing the jobs.
+
+=item C<$jobs>: Hash reference with job IDs that should be handled. With optional, additional
+information:
+
+=over
+
+=item C<< $jobs->{$job_id}->{'source-node-name'} >>: (optional, used by mirror) Name of the source
+block node. Will be detached upon completion.
+
+=item C<< $jobs->{$job_id}->{'target-node-name'} >>: (optional, used by mirror) Name of the target
+block node. Will be detached upon cancel.
+
+=back
+
+=item C<$completion>: How to handle the jobs:
+
+=over
+
+=item C<'complete'>: wait until all jobs are ready, job-complete them (default)
+
+=item C<'cancel'>: wait until all jobs are ready, block-job-cancel them
+
+=item C<'skip'>: wait until all jobs are ready, return with block jobs in ready state
+
+=item C<'auto'>: wait until all jobs disappear, only use for jobs which complete automatically
+
+=back
+
+=item C<$operation>: Name of the block job operation.
+
+=item C<$options>: Additional options:
+
+=over
+
+=item C<< $options->{qga} >>: Guest agent string to be considered for guest filesystem freezing.
+
+=item C<< $options->{'dest-vmid'} >>: VM ID of the target of the job.
+
+=back
+
+=back
+
+=cut
+
 sub monitor {
     my ($qmp_peer, $jobs, $completion, $operation, $options) = @_;
 
-- 
2.47.3