From 473ed1c34b98b0515031eef9bfd3a140d8bb2c92 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 16 Nov 2009 17:04:55 +0100 Subject: add experimental saving of /var/log/Xorg*.log for X crashes Signed-off-by: Denys Vlasenko --- doc/DESIGN | 15 +++++++---- doc/PLUGINS-HOWTO | 59 +++++++++++++++++++++++++------------------- lib/Plugins/RunApp.cpp | 32 ++++++++++++++++++------ lib/Utils/DebugDump.cpp | 8 +++--- src/Daemon/MiddleWare.cpp | 23 +++++++++++------ src/Daemon/PluginManager.cpp | 27 +++++++------------- src/Daemon/Settings.cpp | 1 + src/Daemon/abrt.conf | 7 +++--- 8 files changed, 102 insertions(+), 70 deletions(-) diff --git a/doc/DESIGN b/doc/DESIGN index 78947238..7298b4c8 100644 --- a/doc/DESIGN +++ b/doc/DESIGN @@ -107,14 +107,19 @@ The key dbus calls served by abrt-process are: Development plan Since current code does not match the planned design, we need to gradually -change the code to "morph" it into the desired shape. Planned steps: +change the code to "morph" it into the desired shape. -* make kerneloops plugin into separate daemon (convert it to a hook - and get rid of "cron plugins" which are wrong idea since the begining) -* Make abrtd dbus startable +Done: + +* Make abrtd dbus startable. +* Add -t TIMEOUT_SEC option to abrtd. {done} * Make abrt-gui start abrtd on demand, so that abrt-gui can be started even if abrtd does not run at the moment. -* Add -t TIMEOUT_SEC option to abrtd. + +Planned steps: + +* make kerneloops plugin into separate daemon (convert it to a hook + and get rid of "cron plugins" which are wrong idea since the begining) * ??? * ??? * ??? diff --git a/doc/PLUGINS-HOWTO b/doc/PLUGINS-HOWTO index ef0c6ba6..d8467801 100644 --- a/doc/PLUGINS-HOWTO +++ b/doc/PLUGINS-HOWTO @@ -18,57 +18,64 @@ The files that were created when the application crashed (such as core dumps) are all stored here. In addition, the plugins can write their output files there, if any. -Let's discuss the plugin types in detail: +Plugin types in detail: + Action Plugin ------------- -You use this type of plugin when you need some action to be -performed when a crash is encountered. -you have to override one method: +This type of plugin is useful when you need some action to be performed +immediately when a crash is encountered (using ActionsAndReporters = ... +directive in abrt.conf), or if you need some action to be performed +periodically ([Cron] section in the same file). + +You have to override one method: + +virtual void Run(const char *dir, const char *args); + +The first argument is a directory that contains the current debug +dump, or all debug dumps for periodic actions. +The second argument is a string with arguments (specified in config file). -virtual void Run(const std::string& pActiveDir, - const std::string& pArgs) = 0; --This method runs the specified action. - The first argument is a directory name. It can be either the current debug - dump dir or a directory that contains all debug dumps. - The second argument is a string with arguments specified for the action. Analyzer Plugin --------------- This plugin has to compute the UUID of the crash. Crashes differ, depending on where they occur, for example crashes in the kernel differ from crashes in userspace binaries, which differ from crashes in python scripts. Therefore, -you need a plugin for each type of application that you want "abrt" to handle. +you need a plugin for each type of application that you want abrt to handle. -you have to override these methods: +You have to override these methods: -virtual std::string GetLocalUUID(const std::string& pDebugDumpPath) = 0; +virtual std::string GetLocalUUID(const std::string& pDebugDumpPath); - This method computes the local UUID of the crash. -virtual std::string GetGlobalUUID(const std::string& pDebugDumpPath) = 0; +virtual std::string GetGlobalUUID(const std::string& pDebugDumpPath); - This method computes the global UUID of the crash. -NOTE:The difference between local and global UUID is that the local UUID +{something is fishy here} + +NOTE: The difference between local and global UUID is that the local UUID is specific for the machine architecture on which the crash is encountered. When the crash is reported, abrt has to use the "-debuginfo" packages to render a global UUID, which should be independent of the specific system (the same crash on different architectures/configurations can yield different local UUIDs but has to have the same global UUID). -virtual void CreateReport(const std::string& pDebugDumpPath) = 0; +virtual void CreateReport(const std::string& pDebugDumpPath); - This method creates the report about the crash and stores it in the crash's directory. + Reporter Plugin --------------- This plugin receives the entire finished crash report and posts/reports it somewhere (e.g. logs it, mails it, posts it on some web tool...) -you have to override this method: +You have to override this method: virtual void Report(const crash_report_t& pCrashReport, - const std::string& pArgs) = 0; + const std::string& pArgs); -It is self-explanatory, that this method takes the report and presents it somewhere to the world. The second argument is a string with arguments specified for the reporter. @@ -82,30 +89,30 @@ is not the same as some crash before. The database can be local, or in some centralized location on the network. you have to override these methods: -virtual void Connect() = 0; -virtual void DisConnect() = 0; +virtual void Connect(); +virtual void DisConnect(); - connect and disconnect from the database virtual void Insert(const std::string& pUUID, const std::string& pUID, const std::string& pDebugDumpPath, - const std::string& pTime) = 0; + const std::string& pTime); - insert an entry into the database: you use both UID (user ID) and UUID (ID of the crash) -virtual void Delete(const std::string& pUUID, const std::string& pUID) = 0; +virtual void Delete(const std::string& pUUID, const std::string& pUID); - delete an entry -virtual void SetReported(const std::string& pUUID, const std::string& pUID) = 0; +virtual void SetReported(const std::string& pUUID, const std::string& pUID); - insert information into the database to say that this bug was already reported (so for example the report plugins won't run several times for the same bug) -virtual const vector_database_rows_t GetUIDData(const std::string& pUID) = 0; +virtual const vector_database_rows_t GetUIDData(const std::string& pUID); - get database rows for the specified user ID virtual const database_row_t GetUUIDData(const std::string& pUUID, const - std::string& pUID) = 0; + std::string& pUID); - get a database row for the specified user ID and UUID @@ -124,7 +131,7 @@ PLUGIN_INFO(type, plugin_class, name, version, description, email, www) - "description" is a string with the summary of what the plugin does - "email" and "www" are strings with the contact info for the author -for example: +For example: PLUGIN_INFO(REPORTER, CMailx, "Mailx", diff --git a/lib/Plugins/RunApp.cpp b/lib/Plugins/RunApp.cpp index 3be7bdf4..a2aa6987 100644 --- a/lib/Plugins/RunApp.cpp +++ b/lib/Plugins/RunApp.cpp @@ -21,7 +21,6 @@ #include "RunApp.h" -#include #include "DebugDump.h" #include "ABRTException.h" #include "CommLayerInner.h" @@ -30,20 +29,39 @@ #define COMMAND 0 #define FILENAME 1 +using namespace std; + void CActionRunApp::Run(const char *pActionDir, const char *pArgs) { - update_client(_("Executing RunApp plugin...")); + /* Don't update_client() - actions run at crash time */ + log("RunApp('%s','%s')", pActionDir, pArgs); - std::string output; vector_string_t args; - parse_args(pArgs, args, '"'); - FILE *fp = popen(args[COMMAND].c_str(), "r"); + const char *cmd = args[COMMAND].c_str(); + if (!cmd[0]) + { + return; + } + +//FIXME: need to be able to escape " in .conf + /* Chdir to the dump dir. Command can analyze component and such. + * Example: + * test x"`cat component`" = x"xorg-x11-apps" && cp /var/log/Xorg.0.log . + */ +//Can do it using chdir() in child if we'd open-code popen + string cd_and_cmd = ssprintf("cd '%s'; %s", pActionDir, cmd); + FILE *fp = popen(cd_and_cmd.c_str(), "r"); if (fp == NULL) { - throw CABRTException(EXCEP_PLUGIN, "Can't execute " + args[COMMAND]); + /* Happens only on resource starvation (fork fails or out-of-mem) */ + return; } + +//FIXME: RunApp("gzip -9 1) + if (args.size() > FILENAME) { CDebugDump dd; dd.Open(pActionDir); diff --git a/lib/Utils/DebugDump.cpp b/lib/Utils/DebugDump.cpp index ba11d965..2883d01f 100644 --- a/lib/Utils/DebugDump.cpp +++ b/lib/Utils/DebugDump.cpp @@ -74,7 +74,7 @@ void CDebugDump::Open(const char *pDir) bool CDebugDump::Exist(const char* pPath) { - std::string fullPath = m_sDebugDumpDir + "/" + pPath; + std::string fullPath = concat_path_file(m_sDebugDumpDir.c_str(), pPath); return ExistFileDir(fullPath.c_str()); } @@ -433,7 +433,7 @@ void CDebugDump::LoadText(const char* pName, std::string& pData) { throw CABRTException(EXCEP_DD_OPEN, "CDebugDump::LoadText(): DebugDump is not opened."); } - std::string fullPath = m_sDebugDumpDir + '/' + pName; + std::string fullPath = concat_path_file(m_sDebugDumpDir.c_str(), pName); LoadTextFile(fullPath.c_str(), pData); } @@ -443,7 +443,7 @@ void CDebugDump::SaveText(const char* pName, const char* pData) { throw CABRTException(EXCEP_DD_OPEN, "CDebugDump::SaveText(): DebugDump is not opened."); } - std::string fullPath = m_sDebugDumpDir + "/" + pName; + std::string fullPath = concat_path_file(m_sDebugDumpDir.c_str(), pName); SaveBinaryFile(fullPath.c_str(), pData, strlen(pData)); } void CDebugDump::SaveBinary(const char* pName, const char* pData, unsigned pSize) @@ -452,7 +452,7 @@ void CDebugDump::SaveBinary(const char* pName, const char* pData, unsigned pSize { throw CABRTException(EXCEP_DD_OPEN, "CDebugDump::SaveBinary(): DebugDump is not opened."); } - std::string fullPath = m_sDebugDumpDir + "/" + pName; + std::string fullPath = concat_path_file(m_sDebugDumpDir.c_str(), pName); SaveBinaryFile(fullPath.c_str(), pData, pSize); } diff --git a/src/Daemon/MiddleWare.cpp b/src/Daemon/MiddleWare.cpp index 0bc358e1..dc5a295a 100644 --- a/src/Daemon/MiddleWare.cpp +++ b/src/Daemon/MiddleWare.cpp @@ -278,17 +278,21 @@ void RunActionsAndReporters(const char *pDebugDumpDir) { try { - if (g_pPluginManager->GetPluginType(it_ar->first) == REPORTER) + VERB3 log("RunActionsAndReporters: checking %s", it_ar->first.c_str()); + plugin_type_t tp = g_pPluginManager->GetPluginType(it_ar->first); + if (tp == REPORTER) { CReporter* reporter = g_pPluginManager->GetReporter(it_ar->first); map_crash_report_t crashReport; DebugDumpToCrashReport(pDebugDumpDir, crashReport); + VERB2 log("%s.Report(...)", it_ar->first.c_str()); reporter->Report(crashReport, plugin_settings, it_ar->second); } - else if (g_pPluginManager->GetPluginType(it_ar->first) == ACTION) + else if (tp == ACTION) { CAction* action = g_pPluginManager->GetAction(it_ar->first); + VERB2 log("%s.Run('%s','%s')", it_ar->first.c_str(), pDebugDumpDir, it_ar->second.c_str()); action->Run(pDebugDumpDir, it_ar->second.c_str()); } } @@ -375,18 +379,22 @@ report_status_t Report(const map_crash_report_t& pCrashReport, } } - // analyzer with package name (CCpp:xrog-x11-app) has higher priority + // analyzer with package name (CCpp:xorg-x11-app) has higher priority std::string key = analyzer + ":" + packageName; + map_analyzer_actions_and_reporters_t::iterator end = s_mapAnalyzerActionsAndReporters.end(); map_analyzer_actions_and_reporters_t::iterator keyPtr = s_mapAnalyzerActionsAndReporters.find(key); - if (keyPtr == s_mapAnalyzerActionsAndReporters.end()) + if (keyPtr == end) { // if there is no such settings, then try default analyzer keyPtr = s_mapAnalyzerActionsAndReporters.find(analyzer); + key = analyzer; } std::string message; - if (keyPtr != s_mapAnalyzerActionsAndReporters.end()) + if (keyPtr != end) { + VERB2 log("Found AnalyzerActionsAndReporters for '%s'", key.c_str()); + vector_pair_string_string_t::iterator it_r = keyPtr->second.begin(); for (; it_r != keyPtr->second.end(); it_r++) { @@ -433,7 +441,7 @@ report_status_t Report(const map_crash_report_t& pCrashReport, { ret[pluginName].push_back("0"); ret[pluginName].push_back(e.what()); - update_client("Reporting via %s' was not successful: %s", pluginName.c_str(), e.what()); + update_client("Reporting via '%s' was not successful: %s", pluginName.c_str(), e.what()); } } } @@ -764,7 +772,7 @@ vector_pair_string_string_t GetUUIDsOfCrash(const char *pUID) database->DisConnect(); vector_pair_string_string_t UUIDsUIDs; - int ii; + unsigned ii; for (ii = 0; ii < rows.size(); ii++) { UUIDsUIDs.push_back(make_pair(rows[ii].m_sUUID, rows[ii].m_sUID)); @@ -783,5 +791,6 @@ void AddAnalyzerActionOrReporter(const char *pAnalyzer, void AddActionOrReporter(const char *pActionOrReporter, const char *pArgs) { + VERB3 log("AddActionOrReporter('%s','%s')", pActionOrReporter, pArgs); s_vectorActionsAndReporters.push_back(make_pair(std::string(pActionOrReporter), std::string(pArgs))); } diff --git a/src/Daemon/PluginManager.cpp b/src/Daemon/PluginManager.cpp index 5b871515..21a66adf 100644 --- a/src/Daemon/PluginManager.cpp +++ b/src/Daemon/PluginManager.cpp @@ -278,13 +278,11 @@ CAnalyzer* CPluginManager::GetAnalyzer(const std::string& pName) map_plugins_t::iterator plugin = m_mapPlugins.find(pName); if (plugin == m_mapPlugins.end()) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetAnalyzer():" - "Analyzer plugin: '"+pName+"' is not registered."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered"); } if (m_mapABRTPlugins[pName]->GetType() != ANALYZER) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetAnalyzer():" - "Plugin: '"+pName+"' is not analyzer plugin."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not an analyzer plugin"); } return (CAnalyzer*)(plugin->second); } @@ -294,13 +292,11 @@ CReporter* CPluginManager::GetReporter(const std::string& pName) map_plugins_t::iterator plugin = m_mapPlugins.find(pName); if (plugin == m_mapPlugins.end()) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetReporter():" - "Reporter plugin: '"+pName+"' is not registered."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered"); } if (m_mapABRTPlugins[pName]->GetType() != REPORTER) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetReporter():" - "Plugin: '"+pName+"' is not reporter plugin."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not a reporter plugin"); } return (CReporter*)(plugin->second); } @@ -310,13 +306,11 @@ CAction* CPluginManager::GetAction(const std::string& pName) map_plugins_t::iterator plugin = m_mapPlugins.find(pName); if (plugin == m_mapPlugins.end()) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetAction():" - "Action plugin: '"+pName+"' is not registered."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered"); } if (m_mapABRTPlugins[pName]->GetType() != ACTION) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetAction():" - "Plugin: '"+pName+"' is not action plugin."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not an action plugin"); } return (CAction*)(plugin->second); } @@ -326,13 +320,11 @@ CDatabase* CPluginManager::GetDatabase(const std::string& pName) map_plugins_t::iterator plugin = m_mapPlugins.find(pName); if (plugin == m_mapPlugins.end()) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetDatabase():" - "Database plugin: '"+pName+"' is not registered."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered"); } if (m_mapABRTPlugins[pName]->GetType() != DATABASE) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetDatabase():" - "Plugin: '"+pName+"' is not database plugin."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not a database plugin"); } return (CDatabase*)(plugin->second); } @@ -342,8 +334,7 @@ plugin_type_t CPluginManager::GetPluginType(const std::string& pName) map_plugins_t::iterator plugin = m_mapPlugins.find(pName); if (plugin == m_mapPlugins.end()) { - throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetPluginType():" - "Plugin: '"+pName+"' is not registered."); + throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered"); } return m_mapABRTPlugins[pName]->GetType(); } diff --git a/src/Daemon/Settings.cpp b/src/Daemon/Settings.cpp index cefd35a4..7f419ec5 100644 --- a/src/Daemon/Settings.cpp +++ b/src/Daemon/Settings.cpp @@ -228,6 +228,7 @@ static void ParseAnalyzerActionsAndReporters() set_string_t::iterator it_keys = keys.begin(); for (; it_keys != keys.end(); it_keys++) { + VERB2 log("AnalyzerActionsAndReporters['%s']=...", it_keys->c_str()); g_settings_mapAnalyzerActionsAndReporters[*it_keys] = actionsAndReporters; } } diff --git a/src/Daemon/abrt.conf b/src/Daemon/abrt.conf index e9845e58..f24ec4a8 100644 --- a/src/Daemon/abrt.conf +++ b/src/Daemon/abrt.conf @@ -10,20 +10,21 @@ OpenGPGPublicKeys = /etc/pki/rpm-gpg/RPM-GPG-KEY-fedora # Blacklisted packages BlackList = nspluginwrapper # Enabled plugins. There has to be exactly one database plugin -EnabledPlugins = SQLite3, CCpp, Logger, Kerneloops, KerneloopsScanner, KerneloopsReporter, Bugzilla, Python +EnabledPlugins = SQLite3, CCpp, Logger, Kerneloops, KerneloopsScanner, KerneloopsReporter, Bugzilla, Python, RunApp # Database Database = SQLite3 # Max size for crash storage [MiB] MaxCrashReportsSize = 1000 # Vector of actions and reporters which are activated immediately after a crash occurs -# ActionsAndReporters = Mailx("[abrt] new crash was detected") +#ActionsAndReporters = Mailx("[abrt] new crash was detected") +# TODO: teach parser to escape \". Should be: x"`cat component`" = x"xorg-x11-server-Xorg" +ActionsAndReporters = RunApp("test x`cat component` = xxorg-x11-server-Xorg && cp /var/log/Xorg*.log .") # Reporters association with analyzers [ AnalyzerActionsAndReporters ] Kerneloops = KerneloopsReporter CCpp = Bugzilla, Logger Python = Bugzilla, Logger -# CCpp : xorg-x11-apps = RunApp("date", "RunApp") # Repeated calling of Action plugins [ Cron ] -- cgit