[PATCH 0/4] Add dialog for loading user-defined plugins

[PATCH 0/4] Add dialog for loading user-defined plugins

[Yordan Karadzhov]

So far loading user-defined plugins was possible only from the
command line

./kernelshark -p /path/to/your/plugin-lib.so

This patch set adds the possibility to load a plugin from the GUI
itself by using a dedicated dialog available in Tools


Yordan Karadzhov (4):
  kernel-shark: Define addPlugin method for KsPluginManager
  kernel-shark: Add dialog for user-defined plugin to the Tools menu
  kernel-shark: Rename the manu action for managing registered plugins
  kernel-shark: Add icons for "Add plugin" and "Manage plugins"

 kernel-shark/src/KsMainWindow.cpp | 32 ++++++++++++++++++++++++++-----
 kernel-shark/src/KsMainWindow.hpp |  6 +++++-
 kernel-shark/src/KsUtils.cpp      | 17 ++++++++++++++++
 kernel-shark/src/KsUtils.hpp      |  3 +++
 4 files changed, 52 insertions(+), 6 deletions(-)

-- 
2.19.1

[PATCH 1/4] kernel-shark: Define addPlugin method for KsPluginManager

[Yordan Karadzhov]

The method can be used to register and load a user-defined plugin.
All other previously loaded plugins will be reloaded.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsUtils.cpp | 17 +++++++++++++++++
 kernel-shark/src/KsUtils.hpp |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp
index d7b1753..4d05bda 100644
--- a/kernel-shark/src/KsUtils.cpp
+++ b/kernel-shark/src/KsUtils.cpp
@@ -596,6 +596,23 @@ void KsPluginManager::unregisterPlugin(const QString &plugin)
 	}
 }
 
+/** @brief Add to the list and load a user-provided plugin. All other
+ *	   previously loaded plugins will be reloaded.
+ *
+ * @param fileName: the library file (.so) of the plugin.
+*/
+void KsPluginManager::addPlugin(const QString &fileName)
+{
+	kshark_context *kshark_ctx(nullptr);
+
+	if (!kshark_instance(&kshark_ctx))
+		return;
+
+	kshark_handle_plugins(kshark_ctx, KSHARK_PLUGIN_CLOSE);
+	registerPlugin(fileName);
+	kshark_handle_plugins(kshark_ctx, KSHARK_PLUGIN_INIT);
+}
+
 /** Unload all plugins. */
 void KsPluginManager::unloadAll()
 {
diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp
index cb95b4f..ae7f6d1 100644
--- a/kernel-shark/src/KsUtils.hpp
+++ b/kernel-shark/src/KsUtils.hpp
@@ -210,6 +210,9 @@ public:
 
 	void registerPlugin(const QString &plugin);
 	void unregisterPlugin(const QString &plugin);
+
+	void addPlugin(const QString &fileName);
+
 	void unloadAll();
 
 	void updatePlugins(QVector<int> pluginId);
-- 
2.19.1

Re: [PATCH 1/4] kernel-shark: Define addPlugin method for KsPluginManager

[Slavomir Kaslev]

On Fri, Mar 8, 2019 at 7:14 PM Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
>
> The method can be used to register and load a user-defined plugin.
> All other previously loaded plugins will be reloaded.
>
> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
> ---
>  kernel-shark/src/KsUtils.cpp | 17 +++++++++++++++++
>  kernel-shark/src/KsUtils.hpp |  3 +++
>  2 files changed, 20 insertions(+)
>
> diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp
> index d7b1753..4d05bda 100644
> --- a/kernel-shark/src/KsUtils.cpp
> +++ b/kernel-shark/src/KsUtils.cpp
> @@ -596,6 +596,23 @@ void KsPluginManager::unregisterPlugin(const QString &plugin)
>         }
>  }
>
> +/** @brief Add to the list and load a user-provided plugin. All other
> + *        previously loaded plugins will be reloaded.
> + *
> + * @param fileName: the library file (.so) of the plugin.
> +*/
> +void KsPluginManager::addPlugin(const QString &fileName)
> +{
> +       kshark_context *kshark_ctx(nullptr);
> +
> +       if (!kshark_instance(&kshark_ctx))
> +               return;
> +
> +       kshark_handle_plugins(kshark_ctx, KSHARK_PLUGIN_CLOSE);
> +       registerPlugin(fileName);
> +       kshark_handle_plugins(kshark_ctx, KSHARK_PLUGIN_INIT);
> +}

