Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Zhangjin Wu <falcon@tinylab.org>
To:     w@1wt.eu
Cc:     falcon@tinylab.org, arnd@arndb.de, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, thomas@t-8ch.de
Subject: [PATCH v3 08/12] selftests/nolibc: allow quit qemu-system when poweroff fails
Date:   Sat, 29 Jul 2023 03:40:50 +0800
Message-Id: <20230728194050.151614-1-falcon@tinylab.org>
In-Reply-To: <4b4d792299ca5356f8f5af5fc9a27c687b0e4e38.1690489039.git.falcon@tinylab.org>
References: <4b4d792299ca5356f8f5af5fc9a27c687b0e4e38.1690489039.git.falcon@tinylab.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Feedback-ID: bizesmtp:tinylab.org:qybglogicsvrgz:qybglogicsvrgz5a-1
Precedence: bulk

Hi, Willy

two trivial updates required in this patch.

[...]
> 
> To tell users the test running progress in time, some critical running
> status are also printed and detected.
> 
[...]
> @@ -229,16 +232,39 @@ kernel: $(KERNEL_CONFIG)
>  # common macros for qemu run/rerun targets
>  QEMU_SYSTEM_RUN = qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(KERNEL_IMAGE)" -serial stdio $(QEMU_ARGS)
>  
> +TIMEOUT_CMD = t=$(QEMU_TIMEOUT); past=0; \
> +	bios_timeout=$$(expr $$t - 7); kernel_timeout=$$(expr $$t - 5); init_timeout=$$(expr $$t - 3); test_timeout=$$(expr $$t - 1);              \
> +	err=""; bios=0; kernel=0; init=0; test=0; poweredoff=0; panic=0;                                                                           \

This 'panic=0;' variable init should be removed, it is not required in
the latest version:

	err=""; bios=0; kernel=0; init=0; test=0; poweredoff=0;                                                                                    \

> +	echo "Running $(KERNEL_IMAGE) on qemu-system-$(QEMU_ARCH)";                                                                                \
> +	while [ $$t -gt 0 ]; do                                                                                                                    \
> +	    sleep 2; t=$$(expr $$t - 2); past=$$(expr $$past + 2);                                                                                 \
> +	    if [ $$bios -eq 0 ] && grep -E "Linux version|Kernel command line|printk: console" "$(RUN_OUT)"; then bios=1; fi;                      \
> +	    if [ $$bios -eq 1 -a $$kernel -eq 0 ] && grep -E "Run .* as init process" "$(RUN_OUT)"; then kernel=1; fi;                             \
> +	    if [ $$kernel -eq 1 -a $$init -eq 0 ] && grep -E "Running test" "$(RUN_OUT)"; then init=1; fi;                                         \
> +	    if [ $$init -eq 1 -a $$test -eq 0 ] && grep -E "Leaving init with final status|Exiting with status" "$(RUN_OUT)"; then test=1; fi;     \

It is better to get the line of 'Total number of errors' instead of
'Exiting with status', the later never trigger in qemu-system run.

	    if [ $$init -eq 1 -a $$test -eq 0 ] && grep -E "Leaving init with final status|Total number of errors" "$(RUN_OUT)"; then test=1; fi;  \

> +	    if [ $$init -eq 1 ] && grep -E "Kernel panic - not syncing: Attempted to kill init" "$(RUN_OUT)"; then err="test"; sleep 1; break; fi; \
> +	    if [ $$test -eq 1 ] && grep -E "reboot: System halted|reboot: Power down" "$(RUN_OUT)"; then poweredoff=1; sleep 1; break; fi;         \
> +	    if [ $$past -gt $$bios_timeout -a $$bios -eq 0 ]; then err="bios"; break; fi;                                                          \
> +	    if [ $$past -gt $$kernel_timeout -a $$kernel -eq 0 ]; then err="kernel"; break; fi;                                                    \
> +	    if [ $$past -gt $$init_timeout -a $$init -eq 0 ]; then err="init"; break; fi;                                                          \
> +	    if [ $$past -gt $$test_timeout -a $$test -eq 0 ]; then err="test"; break; fi;                                                          \
> +	done;                                                                                                                                      \
> +	if [ -z "$$err" -a $$poweredoff -eq 0 -a $$panic -eq 0 ]; then err="qemu-system-$(QEMU_ARCH)"; fi;                                         \

And here, we should remove the panic check here too, it is replaced with
'err="test"':

	if [ -z "$$err" -a $$poweredoff -eq 0 ]; then err="qemu-system-$(QEMU_ARCH)"; fi;                                                          \


Thanks,
Zhangjin

> +	if [ -n "$$err" ]; then echo "$$err may timeout, test failed"; tail -10 $(RUN_OUT); else echo "powered off, test finish"; fi;              \
> +	pkill -15 qemu-system-$(QEMU_ARCH) || true
> +
> +TIMEOUT_QEMU_RUN = ($(QEMU_SYSTEM_RUN) > "$(RUN_OUT)" &); $(TIMEOUT_CMD)
> +
>  # run the tests after building the kernel
>  PHONY += $(KERNEL_IMAGE)
>  $(KERNEL_IMAGE): kernel
>  run: $(KERNEL_IMAGE)
> -	$(Q)$(QEMU_SYSTEM_RUN) > "$(RUN_OUT)"
> +	$(Q)$(TIMEOUT_QEMU_RUN)
>  	$(Q)$(REPORT) "$(RUN_OUT)"
>  
>  # re-run the tests from an existing kernel
>  rerun:
> -	$(Q)$(QEMU_SYSTEM_RUN) > "$(RUN_OUT)"
> +	$(Q)$(TIMEOUT_QEMU_RUN)
>  	$(Q)$(REPORT) "$(RUN_OUT)"
>  
>  # report with existing test log
> -- 
> 2.25.1