[ltt-dev] [PATCH v2] Remove ltt-armall/disarmall's unwanted error messages

[ltt-dev] [PATCH v2] Remove ltt-armall/disarmall's unwanted error messages

[Zhaolei]

Hi Mathieu,

> Hrm, is it possible to make the write to /proc/ltt return something else
> that would indicate that the marker is already connected and use that
> instead ? Otherwise, this can be problematic if two instances of
> ltt-armall are executed in parallel.
> 
Thanks for your reply, I rewrote the following patch based on your advice.

Signed-off-by: Zhaolei <zhaolei at cn.fujitsu.com>
---
diff -Nur ltt-control-0.54-10102008.org/lttctl/ltt-armall.sh ltt-control-0.54-10102008/lttctl/ltt-armall.sh
--- ltt-control-0.54-10102008.org/lttctl/ltt-armall.sh	2008-10-15 15:29:54.000000000 +0800
+++ ltt-control-0.54-10102008/lttctl/ltt-armall.sh	2008-10-17 11:14:20.000000000 +0800
@@ -5,7 +5,7 @@
 MARKERS=`cat /proc/ltt|grep -v %k|awk '{print $2}'|sort -u|grep -v ^core_|grep -v ^locking_|grep -v ^lockdep_`
 
 for a in $MARKERS; do
-	echo Connecting $a
+	echo -n "  Connecting $a: "
 
 	#redirect markers carrying state information to dedicated channels
 	case $a in
@@ -26,7 +26,20 @@
 		;;
 	esac
 
-	echo "connect $a default dynamic $CHANNEL" > /proc/ltt
+	err=`LANG=C;echo "connect $a default dynamic $CHANNEL" 2>&1 >/proc/ltt`
+	if [ -z "$err" ]
+	then
+		echo "OK"
+		continue
+	fi
+
+	if echo "$err" | grep -q "write error: File exists$"
+	then
+		echo "Already connected"
+		continue
+	fi
+
+	echo "$err"
 done
 
 
@@ -39,7 +52,7 @@
 #MARKERS=`cat /proc/ltt|grep -v %k|awk '{print $2}'|sort -u|grep -e ^tap_ -e ^lockdep`
 
 for a in $MARKERS; do
-	echo Connecting $a
+	echo -n "  Connecting $a: "
 
 	#redirect markers carrying state information to dedicated channels
 	case $a in
@@ -48,5 +61,18 @@
 		;;
 	esac
 
-	echo "connect $a ltt_tap_marker dynamic $CHANNEL" > /proc/ltt
+	err=`LANG=C;echo "connect $a ltt_tap_marker dynamic $CHANNEL" 2>&1 >/proc/ltt`
+	if [ -z "$err" ]
+	then
+		echo "OK"
+		continue
+	fi
+
+	if echo "$err" | grep -q "write error: File exists$"
+	then
+		echo "Already connected"
+		continue
+	fi
+
+	echo "$err"
 done
diff -Nur ltt-control-0.54-10102008.org/lttctl/ltt-disarmall.sh ltt-control-0.54-10102008/lttctl/ltt-disarmall.sh
--- ltt-control-0.54-10102008.org/lttctl/ltt-disarmall.sh	2008-10-15 15:29:54.000000000 +0800
+++ ltt-control-0.54-10102008/lttctl/ltt-disarmall.sh	2008-10-17 11:14:43.000000000 +0800
@@ -2,7 +2,26 @@
 #excluding core markers, not connected to default.
 echo Disconnecting all markers
 MARKERS=`cat /proc/ltt|grep -v %k|awk '{print $2}'|sort -u|grep -v ^core_|grep -v ^locking_|grep -v ^lockdep_|grep -v ^lockdep|grep -v ^tap_`
