ACPI halt failures in tests

ACPI halt failures in tests

[Colin Watson]

I occasionally get "ACPI shutdown failed" in qemu during tests.  Since
this is printed with grub_printf rather than with grub_dprintf or
whatever, it shows up in the test output and causes tests to fail.  It
only fails some of the time on my system (less than half).  I've tried
to insert 'set debug=acpi' into the grub-shell test setup, but it
stubbornly succeeds when I do that, so I'm not really sure what else to
try.

The system seems to shut down happily anyway, presumably because it goes
on to try other boot methods.  Should we maybe downgrade this to a debug
message?

I've only seen this on Ubuntu, not on Debian, but I have no idea what's
different about our qemu packaging.

2010-11-23  Colin Watson  <cjwatson@ubuntu.com>

	* grub-core/commands/acpihalt.c (grub_acpi_halt): Downgrade "ACPI
	shutdown failed" to a debug message, since it can cause spurious
	test failures.

=== modified file 'grub-core/commands/acpihalt.c'
--- grub-core/commands/acpihalt.c	2010-10-16 18:01:30 +0000
+++ grub-core/commands/acpihalt.c	2010-11-23 15:27:41 +0000
@@ -324,6 +324,6 @@ grub_acpi_halt (void)
 	}
     }
 
-  grub_printf ("ACPI shutdown failed\n");
+  grub_dprintf ("acpi", "ACPI shutdown failed\n");
 }
 #endif

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

[PATCH] Re: ACPI halt failures in tests

[Jordan Uggla]

On Tue, Nov 23, 2010 at 7:29 AM, Colin Watson <cjwatson@ubuntu.com> wrote:
> I occasionally get "ACPI shutdown failed" in qemu during tests.  Since
> this is printed with grub_printf rather than with grub_dprintf or
> whatever, it shows up in the test output and causes tests to fail.  It
> only fails some of the time on my system (less than half).  I've tried
> to insert 'set debug=acpi' into the grub-shell test setup, but it
> stubbornly succeeds when I do that, so I'm not really sure what else to
> try.
>
> The system seems to shut down happily anyway, presumably because it goes
> on to try other boot methods.  Should we maybe downgrade this to a debug
> message?

I think it's more likely a race condition of sorts; it's assumed that
the code that prints "ACPI shutdown failed" is unreachable when ACPI
shutdown succeeds, since the code comes after shutdown has been
requested. Part of the reason I think this is that the symptom I've
been seeing is just "A" or "AC" being printed, which means that the
shutdown is not synchronous (it's happening in the middle of a
printf). Since in real hardware I can't see this being much of a
problem (except maybe causing false alarms for people who log serial
output), and it would be good to have some type of message on the
screen when ACPI shutdown does fail, I don't think this should be just
a dprintf. Instead I think that the unit test's grub.cfg should
disable serial terminal just before running halt as a work around.

=== modified file 'ChangeLog'
--- ChangeLog   2010-12-10 12:56:45 +0000
+++ ChangeLog   2010-12-23 09:02:56 +0000
@@ -1,3 +1,7 @@
+2010-12-23 Jordan Uggal <jordan.uggla@gmail.com>
+       * tests/util/grub-shell.in: Supress "ACPI shutdown failed" error
+       to keep unit tests from failing when they shouldn't.
+
 2010-12-10  Colin Watson  <cjwatson@ubuntu.com>

        * .bzrignore: Ignore grub-core/rs_decoder.S.

=== modified file 'tests/util/grub-shell.in'
--- tests/util/grub-shell.in    2010-10-18 20:50:01 +0000
+++ tests/util/grub-shell.in    2010-12-23 09:04:34 +0000
@@ -132,6 +132,8 @@

 cat <<EOF >>${cfgfile}
 source /boot/grub/testcase.cfg
+terminal_output console # Stop serial output to supress
+                        # "ACPI shutdown failed" error.
 halt
 EOF

-- 
Jordan Uggla (Jordan_U on irc.freenode.net)

Re: [PATCH] Re: ACPI halt failures in tests

[Colin Watson]

On Thu, Dec 23, 2010 at 01:07:15AM -0800, Jordan Uggla wrote:
> I think it's more likely a race condition of sorts; it's assumed that
> the code that prints "ACPI shutdown failed" is unreachable when ACPI
> shutdown succeeds, since the code comes after shutdown has been
> requested. Part of the reason I think this is that the symptom I've
> been seeing is just "A" or "AC" being printed, which means that the
> shutdown is not synchronous (it's happening in the middle of a
> printf). Since in real hardware I can't see this being much of a
> problem (except maybe causing false alarms for people who log serial
> output), and it would be good to have some type of message on the
> screen when ACPI shutdown does fail, I don't think this should be just
> a dprintf. Instead I think that the unit test's grub.cfg should
> disable serial terminal just before running halt as a work around.

Thanks!  This does indeed make more sense than my patch.  I've gone
ahead and committed it, with trivial tweaks (including fixing the
spelling of your name in ChangeLog ;-) ).

-- 
Colin Watson                                       [cjwatson@ubuntu.com]