[Fuego] [PATCH] fio/parser.py: Add support to parse later versions of Fio 2.16

[Fuego] [PATCH] fio/parser.py: Add support to parse later versions of Fio 2.16

[sireesha.nakkala]

From: sireesha <sireesha.nakkala@toshiba-tsip.com>

The existing parser.py file works for fio version 2.0.8 which is the
test version defined and used in fuego tests.

As Fio output has changed from 2.16 version, to use fio with fuego
the parser is modified to parse the output of Fio version 2.0.8 and
later version of 2.16

Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
---
 tests/Benchmark.fio/parser.py | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)
 mode change 100755 => 100644 tests/Benchmark.fio/parser.py

diff --git a/tests/Benchmark.fio/parser.py b/tests/Benchmark.fio/parser.py
old mode 100755
new mode 100644
index 52a44e5..ef081b5
--- a/tests/Benchmark.fio/parser.py
+++ b/tests/Benchmark.fio/parser.py
@@ -4,13 +4,30 @@
 import os, sys
 import common as plib
 
-regex_string = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
+regex_string = ''
+regex_string_old = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
+regex_string_new = '( READ:| WRITE:)(.*)(bw=.*)(.*)(\()([\d.]+)([kKM]B\/s)(\))'
+
+test_version = ''
+with open(plib.TEST_LOG,'r') as cur_file:
+    raw_values = cur_file.readlines()
+    test_version = raw_values[4].rstrip("\n")
+
+if test_version > '2.16':
+    regex_string = regex_string_new
+else:
+    regex_string = regex_string_old
+
 measurements = {}
 
 # handle results in MB or KB
 def set_measure(tguid, match):
-    speed = match[3]
-    units = match[4]
+    if test_version > '2.16':
+        speed = match[5]
+        units = match[6]
+    else:
+        speed = match[3]
+        units = match[4]
     multiplier = 1
     if units.startswith('M'):
         # WARNING - this depends on the value of kb_base specified to
-- 
2.20.1

Re: [Fuego] [PATCH] fio/parser.py: Add support to parse later versions of Fio 2.16

[sireesha.nakkala]

Dear Tim,

I would like to add below change related to Fio parser to Upstream. 

Kindly review and provide your feedback.


Thanks & Regards
Sireesha

-----Original Message-----
From: sireesha.nakkala@toshiba-tsip.com <sireesha.nakkala@toshiba-tsip.com> 
Sent: Wednesday, March 30, 2022 6:48 PM
To: tim.bird@sony.com
Cc: nakkala sireesha(ＴＳＩＰ) <sireesha.nakkala@toshiba-tsip.com>; fuego@lists.linuxfoundation.org; dinesh kumar(ＴＳＩＰ) <dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □ＳＷＣ◯ＡＣＴ) <kazuhiro3.hayashi@toshiba.co.jp>; pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>
Subject: [PATCH] fio/parser.py: Add support to parse later versions of Fio 2.16

From: sireesha <sireesha.nakkala@toshiba-tsip.com>

The existing parser.py file works for fio version 2.0.8 which is the test version defined and used in fuego tests.

As Fio output has changed from 2.16 version, to use fio with fuego the parser is modified to parse the output of Fio version 2.0.8 and later version of 2.16

Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
---
 tests/Benchmark.fio/parser.py | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)  mode change 100755 => 100644 tests/Benchmark.fio/parser.py

diff --git a/tests/Benchmark.fio/parser.py b/tests/Benchmark.fio/parser.py old mode 100755 new mode 100644 index 52a44e5..ef081b5
--- a/tests/Benchmark.fio/parser.py
+++ b/tests/Benchmark.fio/parser.py
@@ -4,13 +4,30 @@
 import os, sys
 import common as plib
 
-regex_string = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
+regex_string = ''
+regex_string_old = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
+regex_string_new = '( READ:| WRITE:)(.*)(bw=.*)(.*)(\()([\d.]+)([kKM]B\/s)(\))'
+
+test_version = ''
+with open(plib.TEST_LOG,'r') as cur_file:
+    raw_values = cur_file.readlines()
+    test_version = raw_values[4].rstrip("\n")
+
+if test_version > '2.16':
+    regex_string = regex_string_new
+else:
+    regex_string = regex_string_old
+
 measurements = {}
 
 # handle results in MB or KB
 def set_measure(tguid, match):
-    speed = match[3]
-    units = match[4]
+    if test_version > '2.16':
+        speed = match[5]
+        units = match[6]
+    else:
+        speed = match[3]
+        units = match[4]
     multiplier = 1
     if units.startswith('M'):
         # WARNING - this depends on the value of kb_base specified to
--
2.20.1

Re: [Fuego] [PATCH] fio/parser.py: Add support to parse later versions of Fio 2.16

[Bird, Tim]

Thanks for this submission.  I have a few questions inline below.

