qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, Igor Mammedov <imammedo@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] tests: report errors when iasl exits with non-zero status
Date: Thu, 16 Nov 2017 17:48:55 +0200	[thread overview]
Message-ID: <20171116174731-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20171018131529.19453-1-berrange@redhat.com>

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

      parent reply	other threads:[~2017-11-16 15:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171116174731-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=berrange@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).