[PATCH 0/3] Fixes for Capture needed befor KS 1.0

[PATCH 0/3] Fixes for Capture needed befor KS 1.0

[Yordan Karadzhov (VMware)]

Adderssing the issues reported by Steven here:

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204287


Yordan Karadzhov (VMware) (3):
  kernel-shark: kshark_import_event_filter() tolerate non-existing
    events
  kernel-shark: Allow KsCaptureControl widget to print text to the
    console
  kernel-shark: Handle errors when loading Capture configurations

 kernel-shark/src/KsCaptureDialog.cpp  | 29 +++++++++++++++++++++----
 kernel-shark/src/KsCaptureDialog.hpp  |  6 ++++++
 kernel-shark/src/libkshark-configio.c | 31 ++++++++++++++++-----------
 kernel-shark/src/libkshark.h          |  8 +++----
 4 files changed, 53 insertions(+), 21 deletions(-)

-- 
2.20.1

[PATCH 1/4] kernel-shark: kshark_import_event_filter() tolerates non-existing events

[Yordan Karadzhov (VMware)]

Instead of aborting, the function will ignore (skip) the non-existing
event and will continue loading the other events in the configuration
file. The return type is changed to int in order to provide information
about the number of events successfully added to the filter.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark/src/libkshark-configio.c | 33 +++++++++++++++------------
 kernel-shark/src/libkshark.h          |  8 +++----
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/kernel-shark/src/libkshark-configio.c b/kernel-shark/src/libkshark-configio.c
index cac27a7..5d7323f 100644
--- a/kernel-shark/src/libkshark-configio.c
+++ b/kernel-shark/src/libkshark-configio.c
@@ -793,7 +793,7 @@ bool kshark_export_event_filter(struct tep_handle *pevent,
 	}
 }
 
