From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16415 invoked by alias); 8 Apr 2010 15:47:15 -0000 Received: (qmail 16402 invoked by uid 22791); 8 Apr 2010 15:47:14 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_DP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 08 Apr 2010 15:47:08 +0000 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o38Fl5oN004271 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 8 Apr 2010 11:47:05 -0400 Received: from fche.csb (vpn-235-68.phx2.redhat.com [10.3.235.68]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o38Fl4cM010817; Thu, 8 Apr 2010 11:47:05 -0400 Received: by fche.csb (Postfix, from userid 2569) id 3507A58132; Thu, 8 Apr 2010 11:47:04 -0400 (EDT) To: Anithra P Janakiraman Cc: systemtap@sourceware.org Subject: Re: [RFC] Framework for easy distribution of SystemTap scripts. References: <4B1FE14D.5070307@linux.vnet.ibm.com> <4B685611.5090402@linux.vnet.ibm.com> <4BA7D773.5050106@linux.vnet.ibm.com> <4BBC4EA1.9040608@linux.vnet.ibm.com> From: fche@redhat.com (Frank Ch. Eigler) Date: Thu, 08 Apr 2010 15:47:00 -0000 In-Reply-To: <4BBC4EA1.9040608@linux.vnet.ibm.com> (Anithra P. Janakiraman's message of "Wed, 07 Apr 2010 14:51:37 +0530") Message-ID: User-Agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org X-SW-Source: 2010-q2/txt/msg00064.txt.bz2 anithra wrote: > diff --git a/scripts/build_pkg/README.stap-buildpkg b/scripts/build_pkg/README.stap-buildpkg > new file mode 100644 > index 0000000..d87a3de > --- /dev/null > +++ b/scripts/build_pkg/README.stap-buildpkg Could this be a normal man page (stap-buildpkg.1) ? > +stap-buildpkg scriptfile [options] [stap-options] > +Options: > + -n pkg_name : specify a package name > + -v pkg_version : specify a package version > + -c pkg_config : provide config parameters through a file > + -p post_process script : post processing script > + -h help_config : help text for config parameters > + All additonal options will be passed to the stap command > +pkg_name and pkg_version will be used to set the name and version > +of the resulting executable. Why does there need to be a separate 'version'? > +An optional config file that contains script specific config > +options and corresponding help entries can also be specified using > +the -c and -h options respectively. You need to elaborate what script-specific config options can be, what they do, how they work. > +A post processing script can be provided using the -p option. You should specify what exactly this means. A '/bin/sh -c' command line to pipe stdout through? > [...] > + [options] [parameters] > + > +Options: > + * --run -r Runs the scripts > + for n minutes where n can be passed as a parameter. The default > + value is 10 minutes. Post processing is performed after the > + script completes. > + * --start -s Invokes the script as a background process. > + * --stop -x Stops the script and performs post processing. > + * --all Same as --run > + * --help Displays this usage text. > + > +Parameters: > + > + * time=[x] x is in minutes. Runs the script for x minutes. valid for > + --run(-r) o --start(-s) or --all(-a) options only How does time=NNN relate to -r 'n'? (We could add a stap and/or staprun option for a simple time-based exit().) > diff --git a/scripts/build_pkg/stap-buildpkg b/scripts/build_pkg/stap-buildpkg > [...] > +#Checking if script is run by root > +if [ $EUID -ne 0 ] > +then > + echo "ERROR :You need to be root to run this !" > + exit > +fi Why? > +prog=stap-buildpkg > + > +echo_usage () { > + echo $"Usage: $prog scriptfile [options] [stap-options]" > + echo $"Options:" > + echo $" -n pkg_name : specify a package name" > + echo $" -v pkg_version : specify a package version" > + echo $" -c pkg_config : provide config parameters through a file" > + echo $" -p post_process script : post processing script" > + echo $" -h help_config : help text for config parameters" > + echo $" All additonal options will be passed to the stap command" > +} > + > +parse_args() { > +while [ -n "$1" ]; do > + case "$1" in > + [...] > + *) STAP_OPTIONS=$@ > + echo $STAP_OPTIONS You'll need to watch the quoting in these $foo variables. They could contain spaces etc. > +module_gen() { > +stap -p4 -k -m $PKG_NAME.ko $script $STAP_OPTIONS > +if [ -f $PKG_NAME.ko ] > +then > + echo " Module $PKG_NAME.ko generated successfully "; > +else > + echo " Module $PKG_NAME.ko generation failed. Exiting now... " > + rm -rf $TEMPDIR > + exit; > +fi > +} Why are you running stap with '-k'? If you're only saving the .ko result, then let stap give it to you in the present working directory. > [...] > +echo "using systemtap runtime executables from $runtime rpm" > +cp `rpm -ql $runtime | grep staprun | grep -v staprun.8.gz` $TEMPDIR/. > +cp `rpm -ql $runtime | grep stapio` $TEMPDIR/. Have you observed systemtap being installed as a relocatable rpm somewhere other than /usr/* ? If not, you could just copy /usr/bin/staprun etc. by normal path name. > [...] > +if [ ! -f "$script" ]; then > + echo "Script file does not exist" > + echo "Please provide a valid script name" > + echo_usage > + exit > +fi (Could a user not specify the script contents with stap-foo -e 'SCRIPT' ?) > [...] > +#Cleanup the mess in temp dir > +#rm -rf $TEMPDIR I assume not actually cleaning up the temporary directory is temporary. > diff --git a/scripts/build_pkg/template.buildpkg b/scripts/build_pkg/template.buildpkg > [...] FWIW, you could include this template within the main buildpkg script, as in echo > $PKG << 'END' function run() .... END > @@ -0,0 +1,153 @@ > +#!/bin/bash > +SCRIPT_LOC=/usr/tapsetrpms/TEMPLATE_PKG_NAME-TEMPLATE_PKG_VERSION What is SCRIPT_LOC for? > [...] > +$RUN_DIR/staprun $RUN_DIR/TEMPLATE_PKG_NAME.ko > $RUN_DIR/TEMPLATE_PKG_NAME.out 2>&1 & > +sleep $time > +echo "Completed" > +mv $RUN_DIR/TEMPLATE_PKG_NAME.out $RUN_DIR/TEMPLATE_PKG_NAME_${TS}.out > +echo "Output file TEMPLATE_PKG_NAME_${TS}.out is in $RUN_DIR directory" (Is all this echoing necessary verbiage?) > +ps -ef | grep TEMPLATE_PKG_NAME | grep stapio | awk '{print $2}' | xargs kill -SIGINT You wouldn't need this is you stored $! after the "staprun ... &" a few lines ago. > +function helptext() > [...] > +# echo " ipaddress=[ip] ip is a standard IPv4 address. The output will be filtered for this address" Really? Perhaps the framework-maker should have a filter-command option such as -P 'grep 127.0.0.1' that it could pass through. > + FILE=$RUN_DIR/TEMPLATE_PKG_NAME.config > + # make sure file exist and readable > + if [ ! -f $FILE ]; then > + echo "$FILE : does not exist" > + elif [ ! -r $FILE ]; then > + echo "$FILE: cannot read" > + fi Why does this template need to exist? The framework maker script could transcribe all of the options right into the output script. - FChE