Calling addPlugin() several times in a row will reinitialize all
already registered plugins several times and lead to quadratic
complexity. This seems wrong to me.

Maybe we should split adding plugins from (re-)initializing them so
that one can add several plugins and then have initialized once when
they are actually needed to run. Wdyt?

--Slavi

> +
>  /** Unload all plugins. */
>  void KsPluginManager::unloadAll()
>  {
> diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp
> index cb95b4f..ae7f6d1 100644
> --- a/kernel-shark/src/KsUtils.hpp
> +++ b/kernel-shark/src/KsUtils.hpp
> @@ -210,6 +210,9 @@ public:
>
>         void registerPlugin(const QString &plugin);
>         void unregisterPlugin(const QString &plugin);
> +
> +       void addPlugin(const QString &fileName);
> +
>         void unloadAll();
>
>         void updatePlugins(QVector<int> pluginId);
> --
> 2.19.1
>


-- 
Slavomir Kaslev

Re: [PATCH 1/4] kernel-shark: Define addPlugin method for KsPluginManager

[Yordan Karadzhov]

On 11.03.19 г. 14:31 ч., Slavomir Kaslev wrote:
> On Fri, Mar 8, 2019 at 7:14 PM Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
>>
>> The method can be used to register and load a user-defined plugin.
>> All other previously loaded plugins will be reloaded.
>>
>> Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
>> ---
>>   kernel-shark/src/KsUtils.cpp | 17 +++++++++++++++++
>>   kernel-shark/src/KsUtils.hpp |  3 +++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/kernel-shark/src/KsUtils.cpp b/kernel-shark/src/KsUtils.cpp
>> index d7b1753..4d05bda 100644
>> --- a/kernel-shark/src/KsUtils.cpp
>> +++ b/kernel-shark/src/KsUtils.cpp
>> @@ -596,6 +596,23 @@ void KsPluginManager::unregisterPlugin(const QString &plugin)
>>          }
>>   }
>>
>> +/** @brief Add to the list and load a user-provided plugin. All other
>> + *        previously loaded plugins will be reloaded.
>> + *
>> + * @param fileName: the library file (.so) of the plugin.
>> +*/
>> +void KsPluginManager::addPlugin(const QString &fileName)
>> +{
>> +       kshark_context *kshark_ctx(nullptr);
>> +
>> +       if (!kshark_instance(&kshark_ctx))
>> +               return;
>> +
>> +       kshark_handle_plugins(kshark_ctx, KSHARK_PLUGIN_CLOSE);
>> +       registerPlugin(fileName);
>> +       kshark_handle_plugins(kshark_ctx, KSHARK_PLUGIN_INIT);
>> +}
> 
> Calling addPlugin() several times in a row will reinitialize all
> already registered plugins several times and lead to quadratic
> complexity. This seems wrong to me.
> 
> Maybe we should split adding plugins from (re-)initializing them so
> that one can add several plugins and then have initialized once when
> they are actually needed to run. Wdyt?
> 

Hi Slavi,

KsPluginManager is a helper class for the GUI. It only does what the GUI 
is supposed to do. Currently the dialog that allows to load user plugins 
provides loading only one plugin at a time, so the KsPluginManager 
provides only this functionality.

So the questions is: do we want a dialog that loads multiple plugins 
with one click?

On the other hand we do encourage the users to use the KernelShark 
library for making custom applications. This is the motivation for 
having the C API, but KsPluginManager it is not part of this API. 
KsPluginManager operates on top of the API and is intended to be used 
only by the GUI code.

Indeed the C API itself separates adding plugins from there initializing.

Thanks a lot!
Y.


