* [Qemu-devel] [PATCH] tests: report errors when iasl exits with non-zero status
@ 2017-10-18 13:15 Daniel P. Berrange
2017-10-18 21:51 ` Philippe Mathieu-Daudé
2017-11-16 15:48 ` Michael S. Tsirkin
0 siblings, 2 replies; 3+ messages in thread
From: Daniel P. Berrange @ 2017-10-18 13:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Igor Mammedov, Daniel P. Berrange
If iasl exits with non-zero status, the test unhelpfully just reports
that the AML did not match, because the data files it thought iasl
generated do not exist. This adds an explicit check for the exit status
of iasl and prints stderr if it was non-zero. Thus gives us a fighting
chance of diagnosing why iasl failed.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
tests/bios-tables-test.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 564da45f65..ee441f1e17 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -304,6 +304,7 @@ static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
gchar *out, *out_err;
gboolean ret;
int i;
+ int status;
fd = g_file_open_tmp("asl-XXXXXX.dsl", &sdt->asl_file, &error);
g_assert_no_error(error);
@@ -324,14 +325,25 @@ static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
g_string_append_printf(command_line, "-d %s", sdt->aml_file);
/* pass 'out' and 'out_err' in order to be redirected */
- ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
+ ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, &status, &error);
g_assert_no_error(error);
if (ret) {
- ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
- &sdt->asl_len, &error);
- g_assert(ret);
- g_assert_no_error(error);
- ret = (sdt->asl_len > 0);
+ if (status != 0) {
+ g_printerr("'%s' exited with status %d", command_line->str, status);
+ if (!g_str_equal(out, "")) {
+ g_printerr("%s", out);
+ }
+ if (!g_str_equal(out_err, "")) {
+ g_printerr("%s", out_err);
+ }
+ ret = FALSE;
+ } else {
+ ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
+ &sdt->asl_len, &error);
+ g_assert(ret);
+ g_assert_no_error(error);
+ ret = (sdt->asl_len > 0);
+ }
}
g_free(out);
--
2.13.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: report errors when iasl exits with non-zero status
2017-10-18 13:15 [Qemu-devel] [PATCH] tests: report errors when iasl exits with non-zero status Daniel P. Berrange
@ 2017-10-18 21:51 ` Philippe Mathieu-Daudé
2017-11-16 15:48 ` Michael S. Tsirkin
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-10-18 21:51 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Igor Mammedov, Michael S. Tsirkin
On 10/18/2017 10:15 AM, Daniel P. Berrange wrote:
> If iasl exits with non-zero status, the test unhelpfully just reports
> that the AML did not match, because the data files it thought iasl
> generated do not exist. This adds an explicit check for the exit status
> of iasl and prints stderr if it was non-zero. Thus gives us a fighting
> chance of diagnosing why iasl failed.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> tests/bios-tables-test.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 564da45f65..ee441f1e17 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -304,6 +304,7 @@ static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> gchar *out, *out_err;
> gboolean ret;
> int i;
> + int status;
>
> fd = g_file_open_tmp("asl-XXXXXX.dsl", &sdt->asl_file, &error);
> g_assert_no_error(error);
> @@ -324,14 +325,25 @@ static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> g_string_append_printf(command_line, "-d %s", sdt->aml_file);
>
> /* pass 'out' and 'out_err' in order to be redirected */
> - ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> + ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, &status, &error);
> g_assert_no_error(error);
> if (ret) {
> - ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> - &sdt->asl_len, &error);
> - g_assert(ret);
> - g_assert_no_error(error);
> - ret = (sdt->asl_len > 0);
> + if (status != 0) {
> + g_printerr("'%s' exited with status %d", command_line->str, status);
> + if (!g_str_equal(out, "")) {
> + g_printerr("%s", out);
> + }
> + if (!g_str_equal(out_err, "")) {
> + g_printerr("%s", out_err);
> + }
> + ret = FALSE;
> + } else {
> + ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> + &sdt->asl_len, &error);
> + g_assert(ret);
> + g_assert_no_error(error);
> + ret = (sdt->asl_len > 0);
> + }
> }
>
> g_free(out);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] tests: report errors when iasl exits with non-zero status
2017-10-18 13:15 [Qemu-devel] [PATCH] tests: report errors when iasl exits with non-zero status Daniel P. Berrange
2017-10-18 21:51 ` Philippe Mathieu-Daudé
@ 2017-11-16 15:48 ` Michael S. Tsirkin
1 sibling, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2017-11-16 15:48 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: qemu-devel, Igor Mammedov
On Wed, Oct 18, 2017 at 02:15:28PM +0100, Daniel P. Berrange wrote:
> If iasl exits with non-zero status, the test unhelpfully just reports
> that the AML did not match, because the data files it thought iasl
> generated do not exist. This adds an explicit check for the exit status
> of iasl and prints stderr if it was non-zero. Thus gives us a fighting
> chance of diagnosing why iasl failed.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Given the state of iasl, I'm not sure this will not
give false positives where iasl returns an error status
but does produce a valid input.
I propose this is reworked to just add a warning about the
bad status, but still attempt to process the file.
> ---
> tests/bios-tables-test.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 564da45f65..ee441f1e17 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -304,6 +304,7 @@ static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> gchar *out, *out_err;
> gboolean ret;
> int i;
> + int status;
>
> fd = g_file_open_tmp("asl-XXXXXX.dsl", &sdt->asl_file, &error);
> g_assert_no_error(error);
> @@ -324,14 +325,25 @@ static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> g_string_append_printf(command_line, "-d %s", sdt->aml_file);
>
> /* pass 'out' and 'out_err' in order to be redirected */
> - ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> + ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, &status, &error);
> g_assert_no_error(error);
> if (ret) {
> - ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> - &sdt->asl_len, &error);
> - g_assert(ret);
> - g_assert_no_error(error);
> - ret = (sdt->asl_len > 0);
> + if (status != 0) {
> + g_printerr("'%s' exited with status %d", command_line->str, status);
> + if (!g_str_equal(out, "")) {
> + g_printerr("%s", out);
> + }
> + if (!g_str_equal(out_err, "")) {
> + g_printerr("%s", out_err);
> + }
> + ret = FALSE;
> + } else {
> + ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> + &sdt->asl_len, &error);
> + g_assert(ret);
> + g_assert_no_error(error);
> + ret = (sdt->asl_len > 0);
> + }
> }
>
> g_free(out);
> --
> 2.13.6
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-11-16 15:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-18 13:15 [Qemu-devel] [PATCH] tests: report errors when iasl exits with non-zero status Daniel P. Berrange
2017-10-18 21:51 ` Philippe Mathieu-Daudé
2017-11-16 15:48 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).