@tohojo requested changes on this pull request.
Please add a paragraph of text to the commit message describing why the feature
is useful.
A few nits on the code, but otherwise looks good! :)
> # Sanity check; is /dev/urandom readable? If so, use it to
# pre-fill netperf's buffers
- self.run_simple(['dd', 'if=/dev/urandom', 'of=/dev/null',
'bs=1', 'count=1'], errmsg="Err")
- netperf['buffer'] = '-F /dev/urandom'
+ fillFile = args['test_payload']
+ self.run_simple(['dd', f'if={fillFile}', 'of=/dev/null',
'bs=1', 'count=1'], errmsg="Err")
+ netperf['buffer'] = f'-F {fillFile}'
Flent runs on Python versions as old as 3.5, so we can't use f-strings, sadly...
> @@ -1022,10 +1023,14 @@ def check(self):
netperf['-e'] = True
try:
+ # If --test-payload option is specified, use data from that
file
+ # else use the default value /dev/urandom.
+ # Below sanity check will be performed in either case.
I'm not sure if the sanity check makes sense for a user-specified file: it
means we'll just silently ignore the option if the file is not readable. I
think it's better to fail noisily. We could do this either by doing the check
but erroring out if it fails (it's the default), or we could skip the test for
custom values and just let netperf fail when it can't open it.
> @@ -378,6 +378,12 @@ def __call__(self, parser, namespace, values,
> option_string=None):
"to decrease the socket buffer size. Can be specified multiple times, "
"with each value corresponding to a stream of a test.")
+test_group.add_argument(
+ "--test-payload",
+ action="store", type=unicode, dest="TEST_PAYLOAD", default='/dev/urandom',
+ help="Path to file containing payload to pre-fill the netperf buffers with"
Missing space at the end of the first string here
> # Sanity check; is /dev/urandom readable? If so, use it to
# pre-fill netperf's buffers
- self.run_simple(['dd', 'if=/dev/urandom', 'of=/dev/null',
'bs=1', 'count=1'], errmsg="Err")
- netperf['buffer'] = '-F /dev/urandom'
+ fillFile = args['test_payload']
Variables are generally lowercase and underscore-separated rather then
camelCase...
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/tohojo/flent/pull/243#pullrequestreview-783839744_______________________________________________
Flent-users mailing list -- flent-users@flent.org
To unsubscribe send an email to flent-users-le...@flent.org
