All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/8] Various modifications and fixes toward KS 1.0
@ 2019-04-19 13:50 Yordan Karadzhov
  2019-04-19 13:50 ` [PATCH v3 1/8] kernel-shark: Configuration information in ${HOME}/.cache/kernelshark Yordan Karadzhov
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov

The patch-set contains reimplementation of patches that have been sent
already, but have been found to have various problems. The last two
patch are fixes of bugs that have been found recently.

Changes in v3:
 - Corrections from Slavomir (in patches 1/8 and 3/8)

Yordan Karadzhov (8):
  kernel-shark: Configuration information in ${HOME}/.cache/kernelshark
  kernel-shark: Remove the definition of KS_CONF_DIR
  kernel-shark: Add logic for the initial path of Open-File dialogs
  kernel-shark: Add logic for the plugins search path
  kernel-shark: Rename KS_DIR to KS_SOURCE_DIR
  kernel-shark: Load Last Session from command line
  kernel-shark: Use proper searching condition when the dataset is small
  kernel-shark: Handle the case when the marker points to a filtered
    entry

 kernel-shark/CMakeLists.txt             | 37 +++++-------
 kernel-shark/build/deff.h.cmake         |  6 +-
 kernel-shark/src/CMakeLists.txt         | 10 ++--
 kernel-shark/src/KsCaptureDialog.cpp    |  6 +-
 kernel-shark/src/KsMainWindow.cpp       | 80 ++++++++++++++++++++-----
 kernel-shark/src/KsMainWindow.hpp       |  4 ++
 kernel-shark/src/KsTraceViewer.cpp      | 32 ++++++----
 kernel-shark/src/KsTraceViewer.hpp      |  2 +
 kernel-shark/src/KsUtils.cpp            | 35 +++++++----
 kernel-shark/src/KsUtils.hpp            | 16 +++++
 kernel-shark/src/kernelshark.cpp        |  9 ++-
 kernel-shark/src/plugins/CMakeLists.txt |  2 +-
 12 files changed, 168 insertions(+), 71 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/8] kernel-shark: Configuration information in ${HOME}/.cache/kernelshark
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
@ 2019-04-19 13:50 ` Yordan Karadzhov
  2019-04-19 13:50 ` [PATCH v3 2/8] kernel-shark: Remove the definition of KS_CONF_DIR Yordan Karadzhov
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov

By default the "Last session" configuration file will be saved in
${HOME}/.cache/kernelshark. If ${HOME}/.cache/kernelshark doesn't exist,
it will be created automatically. The user can select another directory
to be used to store the cached data. This can be done by setting the
environment variable KS_USER_CACHE_DIR. In this case if the path (the
value of KS_USER_CACHE_DIR) doesn't exist the user will be asked before
create it.

Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsMainWindow.cpp | 68 +++++++++++++++++++++++++++----
 kernel-shark/src/KsMainWindow.hpp |  4 ++
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 7afb721..c839aca 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -133,13 +133,15 @@ KsMainWindow::KsMainWindow(QWidget *parent)
 KsMainWindow::~KsMainWindow()
 {
 	kshark_context *kshark_ctx(nullptr);
-	QString file = KS_CONF_DIR;
+	QString file = lastSessionFile();
 
-	file += "/lastsession.json";
+	if (!file.isEmpty()) {
+		QByteArray fileBA = file.toLocal8Bit();
 
-	_updateSession();
-	kshark_save_config_file(file.toLocal8Bit().data(),
-				_session.getConfDocPtr());
+		_updateSession();
+		kshark_save_config_file(fileBA.data(),
+					_session.getConfDocPtr());
+	}
 
 	_data.clear();
 
@@ -368,12 +370,60 @@ void KsMainWindow::_open()
 		loadDataFile(fileName);
 }
 
-void KsMainWindow::_restoreSession()
+QString KsMainWindow::_getCacheDir()
+{
+	QString dir;
+
+	auto lamMakePath = [&] (bool ask) {
+		if (ask) {
+			QMessageBox::StandardButton reply;
+			QString err("KernelShark cache directory not found!\n");
+			QString question =
+				QString("Do you want to create %1").arg(dir);
+
+			reply = QMessageBox::question(this, "KernelShark",
+						      err + question,
+						      QMessageBox::Yes |
+						      QMessageBox::No);
+
+			if (reply == QMessageBox::No) {
+				dir.clear();
+				return;
+			}
+		}
+
+		QDir().mkpath(dir);
+	};
+
+	dir = getenv("KS_USER_CACHE_DIR");
+	if (!dir.isEmpty()) {
+		if (!QDir(dir).exists())
+			lamMakePath(true);
+	} else {
+		dir = QString(QDir::homePath()) +
+		      "/.cache/kernelshark";
+		if (!QDir(dir).exists())
+			lamMakePath(false);
+	}
+
+	return dir;
+}
+
+/** Get the description file of the last session. */
+QString KsMainWindow::lastSessionFile()
 {
-	QString file = KS_CONF_DIR;
-	file += "/lastsession.json";
+	QString file;
 
-	loadSession(file);
+	file = _getCacheDir();
+	if (!file.isEmpty())
+		file += "/lastsession.json";
+
+	return file;
+}
+
+void KsMainWindow::_restoreSession()
+{
+	loadSession(lastSessionFile());
 	_graph.updateGeom();
 }
 
diff --git a/kernel-shark/src/KsMainWindow.hpp b/kernel-shark/src/KsMainWindow.hpp
index 78cd442..ec6506e 100644
--- a/kernel-shark/src/KsMainWindow.hpp
+++ b/kernel-shark/src/KsMainWindow.hpp
@@ -37,6 +37,8 @@ public:
 
 	void loadSession(const QString &fileName);
 
+	QString lastSessionFile();
+
 	/**
 	 * @brief
 	 *
@@ -230,6 +232,8 @@ private:
 
 	void _filterSyncCBoxUpdate(kshark_context *kshark_ctx);
 
+	QString _getCacheDir();
+
 private slots:
 	void _captureFinished(int, QProcess::ExitStatus);
 };
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/8] kernel-shark: Remove the definition of KS_CONF_DIR
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
  2019-04-19 13:50 ` [PATCH v3 1/8] kernel-shark: Configuration information in ${HOME}/.cache/kernelshark Yordan Karadzhov