> --Slavi 
> 
>> +
>>   /** Unload all plugins. */
>>   void KsPluginManager::unloadAll()
>>   {
>> diff --git a/kernel-shark/src/KsUtils.hpp b/kernel-shark/src/KsUtils.hpp
>> index cb95b4f..ae7f6d1 100644
>> --- a/kernel-shark/src/KsUtils.hpp
>> +++ b/kernel-shark/src/KsUtils.hpp
>> @@ -210,6 +210,9 @@ public:
>>
>>          void registerPlugin(const QString &plugin);
>>          void unregisterPlugin(const QString &plugin);
>> +
>> +       void addPlugin(const QString &fileName);
>> +
>>          void unloadAll();
>>
>>          void updatePlugins(QVector<int> pluginId);
>> --
>> 2.19.1
>>
> 
> 

Re: [PATCH 1/4] kernel-shark: Define addPlugin method for KsPluginManager

[Steven Rostedt]

On Mon, 11 Mar 2019 17:39:11 +0000
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> > Calling addPlugin() several times in a row will reinitialize all
> > already registered plugins several times and lead to quadratic
> > complexity. This seems wrong to me.
> > 
> > Maybe we should split adding plugins from (re-)initializing them so
> > that one can add several plugins and then have initialized once when
> > they are actually needed to run. Wdyt?
> >   
> 
> Hi Slavi,
> 
> KsPluginManager is a helper class for the GUI. It only does what the GUI 
> is supposed to do. Currently the dialog that allows to load user plugins 
> provides loading only one plugin at a time, so the KsPluginManager 
> provides only this functionality.
> 
> So the questions is: do we want a dialog that loads multiple plugins 
> with one click?

What would be the issue with allowing that?

> 
> On the other hand we do encourage the users to use the KernelShark 
> library for making custom applications. This is the motivation for 
> having the C API, but KsPluginManager it is not part of this API. 
> KsPluginManager operates on top of the API and is intended to be used 
> only by the GUI code.
> 
> Indeed the C API itself separates adding plugins from there initializing.

Just like clicking the plugin dialog, where you select the plugins to
add or remove and hit apply. Couldn't this be the same, or am I missing
something?

-- Steve

[PATCH 2/4] kernel-shark: Add dialog for user-defined plugin to the Tools menu

[Yordan Karadzhov]

The dialog allows the user to find and load the plugin's
lobrary (.so file).

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsMainWindow.cpp | 20 ++++++++++++++++++++
 kernel-shark/src/KsMainWindow.hpp |  4 ++++
 2 files changed, 24 insertions(+)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 362b955..004c9ac 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -59,6 +59,7 @@ KsMainWindow::KsMainWindow(QWidget *parent)
   _cpuSelectAction("CPUs", this),
   _taskSelectAction("Tasks", this),
   _pluginsAction("Plugins", this),
+  _addPluginAction("Add plugin", this),
   _captureAction("Record", this),
   _colorAction(this),
   _colSlider(this),
@@ -233,6 +234,11 @@ void KsMainWindow::_createActions()
 	connect(&_pluginsAction,	&QAction::triggered,
 		this,			&KsMainWindow::_pluginSelect);
 
+	_addPluginAction.setStatusTip("Add plugin");
+
+	connect(&_addPluginAction,	&QAction::triggered,
+		this,			&KsMainWindow::_pluginAdd);
+
 	_captureAction.setIcon(QIcon::fromTheme("media-record"));
 	_captureAction.setShortcut(tr("Ctrl+R"));
 	_captureAction.setStatusTip("Capture trace data");
@@ -336,6 +342,7 @@ void KsMainWindow::_createMenus()
 	/* Tools menu */
 	tools = menuBar()->addMenu("Tools");
 	tools->addAction(&_pluginsAction);
+	tools->addAction(&_addPluginAction);
 	tools->addAction(&_captureAction);
 	tools->addSeparator();
 	tools->addAction(&_colorAction);
@@ -796,6 +803,19 @@ void KsMainWindow::_pluginSelect()
 	dialog->show();
 }
 