> -----Original Message-----
> From: sireesha.nakkala@toshiba-tsip.com <sireesha.nakkala@toshiba-tsip.com>
> Subject: RE: [PATCH] fio/parser.py: Add support to parse later versions of Fio 2.16
> 
> 
> Dear Tim,
> 
> I would like to add below change related to Fio parser to Upstream.
> 
> Kindly review and provide your feedback.
> 
> 
> Thanks & Regards
> Sireesha
> 
> -----Original Message-----
> From: sireesha.nakkala@toshiba-tsip.com <sireesha.nakkala@toshiba-tsip.com>
> Sent: Wednesday, March 30, 2022 6:48 PM
> To: tim.bird@sony.com
> Cc: nakkala sireesha(ＴＳＩＰ) <sireesha.nakkala@toshiba-tsip.com>; fuego@lists.linuxfoundation.org; dinesh kumar(ＴＳＩＰ)
> <dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □ＳＷＣ◯ＡＣＴ) <kazuhiro3.hayashi@toshiba.co.jp>; pyla venkata(ＴＳＩ
> Ｐ) <Venkata.Pyla@toshiba-tsip.com>
> Subject: [PATCH] fio/parser.py: Add support to parse later versions of Fio 2.16
> 
> From: sireesha <sireesha.nakkala@toshiba-tsip.com>
> 
> The existing parser.py file works for fio version 2.0.8 which is the test version defined and used in fuego tests.
> 
> As Fio output has changed from 2.16 version, to use fio with fuego the parser is modified to parse the output of Fio version 2.0.8 and later
> version of 2.16
> 
> Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> ---
>  tests/Benchmark.fio/parser.py | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)  mode change 100755 => 100644 tests/Benchmark.fio/parser.py
> 
> diff --git a/tests/Benchmark.fio/parser.py b/tests/Benchmark.fio/parser.py old mode 100755 new mode 100644 index 52a44e5..ef081b5
> --- a/tests/Benchmark.fio/parser.py
> +++ b/tests/Benchmark.fio/parser.py
> @@ -4,13 +4,30 @@
>  import os, sys
>  import common as plib
> 
> -regex_string = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
> +regex_string = ''
> +regex_string_old = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
> +regex_string_new = '( READ:| WRITE:)(.*)(bw=.*)(.*)(\()([\d.]+)([kKM]B\/s)(\))'
regex_string_new has a rather perplexing sequence here:
(bw=.*)(.*)(\()
The opening paren can be used as a match to terminate that sequence, but
why are there two .* expressions back-to-back.  That's weird.

Also,
Both of these strings (the old and the new) use more groups than they really need to .
I know you are just copying the already-existing regex_string, but the only
groups actually used by the parser are (in the old) string group 3 and group 4.

It might be worth taking out the other unused groups, and just making these
strings:
regex_string_old = '(  READ:|  WRITE:).*aggrb=([\d.]+)([KM]B\/s)'
regex_string_new = '( READ:| WRITE:).*bw=.*\(([\d.]+)([kKM]B\/s)\)'

Then, you could just use groups 1 and 2 for both regex strings.
I think this simplifies the parser (and the patch), if it works.

I don't have the 2.16 version of fio on any of my boards.  Can you
test this there and let me know if it works.

> +test_version = ''
> +with open(plib.TEST_LOG,'r') as cur_file:
> +    raw_values = cur_file.readlines()
> +    test_version = raw_values[4].rstrip("\n")
> +
> +if test_version > '2.16':
Should this be >= instead of just '>'?

> +    regex_string = regex_string_new
> +else:
> +    regex_string = regex_string_old
> +
>  measurements = {}
> 
>  # handle results in MB or KB
>  def set_measure(tguid, match):
> -    speed = match[3]
> -    units = match[4]
> +    if test_version > '2.16':
> +        speed = match[5]
> +        units = match[6]
> +    else:
> +        speed = match[3]
> +        units = match[4]

If you make the group numbers the same for both the old and new regex_strings,
you don't need this extra test.
With my above rewrite of the regext_strings, this would change to:
speed = match[1]
units = match[2]

and would work for both regex_string_old and regex_string_new.

>      multiplier = 1
>      if units.startswith('M'):
>          # WARNING - this depends on the value of kb_base specified to
> --
> 2.20.1
> 

Can you please see if you can rewrite this parser with less groups in the regex_strings,
and try to use the same group indices for the desired parse elements for both the
old and new regex strings?

Thanks,
 -- Tim

[Fuego] [PATCH 0/1] Add support to parse later versions of fio 2.16.

[sireesha.nakkala]

From: sireesha <sireesha.nakkala@toshiba-tsip.com>

Hi Tim, 

As per your suggestion I have modified the regex strings and verified in
3.25 version which is greater than 2.16. 

>> if test_version > '2.16':
> Should this be >= instead of just '>'?
The fio output format has actually changed from 2.17 version and so the
condition holds good.

Kindly review and provide feedback. 


sireesha (1):
  fio/parser.py: Add support to parse later versions of fio 2.16

 tests/Benchmark.fio/parser.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
 mode change 100755 => 100644 tests/Benchmark.fio/parser.py

-- 
2.20.1

[Fuego] [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio 2.16

[sireesha.nakkala]

From: sireesha <sireesha.nakkala@toshiba-tsip.com>

The existing parser.py file works for fio version 2.0.8 which is the
test version defined and used in fuego tests.

As fio output has changed from 2.16 version, to use fio with fuego
the parser is modified to parse the output of fio version 2.0.8 and
later versions of 2.16.

Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
---
 tests/Benchmark.fio/parser.py | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
 mode change 100755 => 100644 tests/Benchmark.fio/parser.py

diff --git a/tests/Benchmark.fio/parser.py b/tests/Benchmark.fio/parser.py
old mode 100755
new mode 100644
index 52a44e5..ab3ea34
--- a/tests/Benchmark.fio/parser.py
+++ b/tests/Benchmark.fio/parser.py
@@ -4,13 +4,22 @@
 import os, sys
 import common as plib
 
-regex_string = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
+test_version = ''
+with open(plib.TEST_LOG,'r') as cur_file:
+    raw_values = cur_file.readlines()
+    test_version = raw_values[4].rstrip("\n")
+
+if test_version > '2.16':
+    regex_string = '( READ:| WRITE:).*bw=.*\(([\d.]+)([kKM]B\/s)\)'
+else:
+    regex_string = '(  READ:|  WRITE:).*aggrb=([\d.]+)([KM]B\/s)'
+
 measurements = {}
 
 # handle results in MB or KB
 def set_measure(tguid, match):
-    speed = match[3]
-    units = match[4]
+    speed = match[1]
+    units = match[2]
     multiplier = 1
     if units.startswith('M'):
         # WARNING - this depends on the value of kb_base specified to
-- 
2.20.1

Re: [Fuego] [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio 2.16

[Bird, Tim]

See comments inline below.

> -----Original Message-----
> From: sireesha.nakkala@toshiba-tsip.com <sireesha.nakkala@toshiba-tsip.com>
> 
> From: sireesha <sireesha.nakkala@toshiba-tsip.com>
> 
> The existing parser.py file works for fio version 2.0.8 which is the
> test version defined and used in fuego tests.
> 
> As fio output has changed from 2.16 version, to use fio with fuego
> the parser is modified to parse the output of fio version 2.0.8 and
> later versions of 2.16.
> 
> Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> ---
>  tests/Benchmark.fio/parser.py | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>  mode change 100755 => 100644 tests/Benchmark.fio/parser.py
> 
> diff --git a/tests/Benchmark.fio/parser.py b/tests/Benchmark.fio/parser.py
> old mode 100755
> new mode 100644
> index 52a44e5..ab3ea34
> --- a/tests/Benchmark.fio/parser.py
> +++ b/tests/Benchmark.fio/parser.py
> @@ -4,13 +4,22 @@
>  import os, sys
>  import common as plib
> 
> -regex_string = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
> +test_version = ''
> +with open(plib.TEST_LOG,'r') as cur_file:
> +    raw_values = cur_file.readlines()
> +    test_version = raw_values[4].rstrip("\n")
> +
> +if test_version > '2.16':
> +    regex_string = '( READ:| WRITE:).*bw=.*\(([\d.]+)([kKM]B\/s)\)'
> +else:
> +    regex_string = '(  READ:|  WRITE:).*aggrb=([\d.]+)([KM]B\/s)'
> +

I'm a little confused about how you are executing an fio version greater
than 2.0.8, which is the one installed by fuego_test.sh.

This test doesn't have logic that detects and uses a different version of the
'fio' binary if it is already on the board.  Do you have other changes to this test
to go along with the parse change?

If so, please send them.

>  measurements = {}
> 
>  # handle results in MB or KB
>  def set_measure(tguid, match):
> -    speed = match[3]
> -    units = match[4]
> +    speed = match[1]
> +    units = match[2]
>      multiplier = 1
>      if units.startswith('M'):
>          # WARNING - this depends on the value of kb_base specified to
> --
> 2.20.1
> 

I'm OK with this parser change.  And thanks for making the requested changes
to simplify the search strings and the groups.

This is applied and pushed, as it future-proofs the parser - which is
a good thing.  But it would be good to know how you are using this
with other versions of the 'fio' binary, since the current test in
fuego-core mainline doesn't handle that.

 -- Tim

Re: [Fuego] [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio 2.16

[sireesha.nakkala]

Thank you for applying the changes.

Please find the comments inline below

> -----Original Message-----
> From: Bird, Tim <Tim.Bird@sony.com>
> Sent: Friday, April 15, 2022 3:18 AM
> To: nakkala sireesha(ＴＳＩＰ) <sireesha.nakkala@toshiba-tsip.com>
> Cc: fuego@lists.linuxfoundation.org; dinesh kumar(ＴＳＩＰ)
> <dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □ＳＷＣ◯ＡＣ
> Ｔ) <kazuhiro3.hayashi@toshiba.co.jp>; pyla venkata(ＴＳＩＰ)
> <Venkata.Pyla@toshiba-tsip.com>
> Subject: RE: [PATCH 1/1] fio/parser.py: Add support to parse later versions of
> fio 2.16
> 
> See comments inline below.
> 
> > -----Original Message-----
> > From: sireesha.nakkala@toshiba-tsip.com
> > <sireesha.nakkala@toshiba-tsip.com>
> >
> > From: sireesha <sireesha.nakkala@toshiba-tsip.com>
> >
> > The existing parser.py file works for fio version 2.0.8 which is the
> > test version defined and used in fuego tests.
> >
> > As fio output has changed from 2.16 version, to use fio with fuego the
> > parser is modified to parse the output of fio version 2.0.8 and later
> > versions of 2.16.
> >
> > Signed-off-by: sireesha <sireesha.nakkala@toshiba-tsip.com>
> > Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> > ---
> >  tests/Benchmark.fio/parser.py | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)  mode change 100755
> > => 100644 tests/Benchmark.fio/parser.py
> >
> > diff --git a/tests/Benchmark.fio/parser.py
> > b/tests/Benchmark.fio/parser.py old mode 100755 new mode 100644
> index
> > 52a44e5..ab3ea34
> > --- a/tests/Benchmark.fio/parser.py
> > +++ b/tests/Benchmark.fio/parser.py
> > @@ -4,13 +4,22 @@
> >  import os, sys
> >  import common as plib
> >
> > -regex_string = '(  READ:|  WRITE:)(.*)(aggrb=)([\d.]+)([KM]B\/s)'
> > +test_version = ''
> > +with open(plib.TEST_LOG,'r') as cur_file:
> > +    raw_values = cur_file.readlines()
> > +    test_version = raw_values[4].rstrip("\n")
> > +
> > +if test_version > '2.16':
> > +    regex_string = '( READ:| WRITE:).*bw=.*\(([\d.]+)([kKM]B\/s)\)'
> > +else:
> > +    regex_string = '(  READ:|  WRITE:).*aggrb=([\d.]+)([KM]B\/s)'
> > +
> 
> I'm a little confused about how you are executing an fio version greater than
> 2.0.8, which is the one installed by fuego_test.sh.
>
We are using the fio from Debian package system which is fio-3.25 version and that is pre-installed in the board, 
so we wanted to update the parsing logic in the fuego-core to report the results correctly.

We contributing this to upstream so that it will be useful in future if the fuego-core test are upgraded with later versions.

> This test doesn't have logic that detects and uses a different version of the
> 'fio' binary if it is already on the board.  Do you have other changes to this
> test to go along with the parse change?
> 
Currently we don’t have any other changes, in future if we see any issue will share with you.

> If so, please send them.
> 
> >  measurements = {}
> >
> >  # handle results in MB or KB
> >  def set_measure(tguid, match):
> > -    speed = match[3]
> > -    units = match[4]
> > +    speed = match[1]
> > +    units = match[2]
> >      multiplier = 1
> >      if units.startswith('M'):
> >          # WARNING - this depends on the value of kb_base specified to
> > --
> > 2.20.1
> >
> 
> I'm OK with this parser change.  And thanks for making the requested
> changes to simplify the search strings and the groups.
> 
> This is applied and pushed, as it future-proofs the parser - which is a good
> thing.  But it would be good to know how you are using this with other
> versions of the 'fio' binary, since the current test in fuego-core mainline
> doesn't handle that.
> 
>  -- Tim

Re: [Fuego] [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio 2.16

[Bird, Tim]

> -----Original Message-----
> From: sireesha.nakkala@toshiba-tsip.com <sireesha.nakkala@toshiba-tsip.com>
> > -----Original Message-----
> > From: Bird, Tim <Tim.Bird@sony.com>
...
> >
> > I'm a little confused about how you are executing an fio version greater than
> > 2.0.8, which is the one installed by fuego_test.sh.
> >
> We are using the fio from Debian package system which is fio-3.25 version and that is pre-installed in the board,
> so we wanted to update the parsing logic in the fuego-core to report the results correctly.
> 
> We contributing this to upstream so that it will be useful in future if the fuego-core test are upgraded with later versions.
> 
> > This test doesn't have logic that detects and uses a different version of the
> > 'fio' binary if it is already on the board.  Do you have other changes to this
> > test to go along with the parse change?
> >
> Currently we don't have any other changes, in future if we see any issue will share with you.
I figured you were running 'fio' from a distribution package (not the fio binary built by the Fuego test).
My question was "how"?

On my system, even if I have a newer version of 'fio' installed in /usr/bin, Fuego
still executes /home/fuego/fuego.Benchmar.fio/fio

I missed the part of fuego_test.sh where 'get_program_path fio' is used.
Apparently in your lab, you either do not run test_deploy, or you have
modified the test_deploy() function to not put 'fio' into $BOARD_TESTDIR/fuego.$TESTDIR.

That's the part I was asking about.

I was wondering if you had something like this in your Benchmark.fio/feugo_test.sh:test_deploy() function:
if ! cmd test -f /usr/bin/fio ; then
   put fio $BOARD_TESTDIR/fuego.$TESTDIR
fi

I'm not familiar with your mechanisms for operating on the local host as the
device under test.
 -- Tim

Re: [Fuego] [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio 2.16

[Venkata.Pyla]

Hi Tim,

Thanks for the prompt response and support always,

I would like know, whether there is any policy for keeping the Benchmark test versions in the fuego-core,
I mean the current version of fio in fuego-core is 2.0.8, but can we upgrade the version to the latest version (may be 3.30)?
Because the new versions got more details in the results, which may help us to use it.
I don’t have strong reason to upgrade it to latest version, but it is nice to have latest version as they give more details.

As you may know, in Toshiba we were using Debian based system, so we use Debian versions of Benchmark tests 
and we skip the build and deploy phases while running the fuego Benchmark tests, but we had to change the parsing logic of test results
such that it had to parse old and newer version of the test results.
Can we have mechanism to choose the parser.py based on test version? 
May be like below just an idea, need to check how feasible it is
parser_2.0.8.py
parser_3.25.py

Also, please find my inline comments in below.

Thanks,
Venkata.

>-----Original Message-----
>From: Bird, Tim <Tim.Bird@sony.com>
>Sent: 20 April 2022 02:28
>To: nakkala sireesha(ＴＳＩＰ) <sireesha.nakkala@toshiba-tsip.com>
>Cc: fuego@lists.linuxfoundation.org; dinesh kumar(ＴＳＩＰ)
><dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □ＳＷＣ◯ＡＣＴ)
><kazuhiro3.hayashi@toshiba.co.jp>; pyla venkata(ＴＳＩＰ)
><Venkata.Pyla@toshiba-tsip.com>
>Subject: RE: [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio
>2.16
>
>> -----Original Message-----
>> From: sireesha.nakkala@toshiba-tsip.com
>> <sireesha.nakkala@toshiba-tsip.com>
>> > -----Original Message-----
>> > From: Bird, Tim <Tim.Bird@sony.com>
>...
>> >
>> > I'm a little confused about how you are executing an fio version
>> > greater than 2.0.8, which is the one installed by fuego_test.sh.
>> >
>> We are using the fio from Debian package system which is fio-3.25
>> version and that is pre-installed in the board, so we wanted to update the
>parsing logic in the fuego-core to report the results correctly.
>>
>> We contributing this to upstream so that it will be useful in future if the fuego-
>core test are upgraded with later versions.
>>
>> > This test doesn't have logic that detects and uses a different
>> > version of the 'fio' binary if it is already on the board.  Do you
>> > have other changes to this test to go along with the parse change?
>> >
>> Currently we don't have any other changes, in future if we see any issue will
>share with you.
>I figured you were running 'fio' from a distribution package (not the fio binary
>built by the Fuego test).
>My question was "how"?
>
>On my system, even if I have a newer version of 'fio' installed in /usr/bin, Fuego
>still executes /home/fuego/fuego.Benchmar.fio/fio
>
>I missed the part of fuego_test.sh where 'get_program_path fio' is used.
>Apparently in your lab, you either do not run test_deploy, or you have modified
>the test_deploy() function to not put 'fio' into
>$BOARD_TESTDIR/fuego.$TESTDIR.
>
>That's the part I was asking about.
>
>I was wondering if you had something like this in your
>Benchmark.fio/feugo_test.sh:test_deploy() function:
>if ! cmd test -f /usr/bin/fio ; then
>   put fio $BOARD_TESTDIR/fuego.$TESTDIR fi
Thanks for reminding this, we have a change similar to that to apply test configurations present in test_deploy function.
But they are little complex to apply them in upstream, so we decided to currently keep them local, may be one day
I will make it simple and forward the patch to upstream.

>
>I'm not familiar with your mechanisms for operating on the local host as the
>device under test.
> -- Tim
>

Re: [Fuego] [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio 2.16

[Bird, Tim]

Hey Venkata,

See my comments inline below.

> -----Original Message-----
> From: Venkata.Pyla@toshiba-tsip.com <Venkata.Pyla@toshiba-tsip.com>
> 
> Hi Tim,
> 
> Thanks for the prompt response and support always,
> 
> I would like know, whether there is any policy for keeping the Benchmark test versions in the fuego-core,
> I mean the current version of fio in fuego-core is 2.0.8, but can we upgrade the version to the latest version (may be 3.30)?
> Because the new versions got more details in the results, which may help us to use it.
> I don't have strong reason to upgrade it to latest version, but it is nice to have latest version as they give more details.

Upgrading 'fio' seems like something that would not be a problem.  There's no hard and fast policy, but
I try hard to be backwards-compatible to avoid breaking any existing users and their workflows.

In the case of 'fio', your team has found the parsing differences between 'fio' versions.  I would want to
look at whether there are results differences between the different fio versions as well.

I did some experimenting here, and found a few things:
 - with 'fio' version 2.0.8 (the version that Fuego ships source for), the output includes the 'aggrb=' string
 - with 'fio' version 2.2.10 (a version I tried on one of my boards), the output also included 'aggrb='
    - this meant that the parsing change that was submitted failed, because the conditional was not correct
 - with 'fio' version 3.16 (another version available to me), the output included the 'bw=' string

However, the Fuego test had other problems with subsequent fio versions, because one of the test
elements has been deprecated.  I got the following messages in my testlog.txt:

./min1.default.6.6/consolelog.txt:fio: failed parsing rwmixcycle=40
./min1.default.6.6/consolelog.txt:Option rwmixcycle is deprecated
This is due to the test using the workload files from the 2.0.8 version of the test, but a later binary
where the rwmixcycle sub-workload is not supported.

(As an aside, are you seeing this message in any of your console logs or testlogs for the fio tests
on your systems?)

I checked, and it would be a bit more complicated to fix that issue. It appears that the
example workload files have not been updated to match the 'fio' binary (in the upstream
project), even for 3.x version binary packages. So these likely need to be patched when
using even updated workload files.
(for example, /usr/share/doc/fio/examples/fsx.fio still includes the statement
rwmixcycle=40, the Ubuntu 3.16 package for fio, even though this directive is obsolete
in the 'fio' binary for that package.)

So, I'm happy to upgrade base packages, but there must be sufficient time
and testing to make sure nothing breaks.  The fio parser.py change has turned out to
be more work than I expected.

> 
> As you may know, in Toshiba we were using Debian based system, so we use Debian versions of Benchmark tests
> and we skip the build and deploy phases while running the fuego Benchmark tests, but we had to change the parsing logic of test results
> such that it had to parse old and newer version of the test results.
> Can we have mechanism to choose the parser.py based on test version?
> May be like below just an idea, need to check how feasible it is
> parser_2.0.8.py
> parser_3.25.py

This is possible, but so far the changes between the different version test outputs are
simple enough that I think it's better to keep them in one parser.py, for the current
set of test measures.  However, maybe you desire to add other measures to the
Benchmark?

There certainly is a lot more data that could be
extracted from the test output, that might be of interest.  I haven't compared
the rest of the output (only the lines that the parser.py is currently examining),
to see if having a single parser is feasible.  If not, then using different parsers
based on version is possible.  But I'd still lean towards just using version checks
in the current parser (and only having a single parser.py module) to handle
the different fio versions' results output.

> 
> Also, please find my inline comments in below.
> 
> Thanks,
> Venkata.
> 
> >-----Original Message-----
> >From: Bird, Tim <Tim.Bird@sony.com>
> >
> >> -----Original Message-----
> >> From: sireesha.nakkala@toshiba-tsip.com
> >...
> >> >
> >> > I'm a little confused about how you are executing an fio version
> >> > greater than 2.0.8, which is the one installed by fuego_test.sh.
> >> >
> >> We are using the fio from Debian package system which is fio-3.25
> >> version and that is pre-installed in the board, so we wanted to update the
> >parsing logic in the fuego-core to report the results correctly.
> >>
> >> We contributing this to upstream so that it will be useful in future if the fuego-
> >core test are upgraded with later versions.
> >>
> >> > This test doesn't have logic that detects and uses a different
> >> > version of the 'fio' binary if it is already on the board.  Do you
> >> > have other changes to this test to go along with the parse change?
> >> >
> >> Currently we don't have any other changes, in future if we see any issue will
> >share with you.
> >I figured you were running 'fio' from a distribution package (not the fio binary
> >built by the Fuego test).
> >My question was "how"?
> >
> >On my system, even if I have a newer version of 'fio' installed in /usr/bin, Fuego
> >still executes /home/fuego/fuego.Benchmar.fio/fio
> >
> >I missed the part of fuego_test.sh where 'get_program_path fio' is used.
> >Apparently in your lab, you either do not run test_deploy, or you have modified
> >the test_deploy() function to not put 'fio' into
> >$BOARD_TESTDIR/fuego.$TESTDIR.
> >
> >That's the part I was asking about.
> >
> >I was wondering if you had something like this in your
> >Benchmark.fio/feugo_test.sh:test_deploy() function:
> >if ! cmd test -f /usr/bin/fio ; then
> >   put fio $BOARD_TESTDIR/fuego.$TESTDIR fi
>
> Thanks for reminding this, we have a change similar to that to apply test configurations present in test_deploy function.
> But they are little complex to apply them in upstream, so we decided to currently keep them local, may be one day
> I will make it simple and forward the patch to upstream.

OK - if you wouldn't mind, can you send a diff as an RFC?  (RFC=Request For Comments) to
the mailing list?

This is not a patch submission, but rather just a patch that indicates how you've done something,
for possible discussion on the mailing list.  I'd like to take a look at it, if you wouldn't mind.

It *may be* too complicated to apply to the upstream code, but it may also give me ideas
for how to support your use case without you having to maintain your own differences
to the base code.

In general I appreciate the patches you are sending, and want to make sure
that Fuego continues to support your use cases.  And I'd like to understand your use cases
and the tests that are most important to you, to make sure they get needed maintenance
and that I avoid breaking them.

It might be worthwhile to have a conference call sometime, to share information and
let me know how best to have Fuego support your testing efforts.
I have been working recently on supporting more use cases and install setups that
I believe are similar to how Toshiba is using the tool, and I think my work could
be better if I knew more about your experience and current practices with Fuego

Let me know what you think and if you'd be amenable to a call sometime.
 -- Tim

[Fuego] [RFC 0/1] fio configuration for fio verion 3.25

[venkata.pyla]

From: venkata pyla <venkata.pyla@toshiba-tsip.com>

Hi Tim,

as you requested in other mail chain[1] about the fio configuration how we
are doing in case of target installed binaries fio-3.25.

the below patch we are applying it locally, which will configure the
target installed fio files and execute the test.

This patch definitely not complaint to upstream fuego version, as it
is not considered the host - target communication, it assumed the script
runs on target machine directly.

we wanted it to improve this patch to comply with upstream, meanwhile if
you have any suggestion please let us know.

[1] https://lists.linuxfoundation.org/pipermail/fuego/2022-April/003833.html

Shivanand Kunijadar (1):
  Benchmark.fio: prepare fio configurations when using host installed
    binary

 tests/Benchmark.fio/fuego_test.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

-- 
2.20.1

[Fuego] [RFC 1/1] Benchmark.fio: prepare fio configurations when using host installed binary

[venkata.pyla]

From: Shivanand Kunijadar <Shivanand.Kunijadar@toshiba-tsip.com>

We use option `-p ptrca` in fio test case to skip build phase and
start run phase, this is useful in case if host installed binary is
being used instead of building fio binary via fuego.

if fio test is using host binary then preparing `fsx-current` and
`ssd-test-current` configurations from host installed fio examples
 is necessary.

Signed-off-by: Shivanand Kunijadar <Shivanand.Kunijadar@toshiba-tsip.com>
---
 tests/Benchmark.fio/fuego_test.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tests/Benchmark.fio/fuego_test.sh b/tests/Benchmark.fio/fuego_test.sh
index 363a5ef..f3e897d 100755
--- a/tests/Benchmark.fio/fuego_test.sh
+++ b/tests/Benchmark.fio/fuego_test.sh
@@ -36,6 +36,25 @@ function test_deploy {
 function test_run {
     hd_test_mount_prepare $BENCHMARK_FIO_MOUNT_BLOCKDEV $BENCHMARK_FIO_MOUNT_POINT
 
+    if [ ! -f $BOARD_TESTDIR/fuego.$TESTDIR/examples/fsx-current ] || \
+        [ ! -f $BOARD_TESTDIR/fuego.$TESTDIR/examples/ssd-test-current ]; then
+        # Assuming host system installaton is used
+        cp /usr/share/doc/fio/examples/fsx.fio fsx-current
+        cp /usr/share/doc/fio/examples/ssd-test.fio ssd-test-current
+
+        # libaio is not available in XXX; O_DIRECT is not supported by XXXfs.
+        sed -i -e "s/libaio/posixaio/g" -e "/direct=/d" fsx-current ssd-test-current
+        sed -i -e "s|mount\-point\-of\-ssd|$BENCHMARK_FIO_MOUNT_POINT/fuego.$TESTDIR|g" -e "s/1g/90m/g" ssd-test-current
+
+        # Decrease test execution time by an order of magnitude
+        sed -i -e "/loops=/cloops=10000" fsx-current
+        echo "directory=$BENCHMARK_FIO_MOUNT_POINT/fuego.$TESTDIR" >> fsx-current
+
+        # remove deprected rwmixcycle=
+        sed -i -e "/rwmixcycle=/d" fsx-current
+
+    fi
+
     get_program_path fio
     report "$PROGRAM_FIO ssd-test-current --timeout=$BENCHMARK_FIO_TIMEOUT"
     report_append "$PROGRAM_FIO fsx-current --timeout=$BENCHMARK_FIO_TIMEOUT"
-- 
2.20.1

Re: [Fuego] [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio 2.16

[Venkata.Pyla]

Hi Tim,

Sorry for my late replay,

Please see my inline coments.

>-----Original Message-----
>From: Bird, Tim <Tim.Bird@sony.com>
>Sent: 21 April 2022 05:55
>To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>; nakkala
>sireesha(ＴＳＩＰ) <sireesha.nakkala@toshiba-tsip.com>
>Cc: fuego@lists.linuxfoundation.org; dinesh kumar(ＴＳＩＰ)
><dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 □ＳＷＣ◯ＡＣＴ)
><kazuhiro3.hayashi@toshiba.co.jp>
>Subject: RE: [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio
>2.16
>
>Hey Venkata,
>
>See my comments inline below.
>
>> -----Original Message-----
>> From: Venkata.Pyla@toshiba-tsip.com <Venkata.Pyla@toshiba-tsip.com>
>>
>> Hi Tim,
>>
>> Thanks for the prompt response and support always,
>>
>> I would like know, whether there is any policy for keeping the
>> Benchmark test versions in the fuego-core, I mean the current version of fio in
>fuego-core is 2.0.8, but can we upgrade the version to the latest version (may be
>3.30)?
>> Because the new versions got more details in the results, which may help us to
>use it.
>> I don't have strong reason to upgrade it to latest version, but it is nice to have
>latest version as they give more details.
>
>Upgrading 'fio' seems like something that would not be a problem.  There's no
>hard and fast policy, but I try hard to be backwards-compatible to avoid breaking
>any existing users and their workflows.
>
>In the case of 'fio', your team has found the parsing differences between 'fio'
>versions.  I would want to look at whether there are results differences between
>the different fio versions as well.
>
>I did some experimenting here, and found a few things:
> - with 'fio' version 2.0.8 (the version that Fuego ships source for), the output
>includes the 'aggrb=' string
> - with 'fio' version 2.2.10 (a version I tried on one of my boards), the output also
>included 'aggrb='
>    - this meant that the parsing change that was submitted failed, because the
>conditional was not correct
> - with 'fio' version 3.16 (another version available to me), the output included
>the 'bw=' string
>
The condition check problem we will re-verify it and send the corrected version.

Regarding the fio result output change , the main change we observed is
 'aggrb=' to 'bw=', and there are other changes but we are not using them in our parsing logic.


>However, the Fuego test had other problems with subsequent fio versions,
>because one of the test elements has been deprecated.  I got the following
>messages in my testlog.txt:
>
>./min1.default.6.6/consolelog.txt:fio: failed parsing rwmixcycle=40
>./min1.default.6.6/consolelog.txt:Option rwmixcycle is deprecated This is due to
>the test using the workload files from the 2.0.8 version of the test, but a later
>binary where the rwmixcycle sub-workload is not supported.
>
>(As an aside, are you seeing this message in any of your console logs or testlogs
>for the fio tests on your systems?)

In other mail chain I have sent the RFC patch, in that we fixed this problem,
 kindly please check it

>
>I checked, and it would be a bit more complicated to fix that issue. It appears
>that the example workload files have not been updated to match the 'fio' binary
>(in the upstream project), even for 3.x version binary packages. So these likely
>need to be patched when using even updated workload files.
>(for example, /usr/share/doc/fio/examples/fsx.fio still includes the statement
>rwmixcycle=40, the Ubuntu 3.16 package for fio, even though this directive is
>obsolete in the 'fio' binary for that package.)
>
>So, I'm happy to upgrade base packages, but there must be sufficient time and
>testing to make sure nothing breaks.  The fio parser.py change has turned out to
>be more work than I expected.
>
>>
>> As you may know, in Toshiba we were using Debian based system, so we
>> use Debian versions of Benchmark tests and we skip the build and
>> deploy phases while running the fuego Benchmark tests, but we had to change
>the parsing logic of test results such that it had to parse old and newer version of
>the test results.
>> Can we have mechanism to choose the parser.py based on test version?
>> May be like below just an idea, need to check how feasible it is
>> parser_2.0.8.py parser_3.25.py
>
>This is possible, but so far the changes between the different version test outputs
>are simple enough that I think it's better to keep them in one parser.py, for the
>current set of test measures.  However, maybe you desire to add other
>measures to the Benchmark?
>
>There certainly is a lot more data that could be extracted from the test output,
>that might be of interest.  I haven't compared the rest of the output (only the
>lines that the parser.py is currently examining), to see if having a single parser is
>feasible.  If not, then using different parsers based on version is possible.  But I'd
>still lean towards just using version checks in the current parser (and only having
>a single parser.py module) to handle the different fio versions' results output.
>
>>
>> Also, please find my inline comments in below.
>>
>> Thanks,
>> Venkata.
>>
>> >-----Original Message-----
>> >From: Bird, Tim <Tim.Bird@sony.com>
>> >
>> >> -----Original Message-----
>> >> From: sireesha.nakkala@toshiba-tsip.com
>> >...
>> >> >
>> >> > I'm a little confused about how you are executing an fio version
>> >> > greater than 2.0.8, which is the one installed by fuego_test.sh.
>> >> >
>> >> We are using the fio from Debian package system which is fio-3.25
>> >> version and that is pre-installed in the board, so we wanted to
>> >> update the
>> >parsing logic in the fuego-core to report the results correctly.
>> >>
>> >> We contributing this to upstream so that it will be useful in
>> >> future if the fuego-
>> >core test are upgraded with later versions.
>> >>
>> >> > This test doesn't have logic that detects and uses a different
>> >> > version of the 'fio' binary if it is already on the board.  Do
>> >> > you have other changes to this test to go along with the parse change?
>> >> >
>> >> Currently we don't have any other changes, in future if we see any
>> >> issue will
>> >share with you.
>> >I figured you were running 'fio' from a distribution package (not the
>> >fio binary built by the Fuego test).
>> >My question was "how"?
>> >
>> >On my system, even if I have a newer version of 'fio' installed in
>> >/usr/bin, Fuego still executes /home/fuego/fuego.Benchmar.fio/fio
>> >
>> >I missed the part of fuego_test.sh where 'get_program_path fio' is used.
>> >Apparently in your lab, you either do not run test_deploy, or you
>> >have modified the test_deploy() function to not put 'fio' into
>> >$BOARD_TESTDIR/fuego.$TESTDIR.
>> >
>> >That's the part I was asking about.
>> >
>> >I was wondering if you had something like this in your
>> >Benchmark.fio/feugo_test.sh:test_deploy() function:
>> >if ! cmd test -f /usr/bin/fio ; then
>> >   put fio $BOARD_TESTDIR/fuego.$TESTDIR fi
>>
>> Thanks for reminding this, we have a change similar to that to apply test
>configurations present in test_deploy function.
>> But they are little complex to apply them in upstream, so we decided
>> to currently keep them local, may be one day I will make it simple and forward
>the patch to upstream.
>
>OK - if you wouldn't mind, can you send a diff as an RFC?  (RFC=Request For
>Comments) to the mailing list?
>
>This is not a patch submission, but rather just a patch that indicates how you've
>done something, for possible discussion on the mailing list.  I'd like to take a look
>at it, if you wouldn't mind.
>
>It *may be* too complicated to apply to the upstream code, but it may also give
>me ideas for how to support your use case without you having to maintain your
>own differences to the base code.
I have shared the RFC patch for the change we are doing, but that is not the correct patch for
Upstream anyway, I may work to improve it  and send you more appropriate for the upstream,
But meanwhile if you have any comment please provide in that patch.

>
>In general I appreciate the patches you are sending, and want to make sure that
>Fuego continues to support your use cases.  And I'd like to understand your use
>cases and the tests that are most important to you, to make sure they get
>needed maintenance and that I avoid breaking them.

Basically in my project we are using fuego with non-jenkins version (ftc command)
 and for the benchmark tests we use Debian provided versions which are installed in the 
target board.

As I understood fuego is using the fixed test version for their Benchmark/Functional tests
I think this is  helpful for comparing the results with the same test versions, and also to use
fuego with different test versions maybe we can make fuego more easily configurable to adopt 
different test versions.
This is just an idea, I am not sure about the complexity or it deviates the goal of this project. 

>
>It might be worthwhile to have a conference call sometime, to share
>information and let me know how best to have Fuego support your testing
>efforts.
>I have been working recently on supporting more use cases and install setups
>that I believe are similar to how Toshiba is using the tool, and I think my work
>could be better if I knew more about your experience and current practices with
>Fuego
>
>Let me know what you think and if you'd be amenable to a call sometime.
I think it is good idea to share our thoughts in the call.
Let me know your available times 
For me it is good in following dates {11,12,13}/May, 
any time between 2 ~ 5PM (Indian Standard Time)

> -- Tim