@ 2019-04-19 13:50 ` Yordan Karadzhov
  2019-04-19 13:50 ` [PATCH v3 3/8] kernel-shark: Add logic for the initial path of Open-File dialogs Yordan Karadzhov
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov

KS_CONF_DIR is no longer used, so we do not need to define it in the
Cmake-generated header file.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/CMakeLists.txt     | 5 -----
 kernel-shark/build/deff.h.cmake | 3 ---
 2 files changed, 8 deletions(-)

diff --git a/kernel-shark/CMakeLists.txt b/kernel-shark/CMakeLists.txt
index 10cb696..1aee858 100644
--- a/kernel-shark/CMakeLists.txt
+++ b/kernel-shark/CMakeLists.txt
@@ -12,11 +12,6 @@ message("\n project: Kernel Shark: (version: ${KS_VERSION_STRING})\n")
 
 set(KS_DIR ${CMAKE_SOURCE_DIR})
 
-# Make a directory to hold configuration files. To change this do:
-# cmake .. -DKS_CONF_DIR=/your/preferred/path
-set(KS_CONF_DIR "${KS_DIR}/.ksconf" CACHE STRING "Directory for configuration files.")
-file(MAKE_DIRECTORY ${KS_CONF_DIR})
-
 include(${KS_DIR}/build/FindTraceCmd.cmake)
 include(${KS_DIR}/build/FindJSONC.cmake)
 
diff --git a/kernel-shark/build/deff.h.cmake b/kernel-shark/build/deff.h.cmake
index 80d624c..8041cfc 100644
--- a/kernel-shark/build/deff.h.cmake
+++ b/kernel-shark/build/deff.h.cmake
@@ -14,9 +14,6 @@
 /** KernelShark source code path. */
 #cmakedefine KS_DIR "@KS_DIR@"
 
-/** KernelShark configuration directory path. */
-#cmakedefine KS_CONF_DIR "@KS_CONF_DIR@"
-
 /** Location of the trace-cmd executable. */
 #cmakedefine TRACECMD_BIN_DIR "@TRACECMD_BIN_DIR@"
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/8] kernel-shark: Add logic for the initial path of Open-File dialogs
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
  2019-04-19 13:50 ` [PATCH v3 1/8] kernel-shark: Configuration information in ${HOME}/.cache/kernelshark Yordan Karadzhov
  2019-04-19 13:50 ` [PATCH v3 2/8] kernel-shark: Remove the definition of KS_CONF_DIR Yordan Karadzhov
@ 2019-04-19 13:50 ` Yordan Karadzhov
  2019-04-19 16:23   ` Steven Rostedt
  2019-04-19 13:50 ` [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path Yordan Karadzhov
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov

If the application has been started from the source code directory,
all Open-File dialogs will start there. If the application has been
started from its installation location, all Open-File dialogs will
start at ${HOME}.

Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsCaptureDialog.cpp |  6 +++---
 kernel-shark/src/KsMainWindow.cpp    | 12 ++++++------
 kernel-shark/src/KsUtils.hpp         | 15 +++++++++++++++
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index 1272c2e..57bf7c7 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -208,7 +208,7 @@ void KsCaptureControl::_importSettings()
 	/* Get the configuration document. */
 	fileName = QFileDialog::getOpenFileName(this,
 						"Import from Filter",
-						KS_DIR,
+						KsUtils::dialogDir(),
 						"Kernel Shark Config files (*.json);;");
 
 	if (fileName.isEmpty())
@@ -259,7 +259,7 @@ void KsCaptureControl::_exportSettings()
 	QString fileName =
 		QFileDialog::getSaveFileName(this,
 					     "Export to File",
-					     KS_DIR,
+					     KsUtils::dialogDir(),
 					     "Kernel Shark Config files (*.json);;");
 
 	if (fileName.isEmpty())
@@ -314,7 +314,7 @@ void KsCaptureControl::_browse()
 	QString fileName =
 		QFileDialog::getSaveFileName(this,
 					     "Save File",
-					     KS_DIR,
+					     KsUtils::dialogDir(),
 					     "trace-cmd files (*.dat);;All files (*)");
 
 	if (!fileName.isEmpty())
diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index c839aca..65ac71a 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -363,7 +363,7 @@ void KsMainWindow::_open()
 	QString fileName =
 		QFileDialog::getOpenFileName(this,
 					     "Open File",
-					     KS_DIR,
+					     KsUtils::dialogDir(),
 					     "trace-cmd files (*.dat);;All files (*)");
 
 	if (!fileName.isEmpty())
@@ -432,7 +432,7 @@ void KsMainWindow::_importSession()
 	QString fileName =
 		QFileDialog::getOpenFileName(this,
 					     "Import Session",
-					     KS_DIR,
+					     KsUtils::dialogDir(),
 					     "Kernel Shark Config files (*.json);;");
 
 	if (fileName.isEmpty())
@@ -463,7 +463,7 @@ void KsMainWindow::_exportSession()
 	QString fileName =
 		QFileDialog::getSaveFileName(this,
 					     "Export Filter",
-					     KS_DIR,
+					     KsUtils::dialogDir(),
 					     "Kernel Shark Config files (*.json);;");
 
 	if (fileName.isEmpty())
@@ -512,7 +512,7 @@ void KsMainWindow::_importFilter()
 	if (!kshark_instance(&kshark_ctx))
 		return;
 
-	fileName = QFileDialog::getOpenFileName(this, "Import Filter", KS_DIR,
+	fileName = QFileDialog::getOpenFileName(this, "Import Filter", KsUtils::dialogDir(),
 						"Kernel Shark Config files (*.json);;");
 
 	if (fileName.isEmpty())
@@ -540,7 +540,7 @@ void KsMainWindow::_exportFilter()
 	if (!kshark_instance(&kshark_ctx))
 		return;
 
-	fileName = QFileDialog::getSaveFileName(this, "Export Filter", KS_DIR,
+	fileName = QFileDialog::getSaveFileName(this, "Export Filter", KsUtils::dialogDir(),
 						"Kernel Shark Config files (*.json);;");
 
 	if (fileName.isEmpty())
@@ -861,7 +861,7 @@ void KsMainWindow::_pluginAdd()
 
 	fileNames =
 		QFileDialog::getOpenFileNames(this, "Add KernelShark plugins",
-					      KS_DIR,
+					      KsUtils::dialogDir(),
 					      "KernelShark Plugins (*.so);;");
 
 	if (fileNames.isEmpty())
diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp
index c8b5e88..77048ab 100644
--- a/kernel-shark/src/KsUtils.hpp
+++ b/kernel-shark/src/KsUtils.hpp
@@ -111,6 +111,21 @@ inline QString Ts2String(int64_t ts, int prec)
 
 bool matchCPUVisible(struct kshark_context *kshark_ctx,
 			      struct kshark_entry *e, int cpu);
+
+/**
+ * @brief Get the directory to be used when opening QFileDialog. If the
+ *	  application has been started from the source code directory, all
+ *	  Open File dialogs will start there. If the application has been
+ *	  started from its installation location, all Open File dialogs will
+ *	  start at ${HOME}.
+ */
+inline QString dialogDir()
+{
+	QString path = QCoreApplication::applicationFilePath();
+
+	return path.contains(KS_DIR) ? KS_DIR : QDir::homePath();
+}
+
 }; // KsUtils
 
 /** Identifier of the Dual Marker active state. */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
                   ` (2 preceding siblings ...)
  2019-04-19 13:50 ` [PATCH v3 3/8] kernel-shark: Add logic for the initial path of Open-File dialogs Yordan Karadzhov
@ 2019-04-19 13:50 ` Yordan Karadzhov
  2019-04-19 17:28   ` Steven Rostedt
  2019-04-19 13:50 ` [PATCH v3 5/8] kernel-shark: Rename KS_DIR to KS_SOURCE_DIR Yordan Karadzhov
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov

If the application has been started from the source code directory, all
build-in plugins will be loaded from kernel-shark/lib/
If the application has been started from its installation location, all
build-in plugins will be loaded from INSTALL_PREFIX/lib/kshark/plugins/

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/build/deff.h.cmake         |  3 +++
 kernel-shark/src/KsUtils.cpp            | 35 +++++++++++++++++--------
 kernel-shark/src/KsUtils.hpp            |  1 +
 kernel-shark/src/plugins/CMakeLists.txt |  2 +-
 4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/kernel-shark/build/deff.h.cmake b/kernel-shark/build/deff.h.cmake
index 8041cfc..ba211f4 100644
--- a/kernel-shark/build/deff.h.cmake
+++ b/kernel-shark/build/deff.h.cmake
@@ -14,6 +14,9 @@
 /** KernelShark source code path. */
 #cmakedefine KS_DIR "@KS_DIR@"
 
+/** KernelShark installation prefix path. */
+#cmakedefine _INSTALL_PREFIX "@_INSTALL_PREFIX@"
+
 /** Location of the trace-cmd executable. */
 #cmakedefine TRACECMD_BIN_DIR "@TRACECMD_BIN_DIR@"
 
diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp
index 6af0c66..b05c0dc 100644
--- a/kernel-shark/src/KsUtils.cpp
+++ b/kernel-shark/src/KsUtils.cpp
@@ -423,13 +423,12 @@ void KsPluginManager::_parsePluginList()
  */
 void KsPluginManager::registerFromList(kshark_context *kshark_ctx)
 {
-	auto lamRegBuiltIn = [&kshark_ctx](const QString &plugin)
+	auto lamRegBuiltIn = [&kshark_ctx, this](const QString &plugin)
 	{
 		char *lib;
 		int n;
 
-		n = asprintf(&lib, "%s/lib/plugin-%s.so",
-			     KS_DIR, plugin.toStdString().c_str());
+		lib = _pluginLibFromName(plugin, n);
 		if (n <= 0)
 			return;
 
@@ -458,13 +457,12 @@ void KsPluginManager::registerFromList(kshark_context *kshark_ctx)
  */
 void KsPluginManager::unregisterFromList(kshark_context *kshark_ctx)
 {
-	auto lamUregBuiltIn = [&kshark_ctx](const QString &plugin)
+	auto lamUregBuiltIn = [&kshark_ctx, this](const QString &plugin)
 	{
 		char *lib;
 		int n;
 
-		n = asprintf(&lib, "%s/lib/plugin-%s.so",
-			     KS_DIR, plugin.toStdString().c_str());
+		lib = _pluginLibFromName(plugin, n);
 		if (n <= 0)
 			return;
 
@@ -487,6 +485,23 @@ void KsPluginManager::unregisterFromList(kshark_context *kshark_ctx)
 			lamUregUser);
 }
 
+char *KsPluginManager::_pluginLibFromName(const QString &plugin, int &n)
+{
+	QString path = QCoreApplication::applicationFilePath();
+	std::string pluginStr = plugin.toStdString();
+	char *lib;
+
+	if (path.contains(KS_DIR)) {
+		n = asprintf(&lib, "%s/lib/plugin-%s.so",
+			     KS_DIR, pluginStr.c_str());
+	} else {
+		n = asprintf(&lib, "%s/lib/kshark/plugins/plugin-%s.so",
+			     _INSTALL_PREFIX, pluginStr.c_str());
+	}
+
+	return lib;
+}
+
 /**
  * @brief Register a Plugin.
  *
@@ -508,8 +523,7 @@ void KsPluginManager::registerPlugin(const QString &plugin)
 			 * The argument is the name of the plugin. From the
 			 * name get the library .so file.
 			 */
-			n = asprintf(&lib, "%s/lib/plugin-%s.so",
-					KS_DIR, plugin.toStdString().c_str());
+			lib = _pluginLibFromName(plugin, n);
 			if (n > 0) {
 				kshark_register_plugin(kshark_ctx, lib);
 				_registeredKsPlugins[i] = true;
@@ -570,8 +584,7 @@ void KsPluginManager::unregisterPlugin(const QString &plugin)
 			 * The argument is the name of the plugin. From the
 			 * name get the library .so file.
 			 */
-			n = asprintf(&lib, "%s/lib/plugin-%s.so", KS_DIR,
-				     plugin.toStdString().c_str());
+			lib = _pluginLibFromName(plugin, n);
 			if (n > 0) {
 				kshark_unregister_plugin(kshark_ctx, lib);
 				_registeredKsPlugins[i] = false;
@@ -579,7 +592,7 @@ void KsPluginManager::unregisterPlugin(const QString &plugin)
 			}
 
 			return;
-		} else if  (plugin.contains("/lib/plugin-" +
+		} else if (plugin.contains("/lib/plugin-" +
 			                   _ksPluginList[i], Qt::CaseInsensitive)) {
 			/*
 			 * The argument is the name of the library .so file.
diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp
index 77048ab..019bde8 100644
--- a/kernel-shark/src/KsUtils.hpp
+++ b/kernel-shark/src/KsUtils.hpp
@@ -238,6 +238,7 @@ signals:
 
 private:
 	void _parsePluginList();
+	char *_pluginLibFromName(const QString &plugin, int &n);
 
 	template <class T>
 	void _forEachInList(const QStringList &pl,
diff --git a/kernel-shark/src/plugins/CMakeLists.txt b/kernel-shark/src/plugins/CMakeLists.txt
index 6098275..64cf98d 100644
--- a/kernel-shark/src/plugins/CMakeLists.txt
+++ b/kernel-shark/src/plugins/CMakeLists.txt
@@ -29,6 +29,6 @@ BUILD_PLUGIN(NAME missed_events
 list(APPEND PLUGIN_LIST "missed_events default") # This plugin will be loaded by default
 
 install(TARGETS sched_events missed_events
-        LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/kshark/)
+        LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/kshark/plugins/)
 
 set(PLUGINS ${PLUGIN_LIST} PARENT_SCOPE)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 5/8] kernel-shark: Rename KS_DIR to KS_SOURCE_DIR
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
                   ` (3 preceding siblings ...)
  2019-04-19 13:50 ` [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path Yordan Karadzhov
@ 2019-04-19 13:50 ` Yordan Karadzhov
  2019-04-19 13:50 ` [PATCH v3 6/8] kernel-shark: Load Last Session from command line Yordan Karadzhov
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov

KS_SOURCE_DIR is a more appropriate name since it defines the path to
the KernelShark source code used to build the application.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/CMakeLists.txt     | 32 ++++++++++++++++----------------
 kernel-shark/build/deff.h.cmake |  2 +-
 kernel-shark/src/CMakeLists.txt | 10 +++++-----
 kernel-shark/src/KsUtils.cpp    |  4 ++--
 kernel-shark/src/KsUtils.hpp    |  2 +-
 5 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/kernel-shark/CMakeLists.txt b/kernel-shark/CMakeLists.txt
index 1aee858..825cabd 100644
--- a/kernel-shark/CMakeLists.txt
+++ b/kernel-shark/CMakeLists.txt
@@ -10,10 +10,10 @@ set(KS_VERSION_PATCH 8)
 set(KS_VERSION_STRING ${KS_VERSION_MAJOR}.${KS_VERSION_MINOR}.${KS_VERSION_PATCH})
 message("\n project: Kernel Shark: (version: ${KS_VERSION_STRING})\n")
 
-set(KS_DIR ${CMAKE_SOURCE_DIR})
+set(KS_SOURCE_DIR ${CMAKE_SOURCE_DIR})
 
-include(${KS_DIR}/build/FindTraceCmd.cmake)
-include(${KS_DIR}/build/FindJSONC.cmake)
+include(${KS_SOURCE_DIR}/build/FindTraceCmd.cmake)
+include(${KS_SOURCE_DIR}/build/FindJSONC.cmake)
 
 find_package(Doxygen)
 
@@ -29,8 +29,8 @@ if (Qt5Widgets_FOUND)
 
 endif (Qt5Widgets_FOUND)
 
-set(LIBRARY_OUTPUT_PATH    "${KS_DIR}/lib")
-set(EXECUTABLE_OUTPUT_PATH "${KS_DIR}/bin")
+set(LIBRARY_OUTPUT_PATH    "${KS_SOURCE_DIR}/lib")
+set(EXECUTABLE_OUTPUT_PATH "${KS_SOURCE_DIR}/bin")
 
 set(CMAKE_C_FLAGS   "${CMAKE_C_FLAGS} -Wall -g -pthread")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -g -std=c++11 -pthread")
@@ -49,8 +49,8 @@ endif (NOT _DEBUG)
 SET(CMAKE_INSTALL_RPATH "${_INSTALL_PREFIX}/lib/kshark/")
 SET(CMAKE_BUILD_WITH_INSTALL_RPATH FALSE)
 
-include_directories(${KS_DIR}/src/
-                    ${KS_DIR}/build/src/
+include_directories(${KS_SOURCE_DIR}/src/
+                    ${KS_SOURCE_DIR}/build/src/
                     ${JSONC_INCLUDE_DIR}
                     ${TRACECMD_INCLUDE_DIR}
                     ${TRACEEVENT_INCLUDE_DIR})
@@ -60,8 +60,8 @@ message(STATUS "C flags      : " ${CMAKE_C_FLAGS})
 message(STATUS "CXX flags    : " ${CMAKE_CXX_FLAGS})
 message(STATUS "Linker flags : " ${CMAKE_EXE_LINKER_FLAGS})
 
-add_subdirectory(${KS_DIR}/src)
-add_subdirectory(${KS_DIR}/examples)
+add_subdirectory(${KS_SOURCE_DIR}/src)
+add_subdirectory(${KS_SOURCE_DIR}/examples)
 
 if (_DOXYGEN_DOC AND DOXYGEN_FOUND)
 
@@ -72,16 +72,16 @@ if (_DOXYGEN_DOC AND DOXYGEN_FOUND)
                        WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/doc)
 
     set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_MAKE_CLEAN_FILES
-                                          "${KS_DIR}/doc/dox_build.log"
-                                          "${KS_DIR}/doc/html"
-                                          "${KS_DIR}/doc/latex")
+                                          "${KS_SOURCE_DIR}/doc/dox_build.log"
+                                          "${KS_SOURCE_DIR}/doc/html"
+                                          "${KS_SOURCE_DIR}/doc/latex")
 
 endif ()
 
-configure_file( ${KS_DIR}/build/ks.desktop.cmake
-                ${KS_DIR}/kernelshark.desktop)
+configure_file(${KS_SOURCE_DIR}/build/ks.desktop.cmake
+               ${KS_SOURCE_DIR}/kernelshark.desktop)
 
-configure_file( ${KS_DIR}/build/org.freedesktop.kshark-record.policy.cmake
-                ${KS_DIR}/org.freedesktop.kshark-record.policy)
+configure_file(${KS_SOURCE_DIR}/build/org.freedesktop.kshark-record.policy.cmake
+               ${KS_SOURCE_DIR}/org.freedesktop.kshark-record.policy)
 
 message("")
diff --git a/kernel-shark/build/deff.h.cmake b/kernel-shark/build/deff.h.cmake
index ba211f4..47c9d9f 100644
--- a/kernel-shark/build/deff.h.cmake
+++ b/kernel-shark/build/deff.h.cmake
@@ -12,7 +12,7 @@
 #cmakedefine KS_VERSION_STRING "@KS_VERSION_STRING@"
 
 /** KernelShark source code path. */
-#cmakedefine KS_DIR "@KS_DIR@"
+#cmakedefine KS_SOURCE_DIR "@KS_SOURCE_DIR@"
 
 /** KernelShark installation prefix path. */
 #cmakedefine _INSTALL_PREFIX "@_INSTALL_PREFIX@"
diff --git a/kernel-shark/src/CMakeLists.txt b/kernel-shark/src/CMakeLists.txt
index b7dbd7e..5f97916 100644
--- a/kernel-shark/src/CMakeLists.txt
+++ b/kernel-shark/src/CMakeLists.txt
@@ -81,13 +81,13 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND)
             RUNTIME DESTINATION ${_INSTALL_PREFIX}/bin/
             LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/kshark/)
 
-    install(FILES "${KS_DIR}/kernelshark.desktop"
+    install(FILES "${KS_SOURCE_DIR}/kernelshark.desktop"
             DESTINATION /usr/share/applications/)
 
-    install(FILES "${KS_DIR}/org.freedesktop.kshark-record.policy"
+    install(FILES "${KS_SOURCE_DIR}/org.freedesktop.kshark-record.policy"
             DESTINATION /usr/share/polkit-1/actions/)
 
-    install(PROGRAMS "${KS_DIR}/bin/kshark-su-record"
+    install(PROGRAMS "${KS_SOURCE_DIR}/bin/kshark-su-record"
             DESTINATION ${_INSTALL_PREFIX}/bin/)
 
 endif (Qt5Widgets_FOUND AND Qt5Network_FOUND)
@@ -96,5 +96,5 @@ add_subdirectory(plugins)
 
 find_program(DO_AS_ROOT pkexec)
 
-configure_file( ${KS_DIR}/build/deff.h.cmake
-                ${KS_DIR}/src/KsCmakeDef.hpp)
+configure_file( ${KS_SOURCE_DIR}/build/deff.h.cmake
+                ${KS_SOURCE_DIR}/src/KsCmakeDef.hpp)
diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp
index b05c0dc..387a1e0 100644
--- a/kernel-shark/src/KsUtils.cpp
+++ b/kernel-shark/src/KsUtils.cpp
@@ -491,9 +491,9 @@ char *KsPluginManager::_pluginLibFromName(const QString &plugin, int &n)
 	std::string pluginStr = plugin.toStdString();
 	char *lib;
 
-	if (path.contains(KS_DIR)) {
+	if (path.contains(KS_SOURCE_DIR)) {
 		n = asprintf(&lib, "%s/lib/plugin-%s.so",
-			     KS_DIR, pluginStr.c_str());
+			     KS_SOURCE_DIR, pluginStr.c_str());
 	} else {
 		n = asprintf(&lib, "%s/lib/kshark/plugins/plugin-%s.so",
 			     _INSTALL_PREFIX, pluginStr.c_str());
diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp
index 019bde8..e776d62 100644
--- a/kernel-shark/src/KsUtils.hpp
+++ b/kernel-shark/src/KsUtils.hpp
@@ -123,7 +123,7 @@ inline QString dialogDir()
 {
 	QString path = QCoreApplication::applicationFilePath();
 
-	return path.contains(KS_DIR) ? KS_DIR : QDir::homePath();
+	return path.contains(KS_SOURCE_DIR) ? KS_SOURCE_DIR : QDir::homePath();
 }
 
 }; // KsUtils
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 6/8] kernel-shark: Load Last Session from command line
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
                   ` (4 preceding siblings ...)
  2019-04-19 13:50 ` [PATCH v3 5/8] kernel-shark: Rename KS_DIR to KS_SOURCE_DIR Yordan Karadzhov
@ 2019-04-19 13:50 ` Yordan Karadzhov
  2019-04-19 18:16   ` Steven Rostedt
  2019-04-19 13:50 ` [PATCH v3 7/8] kernel-shark: Use proper searching condition when the dataset is small Yordan Karadzhov
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov

./kernelshark -l  will load directly the Last Session

Signed-off-by : Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/kernelshark.cpp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel-shark/src/kernelshark.cpp b/kernel-shark/src/kernelshark.cpp
index 2ec91de..1ec6678 100644
--- a/kernel-shark/src/kernelshark.cpp
+++ b/kernel-shark/src/kernelshark.cpp
@@ -28,6 +28,7 @@ void usage(const char *prog)
 	printf("  -p	register plugin, use plugin name, absolute or relative path\n");
 	printf("  -u	unregister plugin, use plugin name or absolute path\n");
 	printf("  -s	import a session\n");
+	printf("  -l	import the last session\n");
 }
 
 int main(int argc, char **argv)
@@ -40,7 +41,7 @@ int main(int argc, char **argv)
 	int c;
 	bool fromSession = false;
 
-	while ((c = getopt(argc, argv, "hvi:p:u:s:")) != -1) {
+	while ((c = getopt(argc, argv, "hvi:p:u:s:l")) != -1) {
 		switch(c) {
 		case 'h':
 			usage(argv[0]);
@@ -65,6 +66,12 @@ int main(int argc, char **argv)
 		case 's':
 			ks.loadSession(QString(optarg));
 			fromSession = true;
+			break;
+
+		case 'l':
+			ks.loadSession(ks.lastSessionFile());
+			fromSession = true;
+			break;
 
 		default:
 			break;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 7/8] kernel-shark: Use proper searching condition when the dataset is small
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
                   ` (5 preceding siblings ...)
  2019-04-19 13:50 ` [PATCH v3 6/8] kernel-shark: Load Last Session from command line Yordan Karadzhov
@ 2019-04-19 13:50 ` Yordan Karadzhov
  2019-04-19 13:50 ` [PATCH v3 8/8] kernel-shark: Handle the case when the marker points to a filtered entry Yordan Karadzhov
  2019-04-19 18:13 ` [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Steven Rostedt
  8 siblings, 0 replies; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov, Slavomir Kaslev

If the data-set is small we do not want to have the overhead added by
the update of the progress bar. Because of this we bypass the state
switching of the FSM. However, in this case the the search condition
has to be updated by hand.

Reported-by: Slavomir Kaslev <kaslevs@vmware.com>
Fixes: 1615b02b (kernel-shark-qt: Optimize the search in a case of a small data-set)
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsTraceViewer.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel-shark/src/KsTraceViewer.cpp b/kernel-shark/src/KsTraceViewer.cpp
index 4e2c93e..04a38b8 100644
--- a/kernel-shark/src/KsTraceViewer.cpp
+++ b/kernel-shark/src/KsTraceViewer.cpp
@@ -596,8 +596,11 @@ size_t KsTraceViewer::_searchItems()
 	if (_proxyModel.rowCount({}) < KS_SEARCH_SHOW_PROGRESS_MIN) {
 		/*
 		 * This is a small data-set. Do a single-threaded search
-		 * without showing the progress.
+		 * without showing the progress. We will bypass the state
+		 * switching, hence the search condition has to be updated
+		 * by hand.
 		 */
+		_searchFSM.updateCondition();
 		_proxyModel.search(column, searchText, _searchFSM.condition(), &_matchList,
 				   nullptr, nullptr);
 	} else {
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 8/8] kernel-shark: Handle the case when the marker points to a filtered entry
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
                   ` (6 preceding siblings ...)
  2019-04-19 13:50 ` [PATCH v3 7/8] kernel-shark: Use proper searching condition when the dataset is small Yordan Karadzhov
@ 2019-04-19 13:50 ` Yordan Karadzhov
  2019-04-19 18:13 ` [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Steven Rostedt
  8 siblings, 0 replies; 16+ messages in thread
From: Yordan Karadzhov @ 2019-04-19 13:50 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, y.karadz, Yordan Karadzhov

Markers can point to entries that are filtered out. When switching
the marker it may happen that new Active Marker points to an entry
that is filtered. In this case no actions must be taken and a warning
message for the user must be displayed.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsTraceViewer.cpp | 27 +++++++++++++++++----------
 kernel-shark/src/KsTraceViewer.hpp |  2 ++
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/kernel-shark/src/KsTraceViewer.cpp b/kernel-shark/src/KsTraceViewer.cpp
index 04a38b8..85da64f 100644
--- a/kernel-shark/src/KsTraceViewer.cpp
+++ b/kernel-shark/src/KsTraceViewer.cpp
@@ -61,7 +61,8 @@ KsTraceViewer::KsTraceViewer(QWidget *parent)
   _graphFollowsCheckBox(this),
   _graphFollows(true),
   _mState(nullptr),
-  _data(nullptr)
+  _data(nullptr),
+  _em(this)
 {
 	this->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::Maximum);
 
@@ -493,15 +494,21 @@ void KsTraceViewer::markSwitch()
 		QModelIndex index =
 			_proxyModel.mapFromSource(_model.index(row, 0));
 
-		/*
-		 * The row of the active marker will be colored according to
-		 * the assigned property of the current state of the Dual
-		 * marker. Auto-scrolling is temporarily disabled because we
-		 * do not want to scroll to the position of the marker yet.
-		 */
-		_view.setAutoScroll(false);
-		_view.selectRow(index.row());
-		_view.setAutoScroll(true);
+		if (index.isValid()) {
+			/*
+			 * The row of the active marker will be colored according to
+			 * the assigned property of the current state of the Dual
+			 * marker. Auto-scrolling is temporarily disabled because we
+			 * do not want to scroll to the position of the marker yet.
+			 */
+			_view.setAutoScroll(false);
+			_view.selectRow(index.row());
+			_view.setAutoScroll(true);
+		} else {
+			_view.clearSelection();
+			QString err("The marker's entry is filtered out.");
+			_em.showMessage(err);
+		}
 	} else {
 		_view.clearSelection();
 	}
diff --git a/kernel-shark/src/KsTraceViewer.hpp b/kernel-shark/src/KsTraceViewer.hpp
index cf529ba..15eee50 100644
--- a/kernel-shark/src/KsTraceViewer.hpp
+++ b/kernel-shark/src/KsTraceViewer.hpp
@@ -119,6 +119,8 @@ private:
 
 	KsDataStore		*_data;
 
+	QErrorMessage	_em;
+
 	enum Condition
 	{
 		Containes = 0,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/8] kernel-shark: Add logic for the initial path of Open-File dialogs
  2019-04-19 13:50 ` [PATCH v3 3/8] kernel-shark: Add logic for the initial path of Open-File dialogs Yordan Karadzhov
@ 2019-04-19 16:23   ` Steven Rostedt
  0 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2019-04-19 16:23 UTC (permalink / raw)
  To: Yordan Karadzhov; +Cc: linux-trace-devel, y.karadz

On Fri, 19 Apr 2019 16:50:31 +0300
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> If the application has been started from the source code directory,
> all Open-File dialogs will start there. If the application has been
> started from its installation location, all Open-File dialogs will
> start at ${HOME}.

Why can't the open dialog boxes start at where the program was executed
from?

Also, if one were to change the location of the open dialog, can we
save that and if the open dialog happens again, it will open where the
user left off the last time (like Mozilla does).

-- Steve

> 
> Suggested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path
  2019-04-19 13:50 ` [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path Yordan Karadzhov
@ 2019-04-19 17:28   ` Steven Rostedt
  2019-04-22 11:29     ` Yordan Karadzhov (VMware)
  0 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2019-04-19 17:28 UTC (permalink / raw)
  To: Yordan Karadzhov; +Cc: linux-trace-devel, y.karadz

On Fri, 19 Apr 2019 16:50:32 +0300
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> +char *KsPluginManager::_pluginLibFromName(const QString &plugin, int &n)
> +{
> +	QString path = QCoreApplication::applicationFilePath();
> +	std::string pluginStr = plugin.toStdString();
> +	char *lib;
> +
> +	if (path.contains(KS_DIR)) {

I'd rather not use the hardcoded path. If I build the code on one
machine, tarball it up and move it to another machine and extract it,
and then run that code from that machine, I want it to still use the
plugins for that machine.

I was hoping to test:

	string = cmdline_path() + "../../kernel-shark/lib/";

If that exists, then we know that we are in the source directory.

-- Steve

> +		n = asprintf(&lib, "%s/lib/plugin-%s.so",
> +			     KS_DIR, pluginStr.c_str());
> +	} else {
> +		n = asprintf(&lib, "%s/lib/kshark/plugins/plugin-%s.so",
> +			     _INSTALL_PREFIX, pluginStr.c_str());
> +	}
> +
> +	return lib;
> +}
> +

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/8] Various modifications and fixes toward KS 1.0
  2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
                   ` (7 preceding siblings ...)
  2019-04-19 13:50 ` [PATCH v3 8/8] kernel-shark: Handle the case when the marker points to a filtered entry Yordan Karadzhov
@ 2019-04-19 18:13 ` Steven Rostedt
  8 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2019-04-19 18:13 UTC (permalink / raw)
  To: Yordan Karadzhov; +Cc: linux-trace-devel, y.karadz

On Fri, 19 Apr 2019 16:50:28 +0300
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> The patch-set contains reimplementation of patches that have been sent
> already, but have been found to have various problems. The last two
> patch are fixes of bugs that have been found recently.
> 
> Changes in v3:
>  - Corrections from Slavomir (in patches 1/8 and 3/8)
> 
> Yordan Karadzhov (8):
>   kernel-shark: Configuration information in ${HOME}/.cache/kernelshark
>   kernel-shark: Remove the definition of KS_CONF_DIR
>   kernel-shark: Add logic for the initial path of Open-File dialogs
>   kernel-shark: Add logic for the plugins search path
>   kernel-shark: Rename KS_DIR to KS_SOURCE_DIR
>   kernel-shark: Load Last Session from command line
>   kernel-shark: Use proper searching condition when the dataset is small
>   kernel-shark: Handle the case when the marker points to a filtered
>     entry

I applied patches:

 1, 2 and 7

-- Steve

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 6/8] kernel-shark: Load Last Session from command line
  2019-04-19 13:50 ` [PATCH v3 6/8] kernel-shark: Load Last Session from command line Yordan Karadzhov
@ 2019-04-19 18:16   ` Steven Rostedt
  0 siblings, 0 replies; 16+ messages in thread
From: Steven Rostedt @ 2019-04-19 18:16 UTC (permalink / raw)
  To: Yordan Karadzhov; +Cc: linux-trace-devel, y.karadz

On Fri, 19 Apr 2019 16:50:34 +0300
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> ./kernelshark -l  will load directly the Last Session
> 
> Signed-off-by : Yordan Karadzhov <ykaradzhov@vmware.com>\\

I just applied this patch too. It failed my script and I didn't notice
due to the space between "by" and ":"

-- Steve


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path
  2019-04-19 17:28   ` Steven Rostedt
@ 2019-04-22 11:29     ` Yordan Karadzhov (VMware)
  2019-04-22 11:50       ` Steven Rostedt
  0 siblings, 1 reply; 16+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-04-22 11:29 UTC (permalink / raw)
  To: Steven Rostedt, Yordan Karadzhov; +Cc: linux-trace-devel



On 19.04.19 г. 20:28 ч., Steven Rostedt wrote:
> On Fri, 19 Apr 2019 16:50:32 +0300
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> 
>> +char *KsPluginManager::_pluginLibFromName(const QString &plugin, int &n)
>> +{
>> +	QString path = QCoreApplication::applicationFilePath();
>> +	std::string pluginStr = plugin.toStdString();
>> +	char *lib;
>> +
>> +	if (path.contains(KS_DIR)) {
> 
> I'd rather not use the hardcoded path. If I build the code on one
> machine, tarball it up and move it to another machine and extract it,
> and then run that code from that machine, I want it to still use the
> plugins for that machine.
> 
> I was hoping to test:
> 
> 	string = cmdline_path() + "../../kernel-shark/lib/";
> 
> If that exists, then we know that we are in the source directory.

I don't thing this is a good idea. If we search for plugins in a path 
that is defined like this:
  cmdline_path() + "/something/hard/coded/lib/"

Then the GUI will do one thing when started like this:
./kernelshark

and anther thing when started like this:
bin/kernelshark

and this can be very surprising behavior for the user

The other solution has it own weaknesses, but at least it sounds like a 
simple rule: "If you want to use the compiled version of the plugins you 
have to start the GUI from the source code directory used to build. 
Otherwise the installed version of the plugins will be used."

Thanks!
Yordan



> 
> -- Steve
> 
>> +		n = asprintf(&lib, "%s/lib/plugin-%s.so",
>> +			     KS_DIR, pluginStr.c_str());
>> +	} else {
>> +		n = asprintf(&lib, "%s/lib/kshark/plugins/plugin-%s.so",
>> +			     _INSTALL_PREFIX, pluginStr.c_str());
>> +	}
>> +
>> +	return lib;
>> +}
>> +

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path
  2019-04-22 11:29     ` Yordan Karadzhov (VMware)
@ 2019-04-22 11:50       ` Steven Rostedt
  2019-04-22 12:21         ` Yordan Karadzhov (VMware)
  0 siblings, 1 reply; 16+ messages in thread
From: Steven Rostedt @ 2019-04-22 11:50 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: Yordan Karadzhov, linux-trace-devel

On Mon, 22 Apr 2019 14:29:49 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:


> > I was hoping to test:
> > 
> > 	string = cmdline_path() + "../../kernel-shark/lib/";
> > 
> > If that exists, then we know that we are in the source directory.  
> 
> I don't thing this is a good idea. If we search for plugins in a path 
> that is defined like this:
>   cmdline_path() + "/something/hard/coded/lib/"

Your missing the "../.." part.

> 
> Then the GUI will do one thing when started like this:
> ./kernelshark
> 
> and anther thing when started like this:
> bin/kernelshark

No it shouldn't, unless you moved the binary.

I said to add "../../plugins" to the path that the kernelshark binary
is executed from. If you were to move kernelshark, then yes. it would
not longer give you the same result.

If the binary is in:

 kernelshark/bin/kernelshark

and you ran it as: kernelshark/bin/kernelshark using the path
"kernelshark/bin" and adding "../lib" would give you "kernelshark/lib"
directory.

 "kernelshark/bin/../lib" == "kernelshark/lib"

Also, if you were to cd to kernelshark and run

 "bin/kernelshark" the path would be "bin/../lib" which would be
 equal to  "lib" and being in the kernelshark directory, would
 give you the plugins directory that is the same as the previous command.

If you cd to kernelshark/bin and ran "./kernelshark" we would then use
"./../lib" which is the same as "../lib" which is still the same path
as the other two.

But last call we discussed a way to find the full path name of the
binary being executed. And if that's the case, we could not only add
"../lib" we could also check that the binary being executed is also in
a "kernelshark/bin" first.

> 
> and this can be very surprising behavior for the user
> 
> The other solution has it own weaknesses, but at least it sounds like a 
> simple rule: "If you want to use the compiled version of the plugins you 
> have to start the GUI from the source code directory used to build. 
> Otherwise the installed version of the plugins will be used."

Building from source, then moving that source tree to another path
shouldn't cause the binary to act differently.

-- Steve

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path
  2019-04-22 11:50       ` Steven Rostedt
@ 2019-04-22 12:21         ` Yordan Karadzhov (VMware)
  0 siblings, 0 replies; 16+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-04-22 12:21 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Yordan Karadzhov, linux-trace-devel



On 22.04.19 г. 14:50 ч., Steven Rostedt wrote:
> On Mon, 22 Apr 2019 14:29:49 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
> 
>>> I was hoping to test:
>>>
>>> 	string = cmdline_path() + "../../kernel-shark/lib/";
>>>
>>> If that exists, then we know that we are in the source directory.
>>
>> I don't thing this is a good idea. If we search for plugins in a path
>> that is defined like this:
>>    cmdline_path() + "/something/hard/coded/lib/"
> 
> Your missing the "../.." part.
> 
>>
>> Then the GUI will do one thing when started like this:
>> ./kernelshark
>>
>> and anther thing when started like this:
>> bin/kernelshark
> 
> No it shouldn't, unless you moved the binary.
> 
> I said to add "../../plugins" to the path that the kernelshark binary
> is executed from. If you were to move kernelshark, then yes. it would
> not longer give you the same result.
> 
> If the binary is in:
> 
>   kernelshark/bin/kernelshark
> 
> and you ran it as: kernelshark/bin/kernelshark using the path
> "kernelshark/bin" and adding "../lib" would give you "kernelshark/lib"
> directory.
> 
>   "kernelshark/bin/../lib" == "kernelshark/lib"
> 
> Also, if you were to cd to kernelshark and run
> 
>   "bin/kernelshark" the path would be "bin/../lib" which would be
>   equal to  "lib" and being in the kernelshark directory, would
>   give you the plugins directory that is the same as the previous command.
> 
> If you cd to kernelshark/bin and ran "./kernelshark" we would then use
> "./../lib" which is the same as "../lib" which is still the same path
> as the other two.
> 
> But last call we discussed a way to find the full path name of the
> binary being executed. And if that's the case, we could not only add
> "../lib" we could also check that the binary being executed is also in
> a "kernelshark/bin" first.
> 

OK, now I think I understand what you want.
Thanks!
Yordan


>>
>> and this can be very surprising behavior for the user
>>
>> The other solution has it own weaknesses, but at least it sounds like a
>> simple rule: "If you want to use the compiled version of the plugins you
>> have to start the GUI from the source code directory used to build.
>> Otherwise the installed version of the plugins will be used."
> 
> Building from source, then moving that source tree to another path
> shouldn't cause the binary to act differently.
> 
> -- Steve
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-04-22 12:21 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-19 13:50 [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Yordan Karadzhov
2019-04-19 13:50 ` [PATCH v3 1/8] kernel-shark: Configuration information in ${HOME}/.cache/kernelshark Yordan Karadzhov
2019-04-19 13:50 ` [PATCH v3 2/8] kernel-shark: Remove the definition of KS_CONF_DIR Yordan Karadzhov
2019-04-19 13:50 ` [PATCH v3 3/8] kernel-shark: Add logic for the initial path of Open-File dialogs Yordan Karadzhov
2019-04-19 16:23   ` Steven Rostedt
2019-04-19 13:50 ` [PATCH v3 4/8] kernel-shark: Add logic for the plugins search path Yordan Karadzhov
2019-04-19 17:28   ` Steven Rostedt
2019-04-22 11:29     ` Yordan Karadzhov (VMware)
2019-04-22 11:50       ` Steven Rostedt
2019-04-22 12:21         ` Yordan Karadzhov (VMware)
2019-04-19 13:50 ` [PATCH v3 5/8] kernel-shark: Rename KS_DIR to KS_SOURCE_DIR Yordan Karadzhov
2019-04-19 13:50 ` [PATCH v3 6/8] kernel-shark: Load Last Session from command line Yordan Karadzhov
2019-04-19 18:16   ` Steven Rostedt
2019-04-19 13:50 ` [PATCH v3 7/8] kernel-shark: Use proper searching condition when the dataset is small Yordan Karadzhov
2019-04-19 13:50 ` [PATCH v3 8/8] kernel-shark: Handle the case when the marker points to a filtered entry Yordan Karadzhov
2019-04-19 18:13 ` [PATCH v3 0/8] Various modifications and fixes toward KS 1.0 Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.