+void KsMainWindow::_pluginAdd()
+{
+	QString fileName;
+
+	fileName = QFileDialog::getOpenFileName(this, "Add KernelShark plugin", KS_DIR,
+						"KernelShark Plugins (*.so);;");
+
+	if (fileName.isEmpty())
+		return;
+
+	_plugins.addPlugin(fileName);
+}
+
 void KsMainWindow::_record()
 {
 #ifndef DO_AS_ROOT
diff --git a/kernel-shark/src/KsMainWindow.hpp b/kernel-shark/src/KsMainWindow.hpp
index 969b603..a4eed19 100644
--- a/kernel-shark/src/KsMainWindow.hpp
+++ b/kernel-shark/src/KsMainWindow.hpp
@@ -132,6 +132,8 @@ private:
 	// Tools menu.
 	QAction		_pluginsAction;
 
+	QAction		_addPluginAction;
+
 	QAction		_captureAction;
 
 	QWidgetAction	_colorAction;
@@ -185,6 +187,8 @@ private:
 
 	void _pluginSelect();
 
+	void _pluginAdd();
+
 	void _record();
 
 	void _setColorPhase(int);
-- 
2.19.1

[PATCH 3/4] kernel-shark: Rename the manu action for managing registered plugins

[Yordan Karadzhov]

The menu action in Tools that is used to manage all registered
plugins is renamed in order to avoid confusion with the "Add plugin"
menu action, implemented in the previous patch.

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

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 004c9ac..f6889ca 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -58,7 +58,7 @@ KsMainWindow::KsMainWindow(QWidget *parent)
   _clearAllFilters("Clear all filters", this),
   _cpuSelectAction("CPUs", this),
   _taskSelectAction("Tasks", this),
-  _pluginsAction("Plugins", this),
+  _managePluginsAction("Manage plugins", this),
   _addPluginAction("Add plugin", this),
   _captureAction("Record", this),
   _colorAction(this),
@@ -228,10 +228,10 @@ void KsMainWindow::_createActions()
 		this,			&KsMainWindow::_taskSelect);
 
 	/* Tools menu */
-	_pluginsAction.setShortcut(tr("Ctrl+P"));
-	_pluginsAction.setStatusTip("Manage plugins");
+	_managePluginsAction.setShortcut(tr("Ctrl+P"));
+	_managePluginsAction.setStatusTip("Manage plugins");
 
-	connect(&_pluginsAction,	&QAction::triggered,
+	connect(&_managePluginsAction,	&QAction::triggered,
 		this,			&KsMainWindow::_pluginSelect);
 
 	_addPluginAction.setStatusTip("Add plugin");
@@ -341,7 +341,7 @@ void KsMainWindow::_createMenus()
 
 	/* Tools menu */
 	tools = menuBar()->addMenu("Tools");
-	tools->addAction(&_pluginsAction);
+	tools->addAction(&_managePluginsAction);
 	tools->addAction(&_addPluginAction);
 	tools->addAction(&_captureAction);
 	tools->addSeparator();
diff --git a/kernel-shark/src/KsMainWindow.hpp b/kernel-shark/src/KsMainWindow.hpp
index a4eed19..0727631 100644
--- a/kernel-shark/src/KsMainWindow.hpp
+++ b/kernel-shark/src/KsMainWindow.hpp
@@ -130,7 +130,7 @@ private:
 	QAction		_taskSelectAction;
 
 	// Tools menu.
-	QAction		_pluginsAction;
+	QAction		_managePluginsAction;
 
 	QAction		_addPluginAction;
 
-- 
2.19.1

[PATCH 4/4] kernel-shark: Add icons for "Add plugin" and "Manage plugins"

[Yordan Karadzhov]

Just a small cosmetic improvement.

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

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index f6889ca..782b230 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -229,11 +229,13 @@ void KsMainWindow::_createActions()
 
 	/* Tools menu */
 	_managePluginsAction.setShortcut(tr("Ctrl+P"));
+	_managePluginsAction.setIcon(QIcon::fromTheme("preferences-system"));
 	_managePluginsAction.setStatusTip("Manage plugins");
 
 	connect(&_managePluginsAction,	&QAction::triggered,
 		this,			&KsMainWindow::_pluginSelect);
 
+	_addPluginAction.setIcon(QIcon::fromTheme("applications-engineering"));
 	_addPluginAction.setStatusTip("Add plugin");
 
 	connect(&_addPluginAction,	&QAction::triggered,
-- 
2.19.1