[PATCH] Introduce olog scan, to check OPAL msglog.
Deb McLemore
debmc at linux.vnet.ibm.com
Tue Mar 22 19:01:20 UTC 2016
I'll summarize the reply here to pull it out of the content below.
I'll provide a patch specific to the args parsing bug that was exposed
by adding new options.
For how the internal special file handling is performed I'll have to dig
into that one unless someone on copy can elaborate. When I tried to
first get the size of the 0 byte msglog of course that doesn't help
figuring out how big a buffer, and when I tried to seek to EOF that
would not reliably actually get me the full output (internal zero bytes,
etc). So somehow the "cat" internals handles this which I'll dig into.
On Tue, 2016-03-22 at 17:10 +0000, Colin Ian King wrote:
> On 22/03/16 16:59, Deborah McLemore wrote:
> > Replies in-lined with >>debmc:
> > =====================================
> > Deb McLemore
> > IBM OpenPower - IBM Systems
> > (512) 286 9980
> >
> > debmc at us.ibm.com
> > debmc at linux.vnet.ibm.com - (plain text)
> > =====================================
> >
> > Inactive hide details for Colin Ian King ---03/22/2016 11:20:26
> > AM---Thanks for you contribution, comments are in-lined below..Colin Ian
> > King ---03/22/2016 11:20:26 AM---Thanks for you contribution, comments
> > are in-lined below... On 21/03/16 20:42, Deb McLemore wrote:
> >
> > From: Colin Ian King <colin.king at canonical.com>
> > To: Deb McLemore <debmc at linux.vnet.ibm.com>, fwts-devel at lists.ubuntu.com
> > Date: 03/22/2016 11:20 AM
> > Subject: Re: [PATCH] Introduce olog scan, to check OPAL msglog.
> > Sent by: fwts-devel-bounces at lists.ubuntu.com
> >
> > ------------------------------------------------------------------------
> >
> >
> >
> > Thanks for you contribution, comments are in-lined below...
> >
> > On 21/03/16 20:42, Deb McLemore wrote:
> >> From: Deb McLemore <debmc at linux.vnet.ibm.com>
> >>
> >> This feature adds the olog option which will scan for patterns defined
> >> in the olog.json data file and compare against the PPC OPAL firmware
> >> msglog. Extending similar capabilities of the klog functionality,
> >> this feature also allows customization to specify any file as the
> >> source and any JSON data file as the patterns.
> >>
> >> Signed-off-by: Deb McLemore <debmc at linux.vnet.ibm.com>
> >> ---
> >> data/Makefile.am | 2 +-
> >> data/olog.json | 33 ++++++
> >> src/Makefile.am | 1 +
> >> src/kernel/olog/olog.c | 95 +++++++++++++++++
> >> src/lib/include/fwts.h | 6 ++
> >> src/lib/include/fwts_framework.h | 3 +
> >> src/lib/include/fwts_klog.h | 6 ++
> >> src/lib/include/fwts_olog.h | 36 +++++++
> >> src/lib/src/Makefile.am | 1 +
> >> src/lib/src/fwts_args.c | 33 +++---
> >> src/lib/src/fwts_framework.c | 86 +++++++++------
> >> src/lib/src/fwts_klog.c | 15 ++-
> >> src/lib/src/fwts_olog.c | 222
> > +++++++++++++++++++++++++++++++++++++++
> >> src/lib/src/fwts_stringextras.c | 4 +
> >> 14 files changed, 488 insertions(+), 55 deletions(-)
> >> create mode 100644 data/olog.json
> >> create mode 100644 src/kernel/olog/olog.c
> >> create mode 100644 src/lib/include/fwts_olog.h
> >> create mode 100644 src/lib/src/fwts_olog.c
> >>
> >> diff --git a/data/Makefile.am b/data/Makefile.am
> >> index f9ee508..525dede 100644
> >> --- a/data/Makefile.am
> >> +++ b/data/Makefile.am
> >> @@ -1,2 +1,2 @@
> >> fwtsdatadir = $(pkgdatadir)
> >> -fwtsdata_DATA = klog.json syntaxcheck.json
> >> +fwtsdata_DATA = klog.json syntaxcheck.json olog.json
> >> diff --git a/data/olog.json b/data/olog.json
> >> new file mode 100644
> >> index 0000000..3dbd63c
> >> --- /dev/null
> >> +++ b/data/olog.json
> >> @@ -0,0 +1,33 @@
> >> +{
> >
> > I guess the following patterns are going to be replaced with real data
> > at some point.
> >
> >>>debmc - Yes, these samples will flag some interesting log entries for
> > now, but will be refined in future contributions.
> >
> >> + "olog_error_warning_patterns":
> >> + [
> >> + {
> >> + "compare_mode": "string",
> >> + "log_level": "LOG_LEVEL_CRITICAL",
> >> + "pattern": "CRITICAL",
> >> + "advice": "SAMPLE TEXT -> Check your log for this condition and
> > give some specific investigative and action steps.",
> >> + "label": "OLOG_Filter_GROUP1"
> >> + },
> >> + {
> >> + "compare_mode": "string",
> >> + "log_level": "LOG_LEVEL_HIGH",
> >> + "pattern": "STOP",
> >> + "advice": "SAMPLE TEXT -> Check your log for this condition and
> > give some specific investigative and action steps.",
> >> + "label": "OLOG_Filter_GROUPA"
> >> + },
> >> + {
> >> + "compare_mode": "string",
> >> + "log_level": "LOG_LEVEL_MEDIUM",
> >> + "pattern": "STOP",
> >> + "advice": "SAMPLE TEXT -> Check your log for this condition and
> > give some specific investigative and action steps.",
> >> + "label": "OLOG_Filter_GROUPB"
> >> + },
> >> + {
> >> + "compare_mode": "regex",
> >> + "log_level": "LOG_LEVEL_LOW",
> >> + "pattern": "Trying.*",
> >> + "advice": "SAMPLE TEXT -> This needs further investigation and
> > review, please take xyz corrective action.",
> >> + "label": "OLOG_Filter_GROUPC"
> >> + }
> >> + ]
> >> +}
> >> diff --git a/src/Makefile.am b/src/Makefile.am
> >> index e1332a2..afe7323 100644
> >> --- a/src/Makefile.am
> >> +++ b/src/Makefile.am
> >> @@ -116,6 +116,7 @@ fwts_SOURCES = main.c \
> >> dmi/dmicheck/dmicheck.c \
> >> hotkey/hotkey/hotkey.c \
> >> kernel/klog/klog.c \
> >> + kernel/olog/olog.c \
> >> kernel/oops/oops.c \
> >> kernel/version/version.c \
> >> pci/aspm/aspm.c \
> >> diff --git a/src/kernel/olog/olog.c b/src/kernel/olog/olog.c
> >> new file mode 100644
> >> index 0000000..2df9f6c
> >> --- /dev/null
> >> +++ b/src/kernel/olog/olog.c
> >> @@ -0,0 +1,95 @@
> >> +/*
> >> + * Copyright (C) 2010-2016 Canonical
> >> + * Some of this work - Copyright (C) 2016 IBM
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * as published by the Free Software Foundation; either version 2
> >> + * of the License, or (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301, USA.
> >> + *
> >> + */
> >> +#include "fwts.h"
> >> +
> >> +#include <stdlib.h>
> >> +#include <stdio.h>
> >> +#include <sys/types.h>
> >> +#include <sys/stat.h>
> >> +#include <unistd.h>
> >> +
> >> +static fwts_list *olog;
> >> +
> >> +static int olog_init(fwts_framework *fw)
> >> +{
> >> + if (fw->olog) {
> >> + olog = fwts_file_open_and_read(fw->olog);
> >> + if (olog == NULL) {
> >> + fwts_log_error(fw, "OLOG -o file %s may not exist, please check that
> > the file exits and is good.", fw->olog);
> >> + return FWTS_ERROR;
> >> + }
> >> + }
> >> + else {
> >> + olog = fwts_olog_read();
> >> + if (olog == NULL) {
> >> + fwts_log_error(fw, "OLOG without any parameters on the platform you
> > are running does nothing, please specify -o for custom log analysis.
> > PPC supports dump and analysis of the default firmware logs.");
> >> + return FWTS_ERROR;
> >
> > missing indentation ^
> >
> >>>debmc - Will fix.
> >
> >> + }
> >> + }
> >> +
> >> + return FWTS_OK;
> >> +}
> >> +
> >> +static int olog_deinit(fwts_framework *fw)
> >> +{
> >> + FWTS_UNUSED(fw);
> >> +
> >> + fwts_klog_free(olog);
> >> +
> >> + return FWTS_OK;
> >> +}
> >> +
> >> +static void olog_progress(fwts_framework *fw, int progress)
> >> +{
> >> + fwts_progress(fw, progress);
> >> +}
> >> +
> >> +static int olog_test1(fwts_framework *fw)
> >> +{
> >> + int errors = 0;
> >> +
> >> + if (fwts_olog_firmware_check(fw, olog_progress, olog, &errors)) {
> >> + fwts_log_error(fw, "Problem in the OLOG processing, see earlier in
> > this log for details on the problem.");
> >> + return FWTS_ERROR;
> >> + }
> >> +
> >> + if (errors > 0)
> >> + /* Checks will log errors as failures automatically */
> >> + fwts_log_info(fw, "OLOG scan and analysis found %d unique issue(s).",
> >> + errors);
> >> + else
> >> + fwts_passed(fw, "OLOG scan and analysis passed.");
> >> +
> >> + return FWTS_OK;
> >> +}
> >> +
> >> +static fwts_framework_minor_test olog_tests[] = {
> >> + { olog_test1, "OLOG scan and analysis checks results." },
> >> + { NULL, NULL }
> >> +};
> >> +
> >> +static fwts_framework_ops olog_ops = {
> >> + .description = "Run OLOG scan and analysis checks.",
> >> + .init = olog_init,
> >> + .deinit = olog_deinit,
> >> + .minor_tests = olog_tests
> >> +};
> >> +
> >> +FWTS_REGISTER("olog", &olog_ops, FWTS_TEST_EARLY, FWTS_FLAG_BATCH)
> >> diff --git a/src/lib/include/fwts.h b/src/lib/include/fwts.h
> >> index d708201..cbc417e 100644
> >> --- a/src/lib/include/fwts.h
> >> +++ b/src/lib/include/fwts.h
> >> @@ -1,5 +1,6 @@
> >> /*
> >> * Copyright (C) 2010-2016 Canonical
> >> + * Some of this work - Copyright (C) 2016 IBM
> >> *
> >> * This program is free software; you can redistribute it and/or
> >> * modify it under the terms of the GNU General Public License
> >> @@ -38,6 +39,10 @@
> >> #define FWTS_ARCH_S390X 1
> >> #endif
> >>
> >> +#if defined(__PPC64__)
> >> +#define FWTS_HAS_ACPI 0
> >> +#define FWTS_HAS_UEFI 0
> >> +#endif
> >>
> >> #define FWTS_UNUSED(var) (void)var
> >>
> >> @@ -67,6 +72,7 @@
> >> #include "fwts_gpe.h"
> >> #include "fwts_iasl.h"
> >> #include "fwts_klog.h"
> >> +#include "fwts_olog.h"
> >> #include "fwts_pipeio.h"
> >> #include "fwts_stringextras.h"
> >> #include "fwts_tty.h"
> >> diff --git a/src/lib/include/fwts_framework.h
> > b/src/lib/include/fwts_framework.h
> >> index c9ea4bb..6c2332a 100644
> >> --- a/src/lib/include/fwts_framework.h
> >> +++ b/src/lib/include/fwts_framework.h
> >> @@ -1,5 +1,6 @@
> >> /*
> >> * Copyright (C) 2010-2016 Canonical
> >> + * Some of this work - Copyright (C) 2016 IBM
> >> *
> >> * This program is free software; you can redistribute it and/or
> >> * modify it under the terms of the GNU General Public License
> >> @@ -117,7 +118,9 @@ typedef struct fwts_framework {
> >> char *acpi_table_path; /* path to raw ACPI tables */
> >> char *acpi_table_acpidump_file; /* path to ACPI dump file */
> >> char *klog; /* path to dump of kernel log */
> >> + char *olog; /* path to OLOG */
> >> char *json_data_path; /* path to application json data files, e.g.
> > json klog data */
> >> + char *json_data_file; /* json file to use for olog analysis */
> >>
> >> fwts_framework_flags flags;
> >>
> >> diff --git a/src/lib/include/fwts_klog.h b/src/lib/include/fwts_klog.h
> >> index db68232..3f0d431 100644
> >> --- a/src/lib/include/fwts_klog.h
> >> +++ b/src/lib/include/fwts_klog.h
> >> @@ -26,6 +26,7 @@
> >> #include "fwts_list.h"
> >> #include "fwts_framework.h"
> >> #include "fwts_log.h"
> >> +#include "fwts_json.h"
> >>
> >> #define KERN_WARNING 0x00000001
> >> #define KERN_ERROR 0x00000002
> >> @@ -47,6 +48,7 @@ typedef struct {
> >> bool compiled_ok;
> >> } fwts_klog_pattern;
> >>
> >> +
> >> typedef void (*fwts_klog_progress_func)(fwts_framework *fw, int percent);
> >> typedef void (*fwts_klog_scan_func)(fwts_framework *fw, char *line,
> > int repeated, char *prevline, void *private, int *errors);
> >>
> >> @@ -63,4 +65,8 @@ int fwts_klog_regex_find(fwts_framework *fw,
> > fwts_list *klog, char *pattern);
> >> char *fwts_klog_remove_timestamp(char *text);
> >> int fwts_klog_write(fwts_framework *fw, const char *msg);
> >>
> >> +fwts_compare_mode fwts_klog_compare_mode_str_to_val(const char *str);
> >> +char *fwts_klog_unique_label(const char *str);
> >> +const char *fwts_json_str(fwts_framework *fw, const char *table, int
> > index, json_object *obj, const char *key, bool log_error);
> >> +
> >> #endif
> >> diff --git a/src/lib/include/fwts_olog.h b/src/lib/include/fwts_olog.h
> >> new file mode 100644
> >> index 0000000..4ccca57
> >> --- /dev/null
> >> +++ b/src/lib/include/fwts_olog.h
> >> @@ -0,0 +1,36 @@
> >> +/*
> >> + * Copyright (C) 2010-2016 Canonical
> >> + * Some of this work - Copyright (C) 2016 IBM
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * as published by the Free Software Foundation; either version 2
> >> + * of the License, or (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301, USA.
> >> + *
> >> + */
> >> +
> >> +#ifndef __FWTS_OPAL_H__
> >> +#define __FWTS_OPAL_H__
> >> +
> >> +#include <sys/types.h>
> >> +#include <regex.h>
> >> +
> >> +#include "fwts_list.h"
> >> +#include "fwts_framework.h"
> >> +#include "fwts_log.h"
> >> +
> >> +fwts_list *fwts_olog_read(void);
> >> +
> >> +typedef void (*fwts_olog_progress_func)(fwts_framework *fw, int percent);
> >> +int fwts_olog_firmware_check(fwts_framework *fw,
> > fwts_olog_progress_func progress, fwts_list *olog, int *errors);
> >> +
> >> +#endif
> >> diff --git a/src/lib/src/Makefile.am b/src/lib/src/Makefile.am
> >> index 5d63804..5302851 100644
> >> --- a/src/lib/src/Makefile.am
> >> +++ b/src/lib/src/Makefile.am
> >> @@ -55,6 +55,7 @@ libfwts_la_SOURCES = \
> >> fwts_ioport.c \
> >> fwts_keymap.c \
> >> fwts_klog.c \
> >> + fwts_olog.c \
> >> fwts_list.c \
> >> fwts_log.c \
> >> fwts_log_html.c \
> >> diff --git a/src/lib/src/fwts_args.c b/src/lib/src/fwts_args.c
> >> index 43c8ee8..a0e43d8 100644
> >> --- a/src/lib/src/fwts_args.c
> >> +++ b/src/lib/src/fwts_args.c
> >> @@ -1,5 +1,6 @@
> >> /*
> >> * Copyright (C) 2011-2016 Canonical
> >> + * Some of this work - Copyright (C) 2016 IBM
> >> *
> >> * This program is free software; you can redistribute it and/or
> >> * modify it under the terms of the GNU General Public License
> >> @@ -102,6 +103,8 @@ int fwts_args_parse(fwts_framework *fw, const int
> > argc, char * const argv[])
> >> int i;
> >> int c;
> >> int option_index;
> >> + int master_option_index;
> >> + int translated_long_option_index;
> >> int ret = FWTS_OK;
> >> char *short_options = NULL;
> >> size_t short_options_len = 0;
> >> @@ -162,6 +165,8 @@ int fwts_args_parse(fwts_framework *fw, const int
> > argc, char * const argv[])
> >> }
> >>
> >> for (;;) {
> >> + master_option_index = total_options;
> >> + translated_long_option_index = 0;
> >> c = getopt_long(argc, argv, short_options, long_options, &option_index);
> >> if (c == -1)
> >> break;
> >> @@ -170,38 +175,36 @@ int fwts_args_parse(fwts_framework *fw, const
> > int argc, char * const argv[])
> >> bool found = false;
> >>
> >> if (c != 0) {
> >> - for (i=0; i<options_table->num_options; i++, n++) {
> >> + for (i=0; i<options_table->num_options; i++) {
> >> char *short_name = options_table->options[i].short_name;
> >> if (index(short_name, c) != NULL) {
> >> found = true;
> >> break;
> >> }
> >> }
> >> - } else if (options_table->num_options > option_index)
> >> - found = true;
> >> + } else { /* c is zero for long option cases but we need the right
> > optarg_handler set */
> >> + for (i=0; i<options_table->num_options; i++) {
> >> + if
> > (strcmp(options_table->options[i].long_name,long_options[option_index].name)
> > == 0) {
> >> + translated_long_option_index = i;
> >> + found = true;
> >> + break;
> >> + }
> >> + }
> >> + }
> >
> > I'm trying to figure out what issue/feature the above changes to
> > fwts_args_parse() are solving. Can you elaborate?
> >
> >>>debmc - In the original source tree the long options don't work for
> > anything but the first options_table (try it). So without this fix when
> > using the getopt_long and really using the --json-data-file for example
> > or the --olog long option the optarg_handler gets called without the
> > proper entry point so nothing happens (but if the table entry is in the
> > first say 11 args block things work). It took quite a bit of debug and
> > analysis to figure out that on each iteration of the logic each
> > iteration of options_table had to adjust for the offset of its
> > options_index to the long_options.
>
> Oh, nice find! Can you send that particular fix as a separate patch as
> it's more of a generic fix and not related to the olog code.
>
> >
> >>
> >> /* Found an option, then run the appropriate handler */
> >> if (found) {
> >> - ret = options_table->optarg_handler(fw, argc, argv, c, option_index);
> >> + ret = options_table->optarg_handler(fw, argc, argv, c,
> > translated_long_option_index);
> >> if (ret != FWTS_OK)
> >> goto exit;
> >> break;
> >> } else {
> >> - option_index -= options_table->num_options;
> >> + master_option_index -= options_table->num_options;
> >> }
> >> - }
> >> - }
> >> -
> >> - /* We've collected all the args, now sanity check the values */
> >>
> >> - fwts_list_foreach(item, &options_list) {
> >> - options_table = fwts_list_data(fwts_options_table *, item);
> >> - if (options_table->optarg_check != NULL) {
> >> - ret = options_table->optarg_check(fw);
> >> - if (ret != FWTS_OK)
> >> - break;
> >> }
> >> }
> >> +
> >> exit:
> >> free(short_options);
> >> free(long_options);
> >> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> >> index fcf2e86..2bfc753 100644
> >> --- a/src/lib/src/fwts_framework.c
> >> +++ b/src/lib/src/fwts_framework.c
> >> @@ -1,5 +1,6 @@
> >> /*
> >> * Copyright (C) 2010-2016 Canonical
> >> + * Some of this work - Copyright (C) 2016 IBM
> >> *
> >> * This program is free software; you can redistribute it and/or
> >> * modify it under the terms of the GNU General Public License
> >> @@ -94,6 +95,7 @@ static fwts_option fwts_framework_options[] = {
> >> { "show-progress", "p", 0, "Output test progress report to stderr." },
> >> { "show-tests", "s", 0, "Show available tests." },
> >> { "klog", "k:", 1, "Specify kernel log file rather than reading it
> > from the kernel, e.g. --klog=dmesg.log" },
> >> + { "olog", "o:", 1, "Specify Other logs to be analyzed, main usage is
> > for custom log analysis, best to use custom json file for pattern
> > matching, e.g. -o /var/log/my_opal_msglog --json-data-file=olog.json
> > --json-data-path=/home/myuser, on PPC this will default to dumping the
> > OPAL msglog for analysis." },
> >> { "log-width", "w:", 1, "Define the output log width in characters." },
> >> { "lspci", "", 1, "Specify path to lspci, e.g. --lspci=path." },
> >> { "batch", "b", 0, "Run non-Interactive tests." },
> >> @@ -113,6 +115,7 @@ static fwts_option fwts_framework_options[] = {
> >> { "show-tests-full", "", 0, "Show available tests including all
> > minor tests." },
> >> { "utils", "u", 0, "Run Utility 'tests'." },
> >> { "json-data-path", "j:", 1, "Specify path to fwts json data files -
> > default is /usr/share/fwts." },
> >> + { "json-data-file", "J:", 1, "Specify the file to use for pattern
> > matching on --olog, you may need to specify the json-data-path also if
> > non-default location." },
> >> { "disassemble-aml", "", 2, "Disassemble AML from DSDT and SSDT
> > tables." },
> >> { "log-type", "", 1, "Specify log type (plaintext, json, html or
> > xml)." },
> >> { "unsafe", "U", 0, "Unsafe tests (tests that can potentially cause
> > kernel oopses)." },
> >> @@ -136,6 +139,7 @@ static fwts_list fwts_framework_test_list =
> > FWTS_LIST_INIT;
> >> static const char *fwts_copyright[] = {
> >> "Some of this work - Copyright (c) 1999 - 2016, Intel Corp. All
> > rights reserved.",
> >> "Some of this work - Copyright (c) 2010 - 2016, Canonical.",
> >> + "Some of this work - Copyright (c) 2016 IBM.",
> >> NULL
> >> };
> >>
> >> @@ -1185,71 +1189,77 @@ int
> > fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
> >> case 9: /* --klog */
> >> fwts_framework_strdup(&fw->klog, optarg);
> >> break;
> >> - case 10: /* --log-width=N */
> >> + case 10: /* --olog */
> >> + fwts_framework_strdup(&fw->olog, optarg);
> >> + break;
> >> + case 11: /* --log-width=N */
> >> fwts_log_set_line_width(atoi(optarg));
> >> break;
> >> - case 11: /* --lspci=pathtolspci */
> >> + case 12: /* --lspci=pathtolspci */
> >> fwts_framework_strdup(&fw->lspci, optarg);
> >> break;
> >> - case 12: /* --batch */
> >> + case 13: /* --batch */
> >> fw->flags |= FWTS_FLAG_BATCH;
> >> break;
> >> - case 13: /* --interactive */
> >> + case 14: /* --interactive */
> >> fw->flags |= FWTS_FLAG_INTERACTIVE;
> >> break;
> >> - case 14: /* --force-clean */
> >> + case 15: /* --force-clean */
> >> fw->flags |= FWTS_FLAG_FORCE_CLEAN;
> >> break;
> >> - case 15: /* --version */
> >> + case 16: /* --version */
> >> fwts_framework_show_version(stdout, argv[0]);
> >> return FWTS_COMPLETE;
> >> - case 16: /* --dump */
> >> + case 17: /* --dump */
> >> fw->flags |= FWTS_FLAG_DUMP;
> >> break;
> >> - case 17: /* --table-path */
> >> + case 18: /* --table-path */
> >> fwts_framework_strdup(&fw->acpi_table_path, optarg);
> >> break;
> >> - case 18: /* --batch-experimental */
> >> + case 19: /* --batch-experimental */
> >> fw->flags |= FWTS_FLAG_BATCH_EXPERIMENTAL;
> >> break;
> >> - case 19: /* --interactive-experimental */
> >> + case 20: /* --interactive-experimental */
> >> fw->flags |= FWTS_FLAG_INTERACTIVE_EXPERIMENTAL;
> >> break;
> >> - case 20: /* --power-states */
> >> + case 21: /* --power-states */
> >> fw->flags |= FWTS_FLAG_POWER_STATES;
> >> break;
> >> - case 21: /* --all */
> >> + case 22: /* --all */
> >> fw->flags |= FWTS_FLAG_RUN_ALL;
> >> break;
> >> - case 22: /* --show-progress-dialog */
> >> + case 23: /* --show-progress-dialog */
> >> fw->flags = (fw->flags &
> >> ~(FWTS_FLAG_QUIET |
> >> FWTS_FLAG_SHOW_PROGRESS))
> >> | FWTS_FLAG_SHOW_PROGRESS_DIALOG;
> >> break;
> >> - case 23: /* --skip-test */
> >> + case 24: /* --skip-test */
> >> if (fwts_framework_skip_test_parse(optarg, &tests_to_skip) != FWTS_OK)
> >> return FWTS_COMPLETE;
> >> break;
> >> - case 24: /* --quiet */
> >> + case 25: /* --quiet */
> >> fw->flags = (fw->flags &
> >> ~(FWTS_FLAG_SHOW_PROGRESS |
> >> FWTS_FLAG_SHOW_PROGRESS_DIALOG))
> >> | FWTS_FLAG_QUIET;
> >> break;
> >> - case 25: /* --dumpfile */
> >> + case 26: /* --dumpfile */
> >> fwts_framework_strdup(&fw->acpi_table_acpidump_file, optarg);
> >> break;
> >> - case 26: /* --show-tests-full */
> >> + case 27: /* --show-tests-full */
> >> fw->flags |= FWTS_FLAG_SHOW_TESTS_FULL;
> >> break;
> >> - case 27: /* --utils */
> >> + case 28: /* --utils */
> >> fw->flags |= FWTS_FLAG_UTILS;
> >> break;
> >> - case 28: /* --json-data-path */
> >> + case 29: /* --json-data-path */
> >> fwts_framework_strdup(&fw->json_data_path, optarg);
> >> break;
> >> - case 29: /* --disassemble-aml */
> >> + case 30: /* --json-data-file */
> >> + fwts_framework_strdup(&fw->json_data_file, optarg);
> >> + break;
> >> + case 31: /* --disassemble-aml */
> >> #if defined(FWTS_HAS_ACPI)
> >> fwts_iasl_disassemble_all_to_file(fw, optarg);
> >> return FWTS_COMPLETE;
> >> @@ -1257,52 +1267,52 @@ int
> > fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
> >> fprintf(stderr, "option not available on this architecture\n");
> >> return FWTS_ERROR;
> >> #endif
> >> - case 30: /* --log-type */
> >> + case 32: /* --log-type */
> >> if (fwts_framework_log_type_parse(fw, optarg) != FWTS_OK)
> >> return FWTS_ERROR;
> >> break;
> >> - case 31: /* --unsafe */
> >> + case 33: /* --unsafe */
> >> fw->flags |= FWTS_FLAG_UNSAFE;
> >> break;
> >> - case 32: /* --filter-error-discard */
> >> + case 34: /* --filter-error-discard */
> >> if (fwts_framework_filter_error_parse(optarg,
> > &fw->errors_filter_discard) != FWTS_OK)
> >> return FWTS_ERROR;
> >> break;
> >> - case 33: /* --filter-error-keep */
> >> + case 35: /* --filter-error-keep */
> >> if (fwts_framework_filter_error_parse(optarg,
> > &fw->errors_filter_keep) != FWTS_OK)
> >> return FWTS_ERROR;
> >> break;
> >> - case 34: /* --acpica-debug */
> >> + case 36: /* --acpica-debug */
> >> fw->flags |= FWTS_FLAG_ACPICA_DEBUG;
> >> break;
> >> - case 35: /* --acpica */
> >> + case 37: /* --acpica */
> >> if (fwts_framework_acpica_parse(fw, optarg) != FWTS_OK)
> >> return FWTS_ERROR;
> >> break;
> >> - case 36: /* --uefitests */
> >> + case 38: /* --uefitests */
> >> fw->flags |= FWTS_FLAG_TEST_UEFI;
> >> break;
> >> - case 37: /* --rsdp */
> >> + case 39: /* --rsdp */
> >> fw->rsdp = (void *)strtoul(optarg, NULL, 0);
> >> break;
> >> - case 38: /* --pm-method */
> >> + case 40: /* --pm-method */
> >> if (fwts_framework_pm_method_parse(fw, optarg) != FWTS_OK)
> >> return FWTS_ERROR;
> >> break;
> >> - case 39: /* --show-tests-categories */
> >> + case 41: /* --show-tests-categories */
> >> fw->flags |= FWTS_FLAG_SHOW_TESTS_CATEGORIES;
> >> break;
> >> - case 40: /* --acpitests */
> >> + case 42: /* --acpitests */
> >> fw->flags |= FWTS_FLAG_TEST_ACPI;
> >> break;
> >> - case 41: /* --acpicompliance */
> >> + case 43: /* --acpicompliance */
> >> fw->flags |= FWTS_FLAG_TEST_COMPLIANCE_ACPI;
> >> break;
> >> - case 42: /* --log-level */
> >> + case 44: /* --log-level */
> >> if (fwts_framework_ll_parse(fw, optarg) != FWTS_OK)
> >> return FWTS_ERROR;
> >> break;
> >> - case 43: /* --arch */
> >> + case 45: /* --arch */
> >> if (fwts_framework_an_parse(fw, optarg) != FWTS_OK)
> >> return FWTS_ERROR;
> >> break;
> >> @@ -1336,11 +1346,17 @@ int
> > fwts_framework_options_handler(fwts_framework *fw, int argc, char * const ar
> >> case 'j': /* --json-data-path */
> >> fwts_framework_strdup(&fw->json_data_path, optarg);
> >> break;
> >> + case 'J': /* --json-data-file */
> >> + fwts_framework_strdup(&fw->json_data_file, optarg);
> >> + break;
> >> case 'k': /* --klog */
> >> fwts_framework_strdup(&fw->klog, optarg);
> >> break;
> >> case 'l': /* --lp-flags */
> >> break;
> >> + case 'o': /* --olog */
> >> + fwts_framework_strdup(&fw->olog, optarg);
> >> + break;
> >> case 'p': /* --show-progress */
> >> fw->flags = (fw->flags &
> >> ~(FWTS_FLAG_QUIET |
> >> @@ -1582,7 +1598,9 @@ tidy_close:
> >> free(fw->lspci);
> >> free(fw->results_logname);
> >> free(fw->klog);
> >> + free(fw->olog);
> >> free(fw->json_data_path);
> >> + free(fw->json_data_file);
> >>
> >> fwts_list_free_items(&fw->errors_filter_discard, NULL);
> >> fwts_list_free_items(&fw->errors_filter_keep, NULL);
> >> diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c
> >> index b5f0965..63918e7 100644
> >> --- a/src/lib/src/fwts_klog.c
> >> +++ b/src/lib/src/fwts_klog.c
> >> @@ -229,7 +229,7 @@ int fwts_klog_scan(fwts_framework *fw,
> >> return FWTS_OK;
> >> }
> >>
> >> -static char *fwts_klog_unique_label(const char *str)
> >> +char *fwts_klog_unique_label(const char *str)
> >> {
> >> static char buffer[1024];
> >> const char *src = str;
> >> @@ -325,7 +325,7 @@ void fwts_klog_scan_patterns(fwts_framework *fw,
> >> * fwts_klog_compare_mode_str_to_val()
> >> * convert compare mode strings (from json database) to compare_mode
> > values
> >> */
> >> -static fwts_compare_mode fwts_klog_compare_mode_str_to_val(const char
> > *str)
> >> +fwts_compare_mode fwts_klog_compare_mode_str_to_val(const char *str)
> >> {
> >> if (strcmp(str, "regex") == 0)
> >> return FWTS_COMPARE_REGEX;
> >> @@ -340,7 +340,7 @@ static fwts_compare_mode
> > fwts_klog_compare_mode_str_to_val(const char *str)
> >> * given a key, fetch the string value associated with this object
> >> * and report an error if it cannot be found.
> >> */
> >> -static const char *fwts_json_str(
> >> +const char *fwts_json_str(
> >> fwts_framework *fw,
> >> const char *table,
> >> int index,
> >> @@ -385,14 +385,19 @@ static int fwts_klog_check(fwts_framework *fw,
> >> fwts_klog_pattern *patterns;
> >> char json_data_path[PATH_MAX];
> >>
> >> - snprintf(json_data_path, sizeof(json_data_path), "%s/%s",
> > fw->json_data_path, KLOG_DATA_JSON_FILE);
> >> + if (fw->json_data_file) {
> >> + snprintf(json_data_path, sizeof(json_data_path), "%s/%s",
> > fw->json_data_path,(fw->json_data_file));
> >> + }
> >> + else { /* use the hard coded KLOG JSON as default */
> >> + snprintf(json_data_path, sizeof(json_data_path), "%s/%s",
> > fw->json_data_path, KLOG_DATA_JSON_FILE);
> >> + }
> >>
> >> /*
> >> * json_object_from_file() can fail when files aren't readable
> >> * so check if we can open for read before calling
> > json_object_from_file()
> >> */
> >> if ((fd = open(json_data_path, O_RDONLY)) < 0) {
> >> - fwts_log_error(fw, "Cannot read file %s.", json_data_path);
> >> + fwts_log_error(fw, "Cannot read file %s, check the path and check
> > that the file exists, you may need to specify -j or -J.", json_data_path);
> >> return FWTS_ERROR;
> >> }
> >> close(fd);
> >> diff --git a/src/lib/src/fwts_olog.c b/src/lib/src/fwts_olog.c
> >> new file mode 100644
> >> index 0000000..9f6c8bd
> >> --- /dev/null
> >> +++ b/src/lib/src/fwts_olog.c
> >> @@ -0,0 +1,222 @@
> >> +/*
> >> + * Copyright (C) 2010-2016 Canonical
> >> + * Some of this work - Copyright (C) 2016 IBM
> >> + *
> >> + * This program is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU General Public License
> >> + * as published by the Free Software Foundation; either version 2
> >> + * of the License, or (at your option) any later version.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program; if not, write to the Free Software
> >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301, USA.
> >> + *
> >> + */
> >> +
> >> +#include <sys/klog.h>
> >> +#include <string.h>
> >> +#include <stdlib.h>
> >> +#include <stdbool.h>
> >> +#include <sys/types.h>
> >> +#include <ctype.h>
> >> +#include <sys/types.h>
> >> +#include <sys/stat.h>
> >> +#include <regex.h>
> >> +#include <fcntl.h>
> >> +
> >> +#include "fwts.h"
> >> +
> >> +/*
> >> + * OLOG pattern matching strings data file, data stored in json format
> >> + */
> >> +#define OLOG_DATA_JSON_FILE "olog.json"
> >> +#define MSGLOG_BUFFER_LINE PATH_MAX
> >> +
> >> +/* SPECIAL CASE USE for OPEN POWER opal Firmware LOGS */
> >> +static const char msglog[] = "/sys/firmware/opal/msglog";
> >> +static const char msglog_outfile[] = "/var/log/opal_msglog";
> >> +
> >> +/*
> >> + * fwts_olog_read()
> >> + * read olog log and return as list of lines
> >> + */
> >> +fwts_list *fwts_olog_read(void)
> >> +{
> >> + size_t len;
> >> + fwts_list *list;
> >> + char cmdline[MSGLOG_BUFFER_LINE*2]; /* using two file paths in
> > system call for cat */
> >> + char *buffer;
> >> + struct stat filestat;
> >> + size_t actual=0;
> >> +
> >> +/* Check for the existance of the opal msglog and only if it exists
> > dump it out */
> >> +/* This makes the use of the OLOG as a custom option and not just for
> > PPC */
> >> +/* We don't use compiler flags since we want to run this as a custom
> > job cross platform */
> >> +
> >> + if(stat(msglog,&filestat)) /* stat fails so not PPC with OPAL msglog
> > and no -o OLOG sent */
> >> + return NULL;
> >
> > minor issue, the fwts coding style is "if (stat(msglog, &filestat))",
> >
> >>>debmc - Sorry, please elaborate, I thought that's what this says.
> >
> >> +
> >> + sprintf(cmdline, "cat %s >%s",msglog,msglog_outfile);
> >> + if (system(cmdline) != 0)
> >> + return NULL; /* return no list since the cat of the file failed */
> >
> > I know this is a pain, but we are trying to refrain from using system()
> > to call external code in fwts where possible, so can this be coded in C
> > rather than calling cat to do the work.
> >
> >>>debmc - This is the only methodology available since the contents of
> > the OPAL msglog has a file size of zero (internal kernel output) and
> > there are not any PPC system calls to perform the capability needed (so
> > there is no klogctl equivalence to perform the same capabilities on the
> > msglog that klog accomplishes with klogctl).
>
> I'm curious now. ;-)
> So cat /sys/firmware/opal/msglog can read this where as open()..
> read().. close() won't work? If that's the case, I wonder how cat can
> read it.
>
> >
> >> +
> >> + FILE* f = fopen(msglog_outfile, "r");
> >
> > and another minor issue is that fwts coding style is to have
> > declarations at the start of each block rather mixed in with executable
> > statements.
> >
> >>>debmc - Will fix.
> >
> >> +
> >> + if (!f)
> >> + return NULL; /* didn't get a good fopen */
> >> +
> >> + fseek(f,0,SEEK_END);
> >
> > we should always check to see if fseek() fails or not.
> >
> >>>debmc - Will fix.
> >
> >> + len = ftell(f);
> >
> > and check if ftell() fails, and should len be a long or a size_t?
> >
> >>>debmc - Will investigate best practice recommendation for use of
> > size_t versus long on PPC.
> >
> >> + fseek(f,0,SEEK_SET);
> >
> > ditto.
> >
> >>>debmc - Will fix.
> > < br> > +
> >> + if ((buffer = calloc(1, len+1)) == NULL)
> >> + return NULL;
> >> +
> >> + actual = fread(buffer,1,len,f);
> >
> > we also need to fclose(f) to fix a resource leak.
> >
> >>>debmc - Will fix.
> >
> >> + if (actual == len) {
> >> + list = fwts_list_from_text(buffer);
> >> + free(buffer);
> >> + return list;
> >> + }
> >> + else {
> >> + free(buffer);
> >> + return NULL;
> >> + }
> >> +}
> >> +
> >> +
> >> +static int fwts_olog_check(fwts_framework *fw,
> >> + const char *table,
> >> + fwts_olog_progress_func progress,
> >> + fwts_list *olog,
> >> + int *errors)
> >> +{
> >> + int ret = FWTS_ERROR;
> >> + int n;
> >> + int i;
> >> + int fd;
> >> + json_object *olog_objs;
> >> + json_object *olog_table;
> >> + fwts_klog_pattern *patterns;
> >> + char json_data_path[PATH_MAX];
> >> +
> >> +
> >> + if (fw->json_data_file) {
> >> + snprintf(json_data_path, sizeof(json_data_path), "%s/%s",
> > fw->json_data_path,(fw->json_data_file));
> >> + }
> >> + else { /* use the hard coded OLOG JSON as default */
> >> + snprintf(json_data_path, sizeof(json_data_path), "%s/%s",
> > fw->json_data_path, OLOG_DATA_JSON_FILE);
> >> + }
> >> +
> >> + /*
> >> + * json_object_from_file() can fail when files aren't readable
> >> + * so check if we can open for read before calling
> > json_object_from_file()
> >> + */
> >> + if ((fd = open(json_data_path, O_RDONLY)) < 0) {
> >> + fwts_log_error(fw, "Cannot read file %s, check the path and check
> > that the file exists, you may need to specify -j or -J.", json_data_path);
> >> + return FWTS_ERROR;
> >> + }
> >> + close(fd);
> >> +
> >> + olog_objs = json_object_from_file(json_data_path);
> >> + if (FWTS_JSON_ERROR(olog_objs)) {
> >> + fwts_log_error(fw, "Cannot load olog data from %s.", json_data_path);
> >> + return FWTS_ERROR;
> >> + }
> >> +
> >> +#if JSON_HAS_GET_EX
> >> + if (!json_object_object_get_ex(olog_objs, table, &olog_table)) {
> >> + fwts_log_error(fw, "Cannot fetch olog table object '%s' from %s.",
> > table, json_data_path);
> >> + goto fail_put;
> >> + }
> >> +#else
> >> + olog_table = json_object_object_get(olog_objs, table);
> >> + if (FWTS_JSON_ERROR(olog_table)) {
> >> + fwts_log_error(fw, "Cannot fetch olog table object '%s' from %s.",
> > table, json_data_path);
> >> + goto fail_put;
> >> + }
> >> +#endif
> >> +
> >> + n = json_object_array_length(olog_table);
> >> +
> >> + /* Last entry is null to indicate end, so alloc n+1 items */
> >> + if ((patterns = calloc(n+1, sizeof(fwts_klog_pattern))) == NULL) {
> >> + fwts_log_error(fw, "Cannot allocate pattern table.");
> >> + goto fail_put;
> >> + }
> >> +
> >> + /* Now fetch json objects and compile regex */
> >> + for (i = 0; i < n; i++) {
> >> + const char *str;
> >> + json_object *obj;
> >> +
> >> + obj = json_object_array_get_idx(olog_table, i);
> >> + if (FWTS_JSON_ERROR(obj)) {
> >> + fwts_log_error(fw, "Cannot fetch %d item from table %s.", i, table);
> >> + goto fail;
> >> + }
> >> + if ((str = fwts_json_str(fw, table, i, obj, "compare_mode", true))
> > == NULL)
> >> + goto fail;
> >> + patterns[i].compare_mode = fwts_klog_compare_mode_str_to_val(str);
> >> +
> >> + if ((str = fwts_json_str(fw, table, i, obj, "log_level", true)) == NULL)
> >> + goto fail;
> >> + patterns[i].level = fwts_log_str_to_level(str);
> >> +
> >> + if ((patterns[i].pattern = fwts_json_str(fw, table, i, obj,
> > "pattern", true)) == NULL)
> >> + goto fail;
> >> +
> >> + if ((patterns[i].advice = fwts_json_str(fw, table, i, obj, "advice",
> > true)) == NULL)
> >> + goto fail;
> >> +
> >> + /* Labels appear in fwts 0.26.0, so are optional with older versions */
> >> + str = fwts_json_str(fw, table, i, obj, "label", false);
> >> + if (str) {
> >> + patterns[i].label = strdup(str);
> >> + } else {
> >> + /* if not specified, auto-magically generate */
> >> + patterns[i].label = strdup(fwts_klog_unique_label(patterns[i].pattern));
> >> + }
> >> + if (patterns[i].label == NULL)
> >> + goto fail;
> >> +
> >> + if (patterns[i].compare_mode == FWTS_COMPARE_REGEX) {
> >> + int rc;
> >> +
> >> + rc = regcomp(&patterns[i].compiled, patterns[i].pattern, REG_EXTENDED);
> >> + if (rc) {
> >> + fwts_log_error(fw, "Regex %s failed to compile: %d.",
> > patterns[i].pattern, rc);
> >> + patterns[i].compiled_ok = false;
> >> + } else {
> >> + patterns[i].compiled_ok = true;
> >> + }
> >> + }
> >> + }
> >> + /* We've now collected up the scan patterns, lets scan the log for
> > errors */
> >> + ret = fwts_klog_scan(fw, olog, fwts_klog_scan_patterns, progress,
> > patterns, errors);
> >> +
> >> +fail:
> >> + for (i = 0; i < n; i++) {
> >> + if (patterns[i].compiled_ok)
> >> + regfree(&patterns[i].compiled);
> >> + if (patterns[i].label)
> >> + free(patterns[i].label);
> >> + }
> >> + free(patterns);
> >> +fail_put:
> >> + json_object_put(olog_objs);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +int fwts_olog_firmware_check(fwts_framework *fw,
> > fwts_olog_progress_func progress,
> >> + fwts_list *olog, int *errors)
> >> +{
> >> + return fwts_olog_check(fw, "olog_error_warning_patterns",
> >> + progress, olog, errors);
> >> +}
> >> diff --git a/src/lib/src/fwts_stringextras.c
> > b/src/lib/src/fwts_stringextras.c
> >> index 17a742c..17d833e 100644
> >> --- a/src/lib/src/fwts_stringextras.c
> >> +++ b/src/lib/src/fwts_stringextras.c
> >> @@ -42,6 +42,10 @@ void fwts_chop_newline(char *str)
> >>
> >> while (len > 0 && str[len-1] == ' ')
> >> str[--len] = '\0';
> >> +
> >> + while (len > 0 && str[len-1] == '\r')
> >> + str[--len] = '\0';
> >> +
> >> }
> >>
> >> /*
> >>
> >
> >
> > --
> > fwts-devel mailing list
> > fwts-devel at lists.ubuntu.com
> > Modify settings or unsubscribe at:
> > https://lists.ubuntu.com/mailman/listinfo/fwts-devel
> >
> >
> >
> >
>
>
--
Deb McLemore
IBM OpenPower - IBM Systems
(512) 286 9980
debmc at us.ibm.com
ipowermc at icloud.com
debmc at linux.vnet.ibm.com - (plain text)
ipowermc at linux.vnet.ibm.com - (plain text)
More information about the fwts-devel
mailing list