diff mbox series

tests/hexloader-test: Don't pass -nographic to the QEMU under test

Message ID 20190104145018.16950-1-peter.maydell@linaro.org
State Superseded
Headers show
Series tests/hexloader-test: Don't pass -nographic to the QEMU under test | expand

Commit Message

Peter Maydell Jan. 4, 2019, 2:50 p.m. UTC
The hexloader test invokes QEMU with the -nographic argument. This
is unnecessary, because the qtest_initf() function will pass it
-display none, which suffices to disable the graphical window.
It also means that the QEMU process will make the stdin/stdout
O_NONBLOCK. Since O_NONBLOCK is not per-file descriptor but per
"file description", this non-blocking behaviour is then shared
with any other process that's using the stdin/stdout of the
'make check' run, including make itself. This can result in make
falling over with "make: write error: stdout" because it got
an unexpected EINTR trying to write output messages to the terminal.
This is particularly noticable if running 'make check' in a loop with
  while make check; do true; done
(It does not affect single make check runs so much because the
shell will remove the O_NONBLOCK status before it reads the
terminal for interactive input.)

Remove the unwanted -nographic argument.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
This seems to be sufficient to resolve my "make falls over"
issues with the current test harness and a build done for
arm targets; I haven't checked whether other test cases which
are specific to other target architectures might have similar
accidental O_NONBLOCK behaviour.

 tests/hexloader-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.19.2

Comments

Thomas Huth Jan. 4, 2019, 3:26 p.m. UTC | #1
On 2019-01-04 15:50, Peter Maydell wrote:
> The hexloader test invokes QEMU with the -nographic argument. This

> is unnecessary, because the qtest_initf() function will pass it

> -display none, which suffices to disable the graphical window.

> It also means that the QEMU process will make the stdin/stdout

> O_NONBLOCK. Since O_NONBLOCK is not per-file descriptor but per

> "file description", this non-blocking behaviour is then shared

> with any other process that's using the stdin/stdout of the

> 'make check' run, including make itself. This can result in make

> falling over with "make: write error: stdout" because it got

> an unexpected EINTR trying to write output messages to the terminal.

> This is particularly noticable if running 'make check' in a loop with

>   while make check; do true; done

> (It does not affect single make check runs so much because the

> shell will remove the O_NONBLOCK status before it reads the

> terminal for interactive input.)

> 

> Remove the unwanted -nographic argument.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This seems to be sufficient to resolve my "make falls over"

> issues with the current test harness and a build done for

> arm targets; I haven't checked whether other test cases which

> are specific to other target architectures might have similar

> accidental O_NONBLOCK behaviour.

> 

>  tests/hexloader-test.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c

> index 834ed52c22b..8b7aa2d72d0 100644

> --- a/tests/hexloader-test.c

> +++ b/tests/hexloader-test.c

> @@ -23,7 +23,7 @@ static void hex_loader_test(void)

>      const unsigned int base_addr = 0x00010000;

>  

>      QTestState *s = qtest_initf(

> -        "-M vexpress-a9 -nographic -device loader,file=tests/data/hex-loader/test.hex");

> +        "-M vexpress-a9 -device loader,file=tests/data/hex-loader/test.hex");

>  

>      for (i = 0; i < 256; ++i) {

>          uint8_t val = qtest_readb(s, base_addr + i);


Reviewed-by: Thomas Huth <thuth@redhat.com>
Paolo Bonzini Jan. 4, 2019, 7:56 p.m. UTC | #2
On 04/01/19 15:50, Peter Maydell wrote:
> The hexloader test invokes QEMU with the -nographic argument. This

> is unnecessary, because the qtest_initf() function will pass it

> -display none, which suffices to disable the graphical window.

> It also means that the QEMU process will make the stdin/stdout

> O_NONBLOCK. Since O_NONBLOCK is not per-file descriptor but per

> "file description", this non-blocking behaviour is then shared

> with any other process that's using the stdin/stdout of the

> 'make check' run, including make itself. This can result in make

> falling over with "make: write error: stdout" because it got

> an unexpected EINTR trying to write output messages to the terminal.

> This is particularly noticable if running 'make check' in a loop with

>   while make check; do true; done

> (It does not affect single make check runs so much because the

> shell will remove the O_NONBLOCK status before it reads the

> terminal for interactive input.)

> 

> Remove the unwanted -nographic argument.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> This seems to be sufficient to resolve my "make falls over"

> issues with the current test harness and a build done for

> arm targets; I haven't checked whether other test cases which

> are specific to other target architectures might have similar

> accidental O_NONBLOCK behaviour.

> 

>  tests/hexloader-test.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c

> index 834ed52c22b..8b7aa2d72d0 100644

> --- a/tests/hexloader-test.c

> +++ b/tests/hexloader-test.c

> @@ -23,7 +23,7 @@ static void hex_loader_test(void)

>      const unsigned int base_addr = 0x00010000;

>  

>      QTestState *s = qtest_initf(

> -        "-M vexpress-a9 -nographic -device loader,file=tests/data/hex-loader/test.hex");

> +        "-M vexpress-a9 -device loader,file=tests/data/hex-loader/test.hex");

>  

>      for (i = 0; i < 256; ++i) {

>          uint8_t val = qtest_readb(s, base_addr + i);

> 


Great, I'll put it in my pull request together with the TAP driver
(however, I'll also put in the </dev/null for good measure).

Paolo
diff mbox series

Patch

diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
index 834ed52c22b..8b7aa2d72d0 100644
--- a/tests/hexloader-test.c
+++ b/tests/hexloader-test.c
@@ -23,7 +23,7 @@  static void hex_loader_test(void)
     const unsigned int base_addr = 0x00010000;
 
     QTestState *s = qtest_initf(
-        "-M vexpress-a9 -nographic -device loader,file=tests/data/hex-loader/test.hex");
+        "-M vexpress-a9 -device loader,file=tests/data/hex-loader/test.hex");
 
     for (i = 0; i < 256; ++i) {
         uint8_t val = qtest_readb(s, base_addr + i);