[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warni
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible |
Date: |
Wed, 26 Feb 2014 17:39:39 +0200 |
On Wed, 2014-02-26 at 17:25 +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> > If the expected (offline) acpi tables loaded correctly,
> > it is safe to assume the iasl installation is OK and
> > issue an error if the actual tables differ from expected
> > ones.
> >
> > Signed-off-by: Marcel Apfelbaum <address@hidden>
>
> I'm not sure I agree with this one.
> It turned out to be too aggressive in the past
> as expected files get out of sync sometimes.
> What I would do is this:
> if IASL did not produce errors or warnings
> on expected files, it should not produce
> them on actual files either.
The whole point of this patch (series) was to find
a way to discover "real" errors in some cases (not in all).
What you suggested above removes the only "discoverable error"
and returns us to warning only mode.
However, the code does not look into stdout/stderr for iasl
errors (it would involve parsing because iasl *always* writes
to both), but checks if the invocation of iasl command terminates successfully,
merely saying: "Well, iasl invocation succeeded for the expected files, there
is no reason to fail for the actual ones". The actual output is not verified.
No impact on "out of sync expected files".
Thanks,
Marcel
>
>
> > ---
> > tests/acpi-test.c | 24 ++++++++++++++++--------
> > 1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> > index 2ce8c18..639b3ab 100644
> > --- a/tests/acpi-test.c
> > +++ b/tests/acpi-test.c
> > @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt,
> > uint32_t signature)
> > return sdt->header.signature == signature;
> > }
> >
> > -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > {
> > AcpiSdtTable *temp;
> > GError *error = NULL;
> > @@ -440,18 +440,22 @@ static void 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 */
> > - 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,
> > NULL, &error);
> > g_assert_no_error(error);
> >
> > - ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > - &sdt->asl_len, &error);
> > - g_assert(ret);
> > - g_assert_no_error(error);
> > - g_assert(sdt->asl_len);
> > + 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);
> > + g_assert(sdt->asl_len);
> > + }
> >
> > g_free(out);
> > g_free(out_err);
> > g_string_free(command_line, true);
> > +
> > + return !ret;
> > }
> >
> > #define COMMENT_END "*/"
> > @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
> > int i;
> > AcpiSdtTable *sdt, *exp_sdt;
> > test_data exp_data;
> > + gboolean err;
> >
> > memset(&exp_data, 0, sizeof(exp_data));
> > exp_data.tables = load_expected_aml(data);
> > @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
> > load_asl(data->tables, sdt);
> > asl = normalize_asl(sdt->asl);
> >
> > - load_asl(exp_data.tables, exp_sdt);
> > + err = load_asl(exp_data.tables, exp_sdt);
> > exp_asl = normalize_asl(exp_sdt->asl);
> >
> > if (g_strcmp0(asl->str, exp_asl->str)) {
> > @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
> > (gchar *)&exp_sdt->header.signature,
> > sdt->asl_file, sdt->aml_file,
> > exp_sdt->asl_file, exp_sdt->aml_file);
> > + if (!err) { /* expected data loaded, iasl OK */
> > + g_assert(false);
> > + }
> > }
> > g_string_free(asl, true);
> > g_string_free(exp_asl, true);
> > --
> > 1.8.3.1
[Qemu-devel] [PATCH v2 1/2] acpi-test: retain both asl and aml files on failure, Marcel Apfelbaum, 2014/02/24