Re: [pve-devel] [PATCH qemu-server] fix #4549: capture and bubble up qemu errors correctly

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

Am 09.05.23 um 10:46 schrieb Christoph Heiss:
> @@ -5922,13 +5916,19 @@ sub vm_start_nolock {
>  		$tpmpid = start_swtpm($storecfg, $vmid, $tpm, $migratedfrom);
>  	    }
>  
> +	    # '\r\n' needs to be used as a line separator here, as that is what run_command() splits
> +	    # lines on. If just a newline is used, all error message lines end up on a single line
> +	    # in the migration tasklog.

So this is essentially a hack to work around the fact that we don't call
print once for each line anymore ;)

> +	    my $err = '';
> +	    $run_params{logfunc} = sub { $err .= "QEMU: $1\r\n"; };

Style nit: please use $run_params->{logfunc}

So you're eating up all the log lines here...

> +
>  	    my $exitcode = run_command($cmd, %run_params);
>  	    if ($exitcode) {
>  		if ($tpmpid) {
>  		    warn "stopping swtpm instance (pid $tpmpid) due to QEMU startup error\n";
>  		    kill 'TERM', $tpmpid;
>  		}
> -		die "QEMU exited with code $exitcode\n";
> +		die "QEMU exited with code $exitcode\r\n$err";

...and only print them in the failure case. This means all non-critical
errors/warnings/info will never see the light of day anymore? To get a
warning for testing you can use e.g.
qm set 100 --args '-no-hpet'
with QEMU >= 8.0.

The current approach also produces a leading space in the task logs for
the follow-up lines:

> TASK ERROR: start failed: QEMU exited with code 1
>  QEMU: kvm: -no-hpet: warning: -no-hpet is deprecated, use '-machine hpet=off' instead
>  QEMU: bridge 'vmbr1' does not exist
>  QEMU: kvm: -netdev type=tap,id=net1,ifname=tap100i1,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on: network script /var/lib/qemu-server/pve-bridge failed with status 512

Instead of getting rid of the "print once per line" handling, can't we
rather adapt the invocation for backup to bubble up the other log lines too?

Always prefixing the messages with "QEMU: " seems fine to me (even
things like the "bridge 'vmbr1' does not exist" message technically come
from the pve-bridge script, it's still part of the QEMU invocation ;)).