[Ocfs2-test-devel] [PATCH 3/3] Splice had a limit of 1000 bytes to write through the pipe.

tristan.ye tristan.ye at oracle.com
Tue Oct 28 22:56:56 PDT 2008


I've tried your script on my testing environment,that's really a cool
tool,flexbile and tunable arguments, detailed log file and a clear
testing and checking mechanism,quite esay to use:)

Just some suggestions among following code fragment, the rest part looks
great to me.

On Tue, 2008-10-28 at 20:00 -0700, Marcos Matsunaga wrote:
> I've increased the size to 10,000,000 bytes to make it more flexible.
> 
> Also, made the splice lenght to be printed to stderr instead of
> stdout. This is to make the splice_read work as it was piping everything
> and causing error in the splice_test.py script that is being added here.
> 
> Makefile was changed to install splice_test.py
> 
> Signed-off-by: Marcos Matsunaga <Marcos.Matsunaga at oracle.com>
> ---
>  programs/splice/Makefile       |    4 +-
>  programs/splice/splice_read.c  |    6 +-
>  programs/splice/splice_test.py |  225 ++++++++++++++++++++++++++++++++++++++++
>  programs/splice/splice_write.c |    6 +-
>  4 files changed, 234 insertions(+), 7 deletions(-)
>  create mode 100644 programs/splice/splice_test.py
> 
> diff --git a/programs/splice/Makefile b/programs/splice/Makefile
> index 6a0bd41..dadbdf6 100644
> --- a/programs/splice/Makefile
> +++ b/programs/splice/Makefile
> @@ -13,10 +13,12 @@ SPLICE_WRITE_OBJECTS = $(patsubst %.c,%.o,$(SPLICE_WRITE_SOURCES))
>  
>  SOURCES = $(SPLICE_WRITE_SOURCES) $(SPLICE_READ_SOURCES)
>  
> -DIST_FILES = $(SOURCES)
> +DIST_FILES = $(SOURCES) splice_test.py
>  
>  BIN_PROGRAMS = splice_read splice_write
>  
> +BIN_EXTRA = splice_test.py
> +
>  splice_read: $(SPLICE_READ_OBJECTS)
>  	$(LINK) 
>  splice_write: $(SPLICE_WRITE_OBJECTS)
> diff --git a/programs/splice/splice_read.c b/programs/splice/splice_read.c
> index ad4af0f..c25522e 100644
> --- a/programs/splice/splice_read.c
> +++ b/programs/splice/splice_read.c
> @@ -16,11 +16,11 @@ int main(int argc, char *argv[])
>  		printf("open file failed.\n");
>  		exit(-1);
>  	}
> -	slen = splice(fd, NULL, STDOUT_FILENO, NULL, 1000, 0);
> +	slen = splice(fd, NULL, STDOUT_FILENO, NULL, 10000000, 0);
>  	if (slen < 0)
> -		printf("splice failed.\n");
> +		fprintf(stderr, "splice failed.\n");
>  	else
> -		printf("spliced length = %d\n",slen);
> +		fprintf(stderr, "spliced length = %d\n",slen);
>  	close(fd);
>  	if (slen <0)
>  		exit(-1);
> diff --git a/programs/splice/splice_test.py b/programs/splice/splice_test.py
> new file mode 100644
> index 0000000..45b9374
> --- /dev/null
> +++ b/programs/splice/splice_test.py
> @@ -0,0 +1,225 @@
> +#!/usr/bin/env python
> +#
> +# * vim: noexpandtab sw=8 ts=8 sts=0:
> +# *
> +# * splice_test.py
> +# *
> +# * Test splice on ocfs2.
> +# *
> +# * Copyright (C) 2004, 2008 Oracle.  All rights reserved.
> +# *
> +# * This program is free software; you can redistribute it and/or
> +# * modify it under the terms of the GNU General Public
> +# * License version 2 as published by the Free Software Foundation.
> +# *
> +# * This program is distributed in the hope that it will be useful,
> +# * but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# * General Public License for more details.
> +#
> +#
> +#	 
> +# Program	: splice_test.py
> +# Description 	: This program will just test the splice feature on ocfs2.
> +# Author	: Marcos E. Matsunaga 
> +
> +#
> +import os, sys, time, optparse, socket, string, o2tf, config, pdb, timing
> +import time, config
> +#
> +MD5SUM = '/usr/bin/md5sum'
> +#
> +DEBUGON = os.getenv('DEBUG',0)
> +PID = os.getpid()
> +node_dirs = {}
> +#
> +logfile = config.LOGFILE
> +filename = 'splice_test.dat'
will be more convenient to set a default directory value here?
such as directory='.'
> +SPLICEWRITE_BIN = config.BINDIR + '/splice_write'
> +SPLICEREAD_BIN = config.BINDIR + '/splice_read'
> +count = 5
> +uname = os.uname()
> +lhostname = str(socket.gethostname())
> +stagedir = ''
the variable stagedir never used later:)
> +BASEMD5SUM=''
> +TEMPMD5SUM=''
> +#
> +# FUNCTIONS
> +#
> +def CreateBaseFile():
> +	from commands import getoutput
> +	global BASEMD5SUM
> +	os.system('ps -efl > %s' % os.path.join('/tmp', filename))
> +	BASEMD5SUM=getoutput('%s %s|cut -f1 -d" "' % \
> +		(MD5SUM, os.path.join('/tmp', filename)))
> +	if DEBUGON:
> +		o2tf.printlog('splice_test: Running CreateBaseFile',
> +			logfile, 0, '')
> +		o2tf.printlog('splice_test: BASEMD5SUM %s' % BASEMD5SUM,
> +			logfile, 0, '')
> +def SpliceWrite():
> +	from commands import getoutput
> +	print('Testing splice_write ........')
> +	from os import access, F_OK
> +	if access(os.path.join(directory, filename), F_OK) ==1:
> +		os.system('rm -fr ' + os.path.join(directory, filename))
> +	os.system('cat %s | %s %s' % (os.path.join('/tmp', filename), \
> +		SPLICEWRITE_BIN, os.path.join(directory, filename)))
> +	TEMPMD5SUM=getoutput('%s %s|cut -f1 -d" "' % \
> +		(MD5SUM, os.path.join(directory, filename)))
> +	if BASEMD5SUM == TEMPMD5SUM:
> +		print('PASSED')
> +	else:
> +		print('FAILED')
> +		o2tf.printlog('splice_test: BASEMD5SUM %s' % BASEMD5SUM,
> +			logfile, 0, '')
> +		o2tf.printlog('splice_test: TEMPMD5SUM %s' % TEMPMD5SUM,
> +			logfile, 0, '')
> +def SpliceRead():
> +	from commands import getoutput
> +	from os import access, F_OK
> +	print('Testing splice_read ........')
> +	if access(os.path.join(directory, filename), F_OK) ==1:
> +		os.system('rm -fr ' + os.path.join(directory, filename))
> +	os.system('%s %s | cat > %s' % (SPLICEREAD_BIN, \
> +		os.path.join('/tmp', filename), \
> +		os.path.join(directory, filename)))
> +	TEMPMD5SUM=getoutput('%s %s|cut -f1 -d" "' % \
> +		(MD5SUM, os.path.join(directory, filename)))
> +	if BASEMD5SUM == TEMPMD5SUM:
> +		print('PASSED')
		here we can return a RC with 0 to indicate a success of read operation
