[PATCH 0/4] kernel-shark: The road to 1.0

[PATCH 0/4] kernel-shark: The road to 1.0

[Steven Rostedt]

Some fixes for getting to 1.0.

Steven Rostedt (VMware) (3):
      revert: "kernel-shark: Remove a duplicate error message"
      kernel-shark: Show the standard error of kshark-record
      kernel-shark: Use '<br>' for dialog and '\n' for console error messages

Yordan Karadzhov (VMware) (1):
      kernel-shark: Remove the "make install" suggestion for capture errors

----
 kernel-shark/src/KsMainWindow.cpp | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

[PATCH 1/4] revert: "kernel-shark: Remove a duplicate error message"

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

It turns out that if the kshark-record dialog does not show up for some
reason, then without this "duplicate" message, the user gets no message at
all to why the dialog did not appear.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel-shark/src/KsMainWindow.cpp | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 8826cf5cf8b2..198b410480a1 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -1154,9 +1154,35 @@ void KsMainWindow::_captureStarted()
 	_captureLocalServer.listen("KSCapture");
 }
 
+/**
+ * If the authorization could not be obtained because the user dismissed
+ * the authentication dialog (clicked Cancel), pkexec exits with a return
+ * value of 126.
+ */
+#define PKEXEC_DISMISS_RET	126
+
 void KsMainWindow::_captureFinished(int ret, QProcess::ExitStatus st)
 {
+	QProcess *capture = (QProcess *)sender();
+
 	_captureLocalServer.close();
+
+	if (ret == PKEXEC_DISMISS_RET) {
+		/*
+		 * Authorization could not be obtained because the user
+		 * dismissed the authentication dialog.
+		 */
+		return;
+	}
+
+	if (ret != 0 || st != QProcess::NormalExit) {
+		QString message = "Capture process failed:<br>";
+
+		message += capture->errorString();
+		message += "<br>Try doing:<br> sudo make install";
+
+		_error(message, "captureFinishedErr", false, false);
+	}
 }
 
 void KsMainWindow::_captureError(QProcess::ProcessError error)
-- 
2.20.1

[PATCH 2/4] kernel-shark: Remove the "make install" suggestion for capture errors

[Steven Rostedt]

From: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>

The message is not appropriate, although it is most likely that the
error occurs because the user didn't install KernelShark and it runs
the executable from its build location.

[ Steve: Although I hit this warning without needing a make install ;-) ]

Link: http://lore.kernel.org/linux-trace-devel/20190709155650.2345-7-y.karadz@gmail.com

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
[ Removed "duplicate" print of it as well ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel-shark/src/KsMainWindow.cpp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 198b410480a1..13b50795340c 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -1179,7 +1179,6 @@ void KsMainWindow::_captureFinished(int ret, QProcess::ExitStatus st)
 		QString message = "Capture process failed:<br>";
 
 		message += capture->errorString();
-		message += "<br>Try doing:<br> sudo make install";
 
 		_error(message, "captureFinishedErr", false, false);
 	}
@@ -1191,8 +1190,6 @@ void KsMainWindow::_captureError(QProcess::ProcessError error)
 	QString message = "Capture process failed:<br>";
 
 	message += capture->errorString();
-	message += "<br>Try doing:<br> sudo make install";
-
 	_error(message, "captureFinishedErr", false, false);
 }
 
-- 
2.20.1

[PATCH 3/4] kernel-shark: Show the standard error of kshark-record

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

When kshark-record fails, report the standard error which can be very useful
for debugging the issue that when wrong. For example, after removing the
kshark lib functions, instead of just having "Unknow error" print when
kshark-record dialog fails, I now get:

  /usr/local/bin/kshark-record: error while loading shared libraries: libkshark-gui.so.0.9.8: cannot open shared object file: No such file or directory

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel-shark/src/KsMainWindow.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 13b50795340c..54aa6d782ad1 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -1179,7 +1179,7 @@ void KsMainWindow::_captureFinished(int ret, QProcess::ExitStatus st)
 		QString message = "Capture process failed:<br>";
 
 		message += capture->errorString();
-
+		message += capture->readAllStandardError();
 		_error(message, "captureFinishedErr", false, false);
 	}
 }