-static bool kshark_event_filter_from_json(struct tep_handle *pevent,
+static int kshark_event_filter_from_json(struct tep_handle *pevent,
 					  struct tracecmd_filter_id *filter,
 					  const char *filter_name,
 					  struct json_object *jobj)
@@ -801,7 +801,7 @@ static bool kshark_event_filter_from_json(struct tep_handle *pevent,
 	json_object *jfilter, *jevent, *jsystem, *jname;
 	const char *system_str, *name_str;
 	struct tep_event *event;
-	int i, length;
+	int i, length, count = 0;
 
 	/*
 	 * Use the name of the filter to find the array of events associated
@@ -809,7 +809,7 @@ static bool kshark_event_filter_from_json(struct tep_handle *pevent,
 	 * contain no data for this particular filter.
 	 */
 	if (!json_object_object_get_ex(jobj, filter_name, &jfilter))
-		return false;
+		return 0;
 
 	if (!kshark_json_type_check(jobj, "kshark.config.filter") ||
 	    json_object_get_type(jfilter) != json_type_array)
@@ -829,16 +829,21 @@ static bool kshark_event_filter_from_json(struct tep_handle *pevent,
 
 		event = tep_find_event_by_name(pevent, system_str, name_str);
 		if (!event)
-			goto fail;
+			continue;
 
 		tracecmd_filter_id_add(filter, event->id);
+		++count;
 	}
 
-	return true;
+	if (count != length)
+		count = -count;
+
+	return count;
 
  fail:
 	fprintf(stderr, "Failed to load event filter from json_object.\n");
-	return false;
+	tracecmd_filter_id_clear(filter);
+	return 0;
 }
 
 /**
@@ -851,14 +856,14 @@ static bool kshark_event_filter_from_json(struct tep_handle *pevent,
  * @param conf: Input location for the kshark_config_doc instance. Currently
  *		only Json format is supported.
  *
- * @returns True, if a filter has been loaded. If the filter configuration
- *	    document contains no data for this particular filter or in a case
- *	    of an error, the function returns False.
+ * @returns The total number of events added to the filter. If not all events
+ *	    listed in the input configuration have been added successfully,
+ *	    the returned number is negative.
  */
-bool kshark_import_event_filter(struct tep_handle *pevent,
-				struct tracecmd_filter_id *filter,
-				const char *filter_name,
-				struct kshark_config_doc *conf)
+int kshark_import_event_filter(struct tep_handle *pevent,
+			       struct tracecmd_filter_id *filter,
+			       const char *filter_name,
+			       struct kshark_config_doc *conf)
 {
 	switch (conf->format) {
 	case KS_CONFIG_JSON:
@@ -869,7 +874,7 @@ bool kshark_import_event_filter(struct tep_handle *pevent,
 	default:
 		fprintf(stderr, "Document format %d not supported\n",
 			conf->format);
-		return false;
+		return 0;
 	}
 }
 
diff --git a/kernel-shark/src/libkshark.h b/kernel-shark/src/libkshark.h
index fe35333..04e9cbf 100644
--- a/kernel-shark/src/libkshark.h
+++ b/kernel-shark/src/libkshark.h
@@ -587,10 +587,10 @@ bool kshark_export_event_filter(struct tep_handle *pevent,
 				const char *filter_name,
 				struct kshark_config_doc *conf);
 
-bool kshark_import_event_filter(struct tep_handle *pevent,
-				struct tracecmd_filter_id *filter,
-				const char *filter_name,
-				struct kshark_config_doc *conf);
+int kshark_import_event_filter(struct tep_handle *pevent,
+			       struct tracecmd_filter_id *filter,
+			       const char *filter_name,
+			       struct kshark_config_doc *conf);
 
 bool kshark_export_user_mask(struct kshark_context *kshark_ctx,
 			     struct kshark_config_doc **conf);
-- 
2.20.1

[PATCH 2/4] kernel-shark: Allow KsCaptureControl widget to print text to the console

[Yordan Karadzhov (VMware)]

This will be used to show Warning messages.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark/src/KsCaptureDialog.cpp | 3 +++
 kernel-shark/src/KsCaptureDialog.hpp | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index 888ffea..6e526ff 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -437,6 +437,9 @@ void KsCaptureMonitor::connectMe(QProcess *proc, KsCaptureControl *ctrl)
 
 	connect(ctrl,	&KsCaptureControl::argsReady,
 		this,	&KsCaptureMonitor::_argsReady);
+
+	connect(ctrl,	&KsCaptureControl::print,
+		this,	&KsCaptureMonitor::print);
 }
 
 void KsCaptureMonitor::_captureStarted()
diff --git a/kernel-shark/src/KsCaptureDialog.hpp b/kernel-shark/src/KsCaptureDialog.hpp
index b168a2b..f8ddf4a 100644
--- a/kernel-shark/src/KsCaptureDialog.hpp
+++ b/kernel-shark/src/KsCaptureDialog.hpp
@@ -40,6 +40,12 @@ signals:
 	/** This signal is emitted when the "Apply" button is pressed. */
 	void argsReady(const QString &args);
 
+	/**
+	 * This signal is emitted when text has to be printed on the
+	 * KsCaptureMonitor widget.
+	 */
+	void print(const QString &message);
+
 private:
 	tep_handle		*_localTEP;
 
-- 
2.20.1

[PATCH 3/4] kernel-shark: Handle errors when loading Capture configurations

[Yordan Karadzhov (VMware)]

A configuration file (Json) for the Capture dialog can contain
non-existing tracer plugin or events. This can happen if the
configuration was exported on one machine and then imported on
another. In such a case the non-existing plugin/events will be
ignored and a warning message will be printed to the console-like
widget.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark/src/KsCaptureDialog.cpp | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index 6e526ff..02bbbbc 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -194,7 +194,7 @@ QStringList KsCaptureControl::_getPlugins()
 
 void KsCaptureControl::_importSettings()
 {
-	int nEvts = tep_get_events_count(_localTEP);
+	int nEvts = tep_get_events_count(_localTEP), nIds;
 	kshark_config_doc *conf, *jevents, *temp;
 	QVector<bool> v(nEvts, false);
 	tracecmd_filter_id *eventHash;
@@ -227,7 +227,14 @@ void KsCaptureControl::_importSettings()
 		return;
 
 	eventHash = tracecmd_filter_id_hash_alloc();
-	kshark_import_event_filter(_localTEP, eventHash, "Events", jevents);
+	nIds = kshark_import_event_filter(_localTEP, eventHash, "Events", jevents);
+	if (nIds < 0) {
+		QString err("WARNING: ");
+		err += "Some of the imported events are not available on this system.\n";
+		err += "All missing events are ignored.\n";
+		emit print(err);
+	}
+
 	for (int i = 0; i < nEvts; ++i) {
 		if (tracecmd_filter_id_find(eventHash, events[i]->id))
 			v[i] = true;
@@ -239,8 +246,19 @@ void KsCaptureControl::_importSettings()
 	/** Get all available plugins. */
 	temp = kshark_string_config_alloc();
 
-	if (kshark_config_doc_get(conf, "Plugin", temp))
-		_pluginsComboBox.setCurrentText(KS_C_STR_CAST(temp->conf_doc));
+	if (kshark_config_doc_get(conf, "Plugin", temp)) {
+		const char *plugin = KS_C_STR_CAST(temp->conf_doc);
+		int pluginIndex = _pluginsComboBox.findText(plugin);
+
+		if (pluginIndex >= 0) {
+			_pluginsComboBox.setCurrentText(KS_C_STR_CAST(temp->conf_doc));
+		} else {
+			QString err("WARNING: The traceer plugin \"");
+			err += plugin;
+			err += "\" is not available on this machine\n";
+			emit print(err);
+		}
+	}
 
 	if (kshark_config_doc_get(conf, "Output", temp))
 		_outputLineEdit.setText(KS_C_STR_CAST(temp->conf_doc));
-- 
2.20.1

[PATCH 4/4] kernel-shark: Handle corrupted configuration file for the Capture dialog

[Yordan Karadzhov (VMware)]

Error message will be printed to the console-like widget.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark/src/KsCaptureDialog.cpp | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/kernel-shark/src/KsCaptureDialog.cpp b/kernel-shark/src/KsCaptureDialog.cpp
index 02bbbbc..addd41d 100644
--- a/kernel-shark/src/KsCaptureDialog.cpp
+++ b/kernel-shark/src/KsCaptureDialog.cpp
@@ -201,6 +201,9 @@ void KsCaptureControl::_importSettings()
 	tep_event **events;
 	QString fileName;
 
+	auto lamImportError = [this] () {
+		emit print("ERROR: Unable to load the configuration file.\n");
+	};
 
 	/** Get all available events. */
 	events = tep_list_events(_localTEP, TEP_EVENT_SORT_SYSTEM);
@@ -210,21 +213,27 @@ void KsCaptureControl::_importSettings()
 				    "Kernel Shark Config files (*.json);;",
 				    _lastFilePath);
 
-	if (fileName.isEmpty())
+	if (fileName.isEmpty()) {
+		lamImportError();
 		return;
+	}
 
 	conf = kshark_open_config_file(fileName.toStdString().c_str(),
 				       "kshark.config.record");
-	if (!conf)
+	if (!conf) {
+		lamImportError();
 		return;
+	}
 
 	/*
 	 * Load the hash table of selected events from the configuration
 	 * document.
 	 */
 	jevents = kshark_config_alloc(KS_CONFIG_JSON);
-	if (!kshark_config_doc_get(conf, "Events", jevents))
+	if (!kshark_config_doc_get(conf, "Events", jevents)) {
+		lamImportError();
 		return;
+	}
 
 	eventHash = tracecmd_filter_id_hash_alloc();
 	nIds = kshark_import_event_filter(_localTEP, eventHash, "Events", jevents);
-- 
2.20.1