> +	else:
> +		print('FAILED')
> +		o2tf.printlog('splice_test: BASEMD5SUM %s' % BASEMD5SUM,
> +			logfile, 0, '')
> +		o2tf.printlog('splice_test: TEMPMD5SUM %s' % TEMPMD5SUM,
> +			logfile, 0, '')
		here we can return a RC with -1 to indicate a failure of read
operation,
		so the same situation with other funcs:)

> +def CheckFile():
> +	global tmpfile, directory, filename
> +	from commands import getoutput
> +	from os import access, remove, F_OK
> +	DestFname = '%s_%s' % (lhostname,filename)
> +	tmpchk=getoutput('%s %s |cut -f1 -d" "' % \
> +		(MD5SUM, tmpfile))
> +	shrchk=getoutput('%s %s |cut -f1 -d" "' % \
> +		(MD5SUM,os.path.join(directory, DestFname)))
> +	if DEBUGON:
> +		print 'DestFname = %s' % DestFname
> +		print 'tmpfile md5sum =  %s' % tmpchk
> +		print 'shared file md5sum =  %s' % shrchk
> +	if tmpchk != shrchk:
> +		o2tf.printlog('splice_test:CheckFile: md5sum mismatch',
> +			logfile, 0, '')
> +		o2tf.printlog('splice_test:CheckFile: tmpfile md5sum %s' % \
> +			tmpchk, logfile, 0, '')
> +		o2tf.printlog('splice_test:CheckFile: shared file md5sum %s' % \
> +			shrchk, logfile, 0, '')
> +	FDT = open(tmpfile, 'r', 0)
> +	FDS = open(os.path.join(directory, DestFname), 'r', 0)
> +	i=1
> +	while True:
> +		LineT = FDT.readline()
> +		LineS = FDS.readline()
> +		if not LineT and not LineS: break
> +		elif not LineT and LineS: 
> +			o2tf.printlog('splice_test:CheckFile: Premature end of \
> +				temporary file %s' % tmpfile,
> +				logfile, 0, '')
> +			break
> +		elif LineT and not LineS: 
> +			o2tf.printlog('splice_test:CheckFile: Premature end of \
> +				shared file %s' % DestFname,
> +				logfile, 0, '')
> +			break
> +		if LineT != LineS:	
> +			o2tf.printlog('splice_test:CheckFile: Line %s from \
> +				tempfile does not match line %s from shared \
> +				file;' % (LineT, LineS),
> +				logfile, 0, '')
> +		else:
> +			o2tf.printlog('splice_test:CheckFile: Line %s match' % \
> +				i, logfile, 0, '')
> +			i=i+1
> +	FDT.close()
> +	FDS.close()
> +#
something comfused with func checkfile,it was never used later.and in
the func, DestFname seems meaningless since it also was never generated
during test:)
> +# MAIN
> +#
> +Usage = 'usage: %prog [-d|--directory directory] \
> +[-c| --count] \
> +[-D| --debug] \
> +[-f|--filename ] \
here should be  [-f|--filename filename] :)
> +[-l|-logfile logfilename] \
> +[-h|--help]'
> +if __name__=='__main__':
> +	parser = optparse.OptionParser(Usage)
> +#
> +	parser.add_option('-d',
> +		'--directory',
> +		dest='directory',
> +		type='string',
> +		help='Directory where the file will be created.')
> +#
> +	parser.add_option('-c',
> +		'--count',
> +		dest='count',
> +		type='int',
> +		help='Number of times the test will be repeated.')
> +#
> +        parser.add_option('-D',
> +                '--debug',
> +                action="store_true",
> +                dest='debug',
> +                default=False,
> +                help='Turn the debug option on. Default=False.')
> +#
> +	parser.add_option('-l',
> +		'--logfile',
> +		dest='logfile',
> +		type='string',
> +		help='Logfile used by the process.')
> +#
> +	parser.add_option('-f',
> +		'--filename',
> +		dest='filename',
> +		type='string',
> +		help='Test filename.')
> +#
> +	(options, args) = parser.parse_args()
> +	if len(args) != 0:
> +		parser.error('incorrect number of arguments')
> +#
> +        if options.debug:
> +                DEBUGON=1
> +#
> +	if options.filename:
> +		filename = options.filename
> +#
> +	if not options.directory:
> +		parser.error('Please specify working directory.')
	here, we not only need to check if the directory option is empty or
