Blob Blame History Raw
From 193f2898d9bf3c7f971d2e37a846b61857e7eb77 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
 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