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