[Ocfs2-test-devel] [PATCH 3/3] Splice had a limit of 1000 bytes to write through the pipe.
Marcos E. Matsunaga
Marcos.Matsunaga at oracle.com
Wed Oct 29 12:13:13 PDT 2008
See my comments below. I fixed the stuff and will be sending it again
shortly.
Regards,
Marcos Eduardo Matsunaga
Oracle USA
Linux Engineering
“The statements and opinions expressed here are my own and do not
necessarily represent those of Oracle Corporation.”
tristan.ye wrote:
> 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='.'
>
No need here.. The directory is a mandatory parameter so, it will be
set anyway.
>> +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:)
>
Good.. I have removed.. I copy this script from another and didn't
finish the cleanup.
>> +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
>
Yeah.. did that.
>> + 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:)
>
Good catch.. I removed the function.
>> +# 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)
>
>
agree... Done.
>> + 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:)
>
Done.
>> +#
>> +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.
>
Yeah.. my bad. I think I'm doing that on almost all scripts.. Will fix it.
>> +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.
>
>
Done.
> 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);
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/ocfs2-test-devel/attachments/20081029/4127df8d/attachment.html
More information about the Ocfs2-test-devel
mailing list