Re: [pve-devel] [PATCH qemu-server 2/3] migrate: call vm_stop_cleanup after stopping in phase3_cleanup

[Fiona Ebner <f.ebner@proxmox.com>]

Am 20.03.24 um 13:51 schrieb Dominik Csapak:
> @@ -1591,12 +1593,10 @@ sub phase3_cleanup {
>  	$self->{errors} = 1;
>      }
>  
> -    # always deactivate volumes - avoid lvm LVs to be active on several nodes
> -    eval {
> -	PVE::Storage::deactivate_volumes($self->{storecfg}, $sourcevollist);
> -    };
> +    # stop with nocheck does not do a cleanup, so do it here with the original config
> +    eval { PVE::QemuServer::vm_stop_cleanup($self->{storecfg}, $vmid, $oldconf, 0) };

The function has more parameters, so this does not set noerr to 0.

>      if (my $err = $@) {
> -	$self->log('err', $err);
> +	$self->log('err', "cleanup for vm failed - $err");
>  	$self->{errors} = 1;
>      }
>  
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index d53e9693..54f73093 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6160,7 +6160,9 @@ sub cleanup_pci_devices {
>  }
>  
>  sub vm_stop_cleanup {
> -    my ($storecfg, $vmid, $conf, $keepActive, $apply_pending_changes) = @_;
> +    my ($storecfg, $vmid, $conf, $keepActive, $apply_pending_changes, $noerr) = @_;
> +
> +    $noerr //= 1; # defaults to warning

Not too happy about this, because usually passing undef and 0 for a
"boolean" is the same, but not here. And Perl won't help you. There's
not many callers, maybe just adapt them instead? Should be its own patch
then.

>  
>      eval {
>  
> @@ -6186,7 +6188,13 @@ sub vm_stop_cleanup {
>  
>  	vmconfig_apply_pending($vmid, $conf, $storecfg) if $apply_pending_changes;
>      };
> -    warn $@ if $@; # avoid errors - just warn
> +    if (my $err = $@) {
> +	if ($noerr) {
> +	    warn $err;
> +	} else {
> +	    die $err;
> +	}

Style nit: we usually have something like
die $err if !$noerr;
warn $err;
which avoids the line bloat.

> +    }
>  }
>  
>  # call only in locked context