Re: [pbs-devel] [pve-devel] [PATCH qemu-server 06/11] make qemu_drive_mirror_monitor more generic

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Mon, Jan 11, 2021 at 12:14:04PM +0100, Stefan Reiter wrote:
> ...so it works with other block jobs as well. Intended use case is
> block-stream, which also requires a new "auto" (wait only) completion
> mode, since it finishes automatically anyway.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  PVE/QemuServer.pm | 41 +++++++++++++++++++++++------------------
>  1 file changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index bca5669..d517dae 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6777,15 +6777,16 @@ sub qemu_drive_mirror {
>  	die "mirroring error: $err\n";
>      }
>  
> -    qemu_drive_mirror_monitor ($vmid, $vmiddst, $jobs, $completion, $qga);
> +    qemu_drive_job_monitor("mirror", $vmid, $vmiddst, $jobs, $completion, $qga);
>  }
>  
>  # $completion can be either
>  # 'complete': wait until all jobs are ready, block-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
> -sub qemu_drive_mirror_monitor {
> -    my ($vmid, $vmiddst, $jobs, $completion, $qga) = @_;
> +# 'auto': wait until all jobs disappear, only use for jobs which complete automatically
> +sub qemu_drive_job_monitor {
> +    my ($op, $vmid, $vmiddst, $jobs, $completion, $qga) = @_;

I'd feel *much* safer if $op was added to the end and initialized with

    $op //= 'mirror';

>  
>      $completion //= 'complete';
>  
> @@ -6793,46 +6794,50 @@ sub qemu_drive_mirror_monitor {
>  	my $err_complete = 0;
>  
>  	while (1) {
> -	    die "storage migration timed out\n" if $err_complete > 300;
> +	    die "block job ('$op') timed out\n" if $err_complete > 300;
>  
>  	    my $stats = mon_cmd($vmid, "query-block-jobs");
>  
> -	    my $running_mirror_jobs = {};
> +	    my $running_jobs = {};
>  	    foreach my $stat (@$stats) {
> -		next if $stat->{type} ne 'mirror';
> -		$running_mirror_jobs->{$stat->{device}} = $stat;
> +		next if $stat->{type} ne $op;
> +		$running_jobs->{$stat->{device}} = $stat;
>  	    }
>  
>  	    my $readycounter = 0;
>  
>  	    foreach my $job (keys %$jobs) {
>  
> -	        if(defined($jobs->{$job}->{complete}) && !defined($running_mirror_jobs->{$job})) {
> -		    print "$job : finished\n";
> +		my $vanished = !defined($running_jobs->{$job});
> +		my $complete = defined($jobs->{$job}->{complete}) && $vanished;
> +	        if($complete || ($vanished && $completion eq 'auto')) {
> +		    print "$job: finished\n";
>  		    delete $jobs->{$job};
>  		    next;
>  		}
>  
> -		die "$job: mirroring has been cancelled\n" if !defined($running_mirror_jobs->{$job});
> +		die "$job: '$op' has been cancelled\n" if !defined($running_jobs->{$job});
>  
> -		my $busy = $running_mirror_jobs->{$job}->{busy};
> -		my $ready = $running_mirror_jobs->{$job}->{ready};
> -		if (my $total = $running_mirror_jobs->{$job}->{len}) {
> -		    my $transferred = $running_mirror_jobs->{$job}->{offset} || 0;
> +		my $busy = $running_jobs->{$job}->{busy};
> +		my $ready = $running_jobs->{$job}->{ready};
> +		if (my $total = $running_jobs->{$job}->{len}) {
> +		    my $transferred = $running_jobs->{$job}->{offset} || 0;
>  		    my $remaining = $total - $transferred;
>  		    my $percent = sprintf "%.2f", ($transferred * 100 / $total);
>  
>  		    print "$job: transferred: $transferred bytes remaining: $remaining bytes total: $total bytes progression: $percent % busy: $busy ready: $ready \n";
>  		}
>  
> -		$readycounter++ if $running_mirror_jobs->{$job}->{ready};
> +		$readycounter++ if $running_jobs->{$job}->{ready};
>  	    }
>  
>  	    last if scalar(keys %$jobs) == 0;
>  
>  	    if ($readycounter == scalar(keys %$jobs)) {
> -		print "all mirroring jobs are ready \n";
> -		last if $completion eq 'skip'; #do the complete later
> +		print "all '$op' jobs are ready\n";
> +
> +		# do the complete later (or has already been done)
> +		last if $completion eq 'skip' || $completion eq 'auto';
>  
>  		if ($vmiddst && $vmiddst != $vmid) {
>  		    my $agent_running = $qga && qga_check_running($vmid);
> @@ -6888,7 +6893,7 @@ sub qemu_drive_mirror_monitor {
>  
>      if ($err) {
>  	eval { PVE::QemuServer::qemu_blockjobs_cancel($vmid, $jobs) };
> -	die "mirroring error: $err";
> +	die "block job ('$op') error: $err";
>      }
>  
>  }
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>