@@ -1190,6 +1190,7 @@ void KsMainWindow::_captureError(QProcess::ProcessError error)
 	QString message = "Capture process failed:<br>";
 
 	message += capture->errorString();
+	message += capture->readAllStandardError();
 	_error(message, "captureFinishedErr", false, false);
 }
 
-- 
2.20.1

[PATCH 4/4] kernel-shark: Use <br> for dialog and \n for console error messages

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

The Qt dialog boxes require '<br>' to print a new line but consoles require
a '\n'. Instead of printing '<br>' to the console or '\n' to the dialog,
always have the '\n' turn into '<br>' for the dialog and all '<br>' turn
into '\n' for the console.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel-shark/src/KsMainWindow.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 54aa6d782ad1..29d44d9a9230 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -1028,10 +1028,12 @@ void KsMainWindow::loadDataFile(const QString& fileName)
 	}
 }
 
-void KsMainWindow::_error(const QString &text, const QString &errCode,
+void KsMainWindow::_error(const QString &mesg, const QString &errCode,
 			  bool resize, bool unloadPlugins)
 {
 	QErrorMessage *em = new QErrorMessage(this);
+	QString text = mesg;
+	QString html = mesg;
 
 	if (resize)
 		_resizeEmpty();
@@ -1039,8 +1041,11 @@ void KsMainWindow::_error(const QString &text, const QString &errCode,
 	if (unloadPlugins)
 		_plugins.unloadAll();
 
+	text.replace("<br>", "\n", Qt::CaseInsensitive);
+	html.replace("\n", "<br>", Qt::CaseInsensitive);
+
 	qCritical().noquote() << "ERROR: " << text;
-	em->showMessage(text, errCode);
+	em->showMessage(html, errCode);
 	em->exec();
 }
 
-- 
2.20.1

Re: [PATCH 0/4] kernel-shark: The road to 1.0

[Steven Rostedt]

On Wed, 10 Jul 2019 09:46:36 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Some fixes for getting to 1.0.
> 
> Steven Rostedt (VMware) (3):
>       revert: "kernel-shark: Remove a duplicate error message"
>       kernel-shark: Show the standard error of kshark-record
>       kernel-shark: Use '<br>' for dialog and '\n' for console error messages
> 
> Yordan Karadzhov (VMware) (1):
>       kernel-shark: Remove the "make install" suggestion for capture errors
> 

Hi Yordan,

Can you review these patches?

Thanks!

-- Steve

Re: [PATCH 4/4] kernel-shark: Use <br> for dialog and \n for console error messages

[Yordan Karadzhov (VMware)]

On 10.07.19 г. 16:46 ч., Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> The Qt dialog boxes require '<br>' to print a new line but consoles require
> a '\n'. Instead of printing '<br>' to the console or '\n' to the dialog,
> always have the '\n' turn into '<br>' for the dialog and all '<br>' turn
> into '\n' for the console.
> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>   kernel-shark/src/KsMainWindow.cpp | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
> index 54aa6d782ad1..29d44d9a9230 100644
> --- a/kernel-shark/src/KsMainWindow.cpp
> +++ b/kernel-shark/src/KsMainWindow.cpp
> @@ -1028,10 +1028,12 @@ void KsMainWindow::loadDataFile(const QString& fileName)
>   	}
>   }
>   
> -void KsMainWindow::_error(const QString &text, const QString &errCode,
> +void KsMainWindow::_error(const QString &mesg, const QString &errCode,
>   			  bool resize, bool unloadPlugins)
>   {
>   	QErrorMessage *em = new QErrorMessage(this);
> +	QString text = mesg;
> +	QString html = mesg;
>   
>   	if (resize)
>   		_resizeEmpty();
> @@ -1039,8 +1041,11 @@ void KsMainWindow::_error(const QString &text, const QString &errCode,
>   	if (unloadPlugins)
>   		_plugins.unloadAll();
>   
> +	text.replace("<br>", "\n", Qt::CaseInsensitive);
> +	html.replace("\n", "<br>", Qt::CaseInsensitive);
> +
>   	qCritical().noquote() << "ERROR: " << text;
> -	em->showMessage(text, errCode);
> +	em->showMessage(html, errCode);
>   	em->exec();
>   }
>   
> 

All patches look good to me.
Thanks!
Yordan

Reviewed-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>