[PATCH] Introduce olog scan, to check OPAL msglog.

Deborah McLemore debmc at us.ibm.com
Tue Mar 22 16:59:45 UTC 2016


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)
=====================================



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.

>
>  		 		 		 /*  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).

> +
> +		 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.

> +
> +		 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



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20160322/fe26707d/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20160322/fe26707d/attachment-0001.gif>


More information about the fwts-devel mailing list