From 193f2898d9bf3c7f971d2e37a846b61857e7eb77 Mon Sep 17 00:00:00 2001 From: Jakub Filak Date: Tue, 5 Apr 2016 15:09:39 +0200 Subject: [PATCH] Merge a-a-save-kernel-data with a-a-save-package-data I forgot to do this while working on commit 650822d0d2438825515c0e41f06cd9fb77a18334 I do not think creating another tool doing almost the same thing was a good idea. I believe we must keep relevant bits together and not to spread functionality among myriads of tools just because someone do not want to code in C. I failed to find any justification for creating 'abrt-action-save-kernel-data' instead of teaching 'abrt-action-save-package-data' to do that job: 130dee46d601f3af6bb196d99a5c911335506adf This commit changes contents of 'package' file from 'kernel' to 'kernel-$version' string. It should not break anything as I could not find any justification for the 'kernel' string except the original author's believe it should be like that (probably because of efficiency). Just for the record, we do not need the version in 'package' file because the version is already included in 'kernel' file. This commit enables GPGCheck and ProcessUnpackaged options for Kerneloopses and VMcores. Signed-off-by: Jakub Filak --- doc/Makefile.am | 1 - doc/abrt-action-save-kernel-data.txt | 33 ------------- src/daemon/abrt-action-save-package-data.c | 63 ++++++++++++++++-------- src/plugins/Makefile.am | 2 - src/plugins/abrt-action-save-kernel-data | 78 ------------------------------ src/plugins/koops_event.conf | 3 +- src/plugins/vmcore_event.conf | 2 +- 7 files changed, 46 insertions(+), 136 deletions(-) delete mode 100644 doc/abrt-action-save-kernel-data.txt delete mode 100755 src/plugins/abrt-action-save-kernel-data diff --git a/doc/Makefile.am b/doc/Makefile.am index d3184c4..4a5d94c 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -32,7 +32,6 @@ MAN1_TXT += abrt-merge-pstoreoops.txt MAN1_TXT += abrt-server.txt MAN1_TXT += abrt-cli.txt MAN1_TXT += abrt-action-save-package-data.txt -MAN1_TXT += abrt-action-save-kernel-data.txt MAN1_TXT += abrt-install-ccpp-hook.txt MAN1_TXT += abrt-action-analyze-ccpp-local.txt MAN1_TXT += abrt-watch-log.txt diff --git a/doc/abrt-action-save-kernel-data.txt b/doc/abrt-action-save-kernel-data.txt deleted file mode 100644 index f82fa35..0000000 --- a/doc/abrt-action-save-kernel-data.txt +++ /dev/null @@ -1,33 +0,0 @@ -abrt-action-save-kernel-data(1) -================================ - -NAME ----- -abrt-action-save-kernel-data - Creates uReport mandatory files for kernel oopses. - -SYNOPSIS --------- -'abrt-action-save-kernel-data' - -DESCRIPTION ------------ -The tool reads problem directory DIR. It analyzes contents of 'kernel' element, -checks database of installed packages, and creates new elements -'pkg_name', 'pkg_arch', 'pkg_version', 'pkg_release'. - -These files are required by reporter-ureporter (mandatory in uReport). - -Integration with ABRT events -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -This tool can be used as an ABRT reporter. Example -fragment for /etc/libreport/report_event.conf: - ------------- -# Determine in which package/component the crash happened (if not yet done): -EVENT=post-create analyzer=Kerneloops - abrt-action-save-kernel-data ------------- - -AUTHORS -------- -* ABRT team diff --git a/src/daemon/abrt-action-save-package-data.c b/src/daemon/abrt-action-save-package-data.c index 72c9878..6f8c80d 100644 --- a/src/daemon/abrt-action-save-package-data.c +++ b/src/daemon/abrt-action-save-package-data.c @@ -217,14 +217,7 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name, const ch return 1; char *type = dd_load_text(dd, FILENAME_TYPE); - if (!strcmp(type, "Kerneloops")) - { - dd_save_text(dd, FILENAME_PACKAGE, "kernel"); - dd_save_text(dd, FILENAME_COMPONENT, "kernel"); - dd_close(dd); - free(type); - return 0; - } + bool kernel_oops = !strcmp(type, "Kerneloops") || !strcmp(type, "vmcore"); free(type); char *cmdline = NULL; @@ -233,12 +226,32 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name, const ch char *package_short_name = NULL; struct pkg_envra *pkg_name = NULL; char *component = NULL; + char *kernel = NULL; int error = 1; /* note: "goto ret" statements below free all the above variables, * but they don't dd_close(dd) */ - cmdline = dd_load_text_ext(dd, FILENAME_CMDLINE, DD_FAIL_QUIETLY_ENOENT); - executable = dd_load_text(dd, FILENAME_EXECUTABLE); + if (kernel_oops) + { + kernel = dd_load_text(dd, FILENAME_KERNEL); + if (!kernel) + { + log("File 'kernel' containing kernel version not " + "found in current directory"); + goto ret; + } + /* Trim trailing white-spaces. */ + strchrnul(kernel, ' ')[0] = '\0'; + + log_info("Looking for kernel package"); + executable = xasprintf("/boot/vmlinuz-%s", kernel); + } + else + { + cmdline = dd_load_text_ext(dd, FILENAME_CMDLINE, DD_FAIL_QUIETLY_ENOENT); + executable = dd_load_text(dd, FILENAME_EXECUTABLE); + } + /* Do not implicitly query rpm database in process's root dir, if * ExploreChroots is disabled. */ @@ -249,8 +262,12 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name, const ch /* Close dd while we query package database. It can take some time, * don't want to keep dd locked longer than necessary */ dd_close(dd); + dd = NULL; - if (is_path_blacklisted(executable)) + /* The check for kernel_oops is there because it could be an unexpected + * behaviour. If one wants to ignore kernel oops, she/he should disable + * the corresponding services. */ + if (!kernel_oops && is_path_blacklisted(executable)) { log("Blacklisted executable '%s'", executable); goto ret; /* return 1 (failure) */ @@ -265,13 +282,17 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name, const ch "proceeding without packaging information", executable); goto ret0; /* no error */ } - log("Executable '%s' doesn't belong to any package" - " and ProcessUnpackaged is set to 'no'", - executable - ); + if (kernel_oops) + log("Can't find kernel package corresponding to '%s'", kernel); + else + log("Executable '%s' doesn't belong to any package" + " and ProcessUnpackaged is set to 'no'", executable); goto ret; /* return 1 (failure) */ } + if (kernel_oops) + goto skip_interperter; + /* Check well-known interpreter names */ const char *basename = strrchr(executable, '/'); if (basename) @@ -314,11 +335,14 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name, const ch pkg_name = script_pkg; } +skip_interperter: package_short_name = xasprintf("%s", pkg_name->p_name); log_info("Package:'%s' short:'%s'", pkg_name->p_nvr, package_short_name); - - if (g_list_find_custom(settings_setBlackListedPkgs, package_short_name, (GCompareFunc)g_strcmp0)) + /* The check for kernel_oops is there because it could be an unexpected + * behaviour. If one wants to ignore kernel oops, she/he should disable + * the corresponding services. */ + if (!kernel_oops && g_list_find_custom(settings_setBlackListedPkgs, package_short_name, (GCompareFunc)g_strcmp0)) { log("Blacklisted package '%s'", package_short_name); goto ret; /* return 1 (failure) */ @@ -358,11 +382,12 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name, const ch if (component) dd_save_text(dd, FILENAME_COMPONENT, component); - dd_close(dd); - ret0: error = 0; ret: + if (dd) + dd_close(dd); + free(cmdline); free(executable); free(rootdir); diff --git a/src/plugins/Makefile.am b/src/plugins/Makefile.am index 005cc9d..2c9028b 100644 --- a/src/plugins/Makefile.am +++ b/src/plugins/Makefile.am @@ -6,7 +6,6 @@ bin_SCRIPTS = \ abrt-action-analyze-vulnerability \ abrt-action-list-dsos \ abrt-action-perform-ccpp-analysis \ - abrt-action-save-kernel-data \ abrt-action-analyze-ccpp-local \ abrt-action-notify @@ -101,7 +100,6 @@ EXTRA_DIST = \ analyze_RetraceServer.xml.in \ abrt-action-analyze-core.in \ abrt-action-generate-machine-id \ - abrt-action-save-kernel-data \ abrt-action-ureport \ abrt-gdb-exploitable \ https-utils.h \ diff --git a/src/plugins/abrt-action-save-kernel-data b/src/plugins/abrt-action-save-kernel-data deleted file mode 100755 index f8b18f0..0000000 --- a/src/plugins/abrt-action-save-kernel-data +++ /dev/null @@ -1,78 +0,0 @@ -#!/bin/bash -# -# Save pkg_{name, arch, version, release} for kernel oopses. -# -# These files are required by reporter-ureporter (mandatory -# in uReport). -# - -function print_usage -{ - echo "Usage: abrt-action-save-package-data [OPTION]" - echo "" - echo " -r, --root ROOT use ROOT as top level directory" - echo " -h, --help Show this help message" -} - -ROOT="/" - -while [ $# -gt 0 ]; -do - case "$1" in - "-r"|"--root") - if [ -z "$2" ]; then - >&2 - echo "$1: requires argument" - print_usage - exit 1 - fi - - ROOT=$2 - shift - ;; - - "-h"|"--help") - echo "Save pkg_{name, arch, version, release} for kernel oopses." - echo "" - echo "These files are required by reporter-ureporter (mandatory" - echo "in uReport)." - - print_usage - exit 0 - ;; - - *) - >&2 - echo "$1: unknown option" - echo - print_usage - exit 1 - ;; - esac - - shift -done - -if [ ! -f kernel ]; then - echo "File 'kernel' containing kernel version not found in current directory" - exit 1 -fi - -echo "Looking for kernel package" -kernel_version="$( sed 's/ .*//' kernel )" - -package="$( rpm --root $ROOT -qf "/boot/vmlinuz-$kernel_version" )" -if [ $? != 0 ]; then - echo "Can't find kernel package corresponding to '$kernel_version'" - echo "Can't record package version data (pkg_version, pkg_release, ...)." - exit 1 -fi - -echo "Kernel package $package found" -rpm --root $ROOT -q --qf "%{name}\n" "$package" > pkg_name -rpm --root $ROOT -q --qf "%{arch}\n" "$package" > pkg_arch -rpm --root $ROOT -q --qf "%{version}\n" "$package" > pkg_version -rpm --root $ROOT -q --qf "%{release}\n" "$package" > pkg_release -epoch="$( rpm --root $ROOT -q --qf "%{epoch}" "$package" )" -test "$epoch" = "(none)" && epoch=0 -echo "$epoch" > pkg_epoch diff --git a/src/plugins/koops_event.conf b/src/plugins/koops_event.conf index df7e446..f273ba2 100644 --- a/src/plugins/koops_event.conf +++ b/src/plugins/koops_event.conf @@ -16,8 +16,7 @@ EVENT=post-create type=Kerneloops remote!=1 if test ! -f uuid -a ! -f duphash; then abrt-action-analyze-oops || exit 1 fi - } && - abrt-action-save-kernel-data + } # If you want behavior similar to one provided by kerneloops daemon diff --git a/src/plugins/vmcore_event.conf b/src/plugins/vmcore_event.conf index ae4dc9f..61bc9d1 100644 --- a/src/plugins/vmcore_event.conf +++ b/src/plugins/vmcore_event.conf @@ -34,7 +34,7 @@ EVENT=post-create type=vmcore remote!=1 # analyze EVENT=analyze_VMcore type=vmcore abrt-action-analyze-oops && - abrt-action-save-kernel-data + abrt-action-save-package-data # If you want behavior similar to one provided by kerneloops daemon # distributed by kerneloops.org - that is, if you want -- 2.7.4