not,but also have to check if it is a real directory:),otherwise a fake
dir always lead to a testing failure as i met.

	so it can be replaced as,
	if not options.directory:
		parser.error('Please specify working directory.')
	else:
		if not isdir(options.directory):
			makedirs(options.directory, 0755)
		
> +	directory = options.directory
> +	if options.logfile:
> +		logfile = options.logfile+'_'+lhostname
> +	if options.count:
> +		count = options.count
> +#
> +# Make nproc at least the number of nodes
> +#
> +# End or arguments parsing
> +#
> +if DEBUGON:
> +	o2tf.printlog('splice_test: directory = (%s)' % directory,
> +		logfile, 0, '')
> +	o2tf.printlog('splice_test: filename = (%s)' % filename,
> +		logfile, 0, '')
> +	o2tf.printlog('splice_test: logfile = (%s)' % logfile,
> +		logfile, 0, '')
> +#
> +for i in range(count):
> +	CreateBaseFile()
> +	SpliceWrite()
> +	SpliceRead()
	with the return code of these funcs, we can break the for loop here:)
> +#
> +o2tf.printlog('splice_test: Job completed successfully',
> +	logfile, 3, '=')
here,seems we always print a successful log recored wether the test
failed or not.so i suggest we should interrupt the test wherever we hit
a error to keep an accident spot for diagnosis,that means the
CreatBaseFile,splicewrite and spliceread funcs should return a RC,when
error occurs,the for loop should be broken here.
> +sys.exit()

at last,i suggest add a cleanup() function to remove the temporary file
you've created during test,it should be called when all tests completed
successfully.

Tristan.

> diff --git a/programs/splice/splice_write.c b/programs/splice/splice_write.c
> index 5c92bbb..a4d8c1a 100644
> --- a/programs/splice/splice_write.c
> +++ b/programs/splice/splice_write.c
> @@ -15,11 +15,11 @@ int main(int argc, char *argv[])
>  		printf("open file failed.\n");
>  		exit(-1);
>  	}
> -	slen = splice(STDIN_FILENO, NULL, fd, NULL, 1000, 0);
> +	slen = splice(STDIN_FILENO, NULL, fd, NULL, 10000000, 0);
>  	if (slen < 0)
> -		printf("splice failed.\n");
> +		fprintf(stderr, "splice failed.\n");
>  	else
> -		printf("spliced length = %d\n",slen);
> +		fprintf(stderr, "spliced length = %d\n",slen);
>  	close(fd);
>  	if (slen < 0)
>  		exit(-1);




More information about the Ocfs2-test-devel mailing list