-for a in $MARKERS; do echo Disconnecting $a; echo "disconnect $a" > /proc/ltt; done
+for a in $MARKERS
+do
+	echo -n "  Disconnecting $a: "
+
+	err=`LANG=C;echo "disconnect $a" 2>&1 >/proc/ltt`
+
+	if [ -z "$err" ]
+	then
+		echo "OK"
+		continue
+	fi
+
+	if echo "$err" | grep -q "write error: No such file or directory$"
+	then
+		echo "Already disconnected"
+		continue
+	fi
+
+	echo "$err"
+done
 
 # Markers starting with "tap_" are considered high-speed.
 echo Disconnecting high-rate markers to tap
@@ -11,8 +30,23 @@
 #Uncomment the following to also stop recording lockdep events.
 #MARKERS=`cat /proc/ltt|grep -v %k|awk '{print $2}'|sort -u|grep -e ^tap_ -e ^lockdep`
 
-for a in $MARKERS; do
-	echo Disconnecting $a
+for a in $MARKERS
+do
+	echo -n "  Disconnecting $a: "
+
+	err=`LANG=C;echo "disconnect $a ltt_tap_marker" 2>&1 >/proc/ltt`
+
+	if [ -z "$err" ]
+	then
+		echo "OK"
+		continue
+	fi
+
+	if echo "$err" | grep -q "write error: No such file or directory$"
+	then
+		echo "Already disconnected"
+		continue
+	fi
 
-	echo "disconnect $a ltt_tap_marker" > /proc/ltt
+	echo "$err"
 done

[ltt-dev] [PATCH v2] Remove ltt-armall/disarmall's unwanted errormessages

[Zhaolei]

Hi, Mathieu

> How about :
> 
> echo "connect $a default dynamic $CHANNEL" 2>&1 >/proc/ltt
> 
> and then use $?  as error value ?
> 
> (try issuing "echo $?" just after the failing command)
Actually, I considered about return value, but if other kind of error
happened, I think it is better to show errormsg to user.
If we judge on return value, user will see "Already connect" if other
 error happened.

Judge on "write error: File exists" maybe little iffy, but on worst case,
it will show whole errormessage to user(maybe the best choose).

Thanks!
> 
> Maybe we could also get rid of the "echo: write error: File exists" by
> returning a more appropriate error value from the write system call ?


> 
> Mathieu
> 

[ltt-dev] [PATCH v2] Remove ltt-armall/disarmall's unwanted errormessages

[Zhaolei]

Mathieu wrote:
>> > How about :
>> > 
>> > echo "connect $a default dynamic $CHANNEL" 2>&1 >/proc/ltt
>> > 
>> > and then use $?  as error value ?
>> > 
>> > (try issuing "echo $?" just after the failing command)
>> Actually, I considered about return value, but if other kind of error
>> happened, I think it is better to show errormsg to user.
>> If we judge on return value, user will see "Already connect" if other
>>  error happened.
>> 
>> Judge on "write error: File exists" maybe little iffy, but on worst case,
>> it will show whole errormessage to user(maybe the best choose).
>> 
> 
> Can you have a quick look at other debugfs, sysfs/procfs files to see
> how they handle this kind of situation ? I feel using the text returned
> as error value is not very standard.
> 
I tried following commands to see how other proc-based control doing:
  #echo AA >/proc/sys/kernel/printk
  -bash: echo: write error: Invalid argument
  # echo AA > /proc/sys/net/ipv4/ip_forward
  -bash: echo: write error: Invalid argument
They are same as lttng when user's input is wrong.
So I think lttng's /proc implement is ok.

But I think it may be difficult to get return value of sys_write in shell-script.
(We can only see $?=1 on any kind of error)
And I haven't find other shell-scripts who handle this kind of situation.

Sorry, but until now, I think we have following choice:
1: use errout string
2: write ltt-armall in C to check return value.

Thanks!
> Thanks,
> 
> Mathieu
> 
>> Thanks!
>> > 
>> > Maybe we could also get rid of the "echo: write error: File exists" by
>> > returning a more appropriate error value from the write system call ?
>> 
>> 
>> > 
>> > Mathieu
>> > 
>> 
> 
> -- 
> Mathieu Desnoyers
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
> 
>