[pve-devel] [PATCH qemu-server 2/4] hide long commandline on vm_start/migrate failure

[Thomas Lamprecht]

On 12/9/19 4:14 PM, Stefan Reiter wrote:
> By default run_command prints the entire commandline executed when an
> error occurs, but QEMU and our migrate command are not only
> uninteresting to the user[*] but also annoyingly long. Hide them and only
> print the exit code.
> 
> [*] Especially our migrate command, since it can't be manually executed
> anyway. QEMU's commandline *might* contain something interesting, but is
> so long that it's tricky to parse anyway, any a user can always call 'qm
> showcmd --pretty'.
> 

But it can hold interesting info about why the command failed.. And my
"pretty" option is just a basic SED script, so could be also nicely formatted
post-error. I mean, I'm tempted to apply it, just a bit concerns about removing
infos for users which just post the whole error and have not much clue for
debugging, for those helping them the command line /could/ be of help - maybe?

> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>  PVE/QemuMigrate.pm |  6 ++++--
>  PVE/QemuServer.pm  | 10 ++++++++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index f909873..b5ec45c 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -595,7 +595,7 @@ sub phase2 {
>  
>      # Note: We try to keep $spice_ticket secret (do not pass via command line parameter)
>      # instead we pipe it through STDIN
> -    PVE::Tools::run_command($cmd, input => $spice_ticket, outfunc => sub {
> +    my $exitcode = PVE::Tools::run_command($cmd, input => $spice_ticket, outfunc => sub {
>  	my $line = shift;
>  
>  	if ($line =~ m/^migration listens on tcp:(localhost|[\d\.]+|\[[\d\.:a-fA-F]+\]):(\d+)$/) {
> @@ -629,7 +629,9 @@ sub phase2 {
>      }, errfunc => sub {
>  	my $line = shift;
>  	$self->log('info', $line);
> -    });
> +    }, noerr => 1);
> +
> +    die "remote command failed with exit code $exitcode\n" if $exitcode;
>  
>      die "unable to detect remote migration address\n" if !$raddr;
>  
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 13798a1..01b677b 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5396,7 +5396,11 @@ sub vm_start {
>  	                                          : $defaults->{cpuunits};
>  
>  	my $start_timeout = ($conf->{hugepages} || $is_suspended) ? 300 : 30;
> -	my %run_params = (timeout => $statefile ? undef : $start_timeout, umask => 0077);
> +	my %run_params = (
> +	    timeout => $statefile ? undef : $start_timeout,
> +	    umask => 0077,
> +	    noerr => 1,
> +	);
>  
>  	my %properties = (
>  	    Slice => 'qemu.slice',
> @@ -5412,7 +5416,9 @@ sub vm_start {
>  	my $run_qemu = sub {
>  	    PVE::Tools::run_fork sub {
>  		PVE::Systemd::enter_systemd_scope($vmid, "Proxmox VE VM $vmid", %properties);
> -		run_command($cmd, %run_params);
> +
> +		my $exitcode = run_command($cmd, %run_params);
> +		die "QEMU exited with code $exitcode\n" if $exitcode;
>  	    };
>  	};
>  
>