[Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Thomas Huth]

If QEMU has been compiled with the flags --enable-tcg-interpreter and
--enable-debug, the guest is running incredibly slow. The pxe boot test
can take up to 400 seconds when testing the pseries ppc64 machine. While
we should still look for ways to speed up the test on the pseries machine,
it's better to increase the timeout in this test to 600 seconds anyway to
allow the test to pass successfully now with this unusal configuration
already.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/boot-sector.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/boot-sector.c b/tests/boot-sector.c
index 9ee8537..be29d5b 100644
--- a/tests/boot-sector.c
+++ b/tests/boot-sector.c
@@ -137,9 +137,9 @@ void boot_sector_test(void)
     uint16_t signature;
     int i;
 
-    /* Wait at most 90 seconds */
+    /* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */
 #define TEST_DELAY (1 * G_USEC_PER_SEC / 10)
-#define TEST_CYCLES MAX((90 * G_USEC_PER_SEC / TEST_DELAY), 1)
+#define TEST_CYCLES MAX((600 * G_USEC_PER_SEC / TEST_DELAY), 1)
 
     /* Poll until code has run and modified memory.  Once it has we know BIOS
      * initialization is done.  TODO: check that IP reached the halt
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Stefan Weil]

Am 22.09.2017 um 05:06 schrieb Thomas Huth:
> If QEMU has been compiled with the flags --enable-tcg-interpreter and
> --enable-debug, the guest is running incredibly slow. The pxe boot test
> can take up to 400 seconds when testing the pseries ppc64 machine. While
> we should still look for ways to speed up the test on the pseries machine,
> it's better to increase the timeout in this test to 600 seconds anyway to
> allow the test to pass successfully now with this unusal configuration
> already.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/boot-sector.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index 9ee8537..be29d5b 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -137,9 +137,9 @@ void boot_sector_test(void)
>      uint16_t signature;
>      int i;
>  
> -    /* Wait at most 90 seconds */
> +    /* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */
>  #define TEST_DELAY (1 * G_USEC_PER_SEC / 10)
> -#define TEST_CYCLES MAX((90 * G_USEC_PER_SEC / TEST_DELAY), 1)
> +#define TEST_CYCLES MAX((600 * G_USEC_PER_SEC / TEST_DELAY), 1)
>  
>      /* Poll until code has run and modified memory.  Once it has we know BIOS
>       * initialization is done.  TODO: check that IP reached the halt
> 

Reviewed-by: Stefan Weil <sw@weilnetz.de>

cc'ing qemu-trivial

Thanks,
Stefan

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Philippe Mathieu-Daudé]

Hi Thomas,

On 09/22/2017 12:06 AM, Thomas Huth wrote:
> If QEMU has been compiled with the flags --enable-tcg-interpreter and
> --enable-debug, the guest is running incredibly slow. The pxe boot test

There already is a qtest_get_arch(), it might be convenient to have a 
qtest_get_accel() at some point.

> can take up to 400 seconds when testing the pseries ppc64 machine. While
> we should still look for ways to speed up the test on the pseries machine,
> it's better to increase the timeout in this test to 600 seconds anyway to
> allow the test to pass successfully now with this unusal configuration
> already.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/boot-sector.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index 9ee8537..be29d5b 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -137,9 +137,9 @@ void boot_sector_test(void)
>       uint16_t signature;
>       int i;
>   
> -    /* Wait at most 90 seconds */
> +    /* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */
>   #define TEST_DELAY (1 * G_USEC_PER_SEC / 10)
> -#define TEST_CYCLES MAX((90 * G_USEC_PER_SEC / TEST_DELAY), 1)
> +#define TEST_CYCLES MAX((600 * G_USEC_PER_SEC / TEST_DELAY), 1)
>   
>       /* Poll until code has run and modified memory.  Once it has we know BIOS
>        * initialization is done.  TODO: check that IP reached the halt
> 

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Thomas Huth]

On 22.09.2017 08:21, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 09/22/2017 12:06 AM, Thomas Huth wrote:
>> If QEMU has been compiled with the flags --enable-tcg-interpreter and
>> --enable-debug, the guest is running incredibly slow. The pxe boot test
> 
> There already is a qtest_get_arch(), it might be convenient to have a
> qtest_get_accel() at some point.

The "accelerator" (or rather "decelerator") is still TCG in this case -
it's just its backend that is so slow.

Anyway, Lukáš recently reported that he could reproduce similar issues
with systems that are under heavy stress:

https://marc.info/?i=dafbe774-2544-f64e-aad2-9cf957025e8d@redhat.com

So I think we should apply this anyway, without any TCI-related checks.

 Thomas

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Michael Tokarev]

22.09.2017 06:06, Thomas Huth wrote:
> If QEMU has been compiled with the flags --enable-tcg-interpreter and
> --enable-debug, the guest is running incredibly slow. The pxe boot test
> can take up to 400 seconds when testing the pseries ppc64 machine. While
> we should still look for ways to speed up the test on the pseries machine,
> it's better to increase the timeout in this test to 600 seconds anyway to
> allow the test to pass successfully now with this unusal configuration
> already.

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Michael S. Tsirkin]

On Fri, Sep 22, 2017 at 05:06:57AM +0200, Thomas Huth wrote:
> If QEMU has been compiled with the flags --enable-tcg-interpreter and
> --enable-debug, the guest is running incredibly slow. The pxe boot test
> can take up to 400 seconds when testing the pseries ppc64 machine. While
> we should still look for ways to speed up the test on the pseries machine,
> it's better to increase the timeout in this test to 600 seconds anyway to
> allow the test to pass successfully now with this unusal configuration
> already.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Well I do break things sometimes and I won't be happy about
spending 10 minutes waiting for a test to fail.

Can we break out of the test if all CPUs are halted with
interrupts disabled?

> ---
>  tests/boot-sector.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/boot-sector.c b/tests/boot-sector.c
> index 9ee8537..be29d5b 100644
> --- a/tests/boot-sector.c
> +++ b/tests/boot-sector.c
> @@ -137,9 +137,9 @@ void boot_sector_test(void)
>      uint16_t signature;
>      int i;
>  
> -    /* Wait at most 90 seconds */
> +    /* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */
>  #define TEST_DELAY (1 * G_USEC_PER_SEC / 10)
> -#define TEST_CYCLES MAX((90 * G_USEC_PER_SEC / TEST_DELAY), 1)
> +#define TEST_CYCLES MAX((600 * G_USEC_PER_SEC / TEST_DELAY), 1)
>  
>      /* Poll until code has run and modified memory.  Once it has we know BIOS
>       * initialization is done.  TODO: check that IP reached the halt
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Michael S. Tsirkin]

On Mon, Sep 25, 2017 at 12:06:40AM +0300, Michael Tokarev wrote:
> 22.09.2017 06:06, Thomas Huth wrote:
> > If QEMU has been compiled with the flags --enable-tcg-interpreter and
> > --enable-debug, the guest is running incredibly slow. The pxe boot test
> > can take up to 400 seconds when testing the pseries ppc64 machine. While
> > we should still look for ways to speed up the test on the pseries machine,
> > it's better to increase the timeout in this test to 600 seconds anyway to
> > allow the test to pass successfully now with this unusal configuration
> > already.
> 
> Applied to -trivial, thanks!
> 
> /mjt

Please do not apply this, trivial is not appropriate for functional
changes like this.

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Peter Maydell]

On 26 September 2017 at 20:31, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Sep 25, 2017 at 12:06:40AM +0300, Michael Tokarev wrote:
>> 22.09.2017 06:06, Thomas Huth wrote:
>> > If QEMU has been compiled with the flags --enable-tcg-interpreter and
>> > --enable-debug, the guest is running incredibly slow. The pxe boot test
>> > can take up to 400 seconds when testing the pseries ppc64 machine. While
>> > we should still look for ways to speed up the test on the pseries machine,
>> > it's better to increase the timeout in this test to 600 seconds anyway to
>> > allow the test to pass successfully now with this unusal configuration
>> > already.
>>
>> Applied to -trivial, thanks!
>>
>> /mjt
>
> Please do not apply this, trivial is not appropriate for functional
> changes like this.

It's not a functional change, it's just bumping a test timeout.
If you think we should be doing something else that's fine (as
with any other patch), but in principle I think this is totally
fine as a -trivial patch.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Stefan Weil]

Am 26.09.2017 um 21:30 schrieb Michael S. Tsirkin:
> On Fri, Sep 22, 2017 at 05:06:57AM +0200, Thomas Huth wrote:
>> If QEMU has been compiled with the flags --enable-tcg-interpreter and
>> --enable-debug, the guest is running incredibly slow. The pxe boot test
>> can take up to 400 seconds when testing the pseries ppc64 machine. While
>> we should still look for ways to speed up the test on the pseries machine,
>> it's better to increase the timeout in this test to 600 seconds anyway to
>> allow the test to pass successfully now with this unusal configuration
>> already.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> Well I do break things sometimes and I won't be happy about
> spending 10 minutes waiting for a test to fail.
> 
> Can we break out of the test if all CPUs are halted with
> interrupts disabled?

Yes, that sounds like a scenario where no more progress
can be expected.

Any fixed timeout is either too long (when you expect a failing
test) or too short (when you need time for the test to pass).

Would it be possible to get some speed factor (like Linux
bogomips) at the beginning of all tests? Then that factor
could be used later for all tests which currently use a
hard timeout.

Regards
Stefan

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Thomas Huth]

On 26.09.2017 22:35, Stefan Weil wrote:
> Am 26.09.2017 um 21:30 schrieb Michael S. Tsirkin:
>> On Fri, Sep 22, 2017 at 05:06:57AM +0200, Thomas Huth wrote:
>>> If QEMU has been compiled with the flags --enable-tcg-interpreter and
>>> --enable-debug, the guest is running incredibly slow. The pxe boot test
>>> can take up to 400 seconds when testing the pseries ppc64 machine. While
>>> we should still look for ways to speed up the test on the pseries machine,
>>> it's better to increase the timeout in this test to 600 seconds anyway to
>>> allow the test to pass successfully now with this unusal configuration
>>> already.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>
>> Well I do break things sometimes and I won't be happy about
>> spending 10 minutes waiting for a test to fail.
>>
>> Can we break out of the test if all CPUs are halted with
>> interrupts disabled?

qemu-system-s390x exits automatically in that case (unless you specified
the -no-shutdown parameter) ... what about x86?

Another idea: We could run the "info registers" HMP command regularly
(is there an equivalent for HMP?) to see whether the guest is still
doing something and abort if the all registers stayed the same for,
let's say 30 seconds?

> Any fixed timeout is either too long (when you expect a failing
> test) or too short (when you need time for the test to pass).
> 
> Would it be possible to get some speed factor (like Linux
> bogomips) at the beginning of all tests? Then that factor
> could be used later for all tests which currently use a
> hard timeout.

We've got at least three changing factors here: Hardware speed of the
host (MHz / bogomips), speed of TCG / the emulated guest (e.g. TCI is
slow) and current other workload of the host. Even if we get the first
two factors right, the workload of the host can still change very
dynamically, so I think it's quite hard to do a good prediction at the
beginning of the test here.

 Thomas

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Michael S. Tsirkin]

On Tue, Sep 26, 2017 at 08:35:59PM +0100, Peter Maydell wrote:
> On 26 September 2017 at 20:31, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Sep 25, 2017 at 12:06:40AM +0300, Michael Tokarev wrote:
> >> 22.09.2017 06:06, Thomas Huth wrote:
> >> > If QEMU has been compiled with the flags --enable-tcg-interpreter and
> >> > --enable-debug, the guest is running incredibly slow. The pxe boot test
> >> > can take up to 400 seconds when testing the pseries ppc64 machine. While
> >> > we should still look for ways to speed up the test on the pseries machine,
> >> > it's better to increase the timeout in this test to 600 seconds anyway to
> >> > allow the test to pass successfully now with this unusal configuration
> >> > already.
> >>
> >> Applied to -trivial, thanks!
> >>
> >> /mjt
> >
> > Please do not apply this, trivial is not appropriate for functional
> > changes like this.
> 
> It's not a functional change, it's just bumping a test timeout.
> If you think we should be doing something else that's fine (as
> with any other patch), but in principle I think this is totally
> fine as a -trivial patch.
> 
> thanks
> -- PMM

OK. I'd rather not see it applied as-is though.

-- 
MST

Re: [Qemu-devel] [PATCH] tests/boot-sector: Increase timeout to 600 seconds

[Peter Maydell]

On 27 September 2017 at 16:14, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Sep 26, 2017 at 08:35:59PM +0100, Peter Maydell wrote:
>> On 26 September 2017 at 20:31, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > Please do not apply this, trivial is not appropriate for functional
>> > changes like this.
>>
>> It's not a functional change, it's just bumping a test timeout.
>> If you think we should be doing something else that's fine (as
>> with any other patch), but in principle I think this is totally
>> fine as a -trivial patch.

> OK. I'd rather not see it applied as-is though.

Oops, looks like it's already hit master (07897000194a).
I can revert it if you feel strongly about it, but my personal
feeling is that the timeout needs to be long enough that you
don't hit it in any configuration. Usually if you're running
tests yourself then you'll notice it's stuck before it
hits a timeout -- the timeout is just so that fully automated
test runs don't hang forever.

thanks
-- PMM