From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id EFA4CA020E for ; Mon, 12 Jun 2023 13:22:43 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D82782221B for ; Mon, 12 Jun 2023 13:22:43 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 12 Jun 2023 13:22:43 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 18DD343A4B for ; Mon, 12 Jun 2023 13:22:43 +0200 (CEST) Message-ID: Date: Mon, 12 Jun 2023 13:22:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 From: Fiona Ebner To: Proxmox VE development discussion , Christoph Heiss References: <20230509084654.666207-1-c.heiss@proxmox.com> Content-Language: en-US In-Reply-To: <20230509084654.666207-1-c.heiss@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.001 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.093 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH qemu-server] fix #4549: capture and bubble up qemu errors correctly X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Jun 2023 11:22:44 -0000 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 ;)).