[Fuego] Fixes for dbench4

[Fuego] Fixes for dbench4

[Daniel Sangorrin]

Hi Tim,

These are some patches to deal with the problem we discussed
on this thread:
https://lists.linuxfoundation.org/pipermail/fuego/2018-July/002216.html

[PATCH 1/3] dbench4: fix chart_config error
[PATCH 2/3] test.yaml: fix spaces
[PATCH 3/3] dbench4: add compatibility with files on the target

By the way, I just found out that Zhong had already sent some patches
to the list:
https://lists.linuxfoundation.org/pipermail/fuego/2018-August/thread.html

I think that my patches already cover
[PATCH 4/5] Benchmark.dbench4: add judgement for client.txt
However, the other 2 need review
[PATCH 3/5] Benchmark.dbench4: with make error
  -> I would rather have a check for libc.a
[PATCH 5/5] Benchmark.dbench4: add sync related cases in spec
  -> not sure this is worth it, but doesn't hurt either

Best regards,
Daniel

[Fuego] [PATCH 1/3] dbench4: fix chart_config error

[Daniel Sangorrin]

I was getting this error:
!!! ERROR: Missing chart_type in chart config file (/fuego-core/engine/tests/Benchmark.dbench4/chart_config.json)
Please add 'chart_type' entry in the file
chart config not found. Using default values.

TODO: if this is a change in the API or a regression, other
may need changes as well.

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 engine/tests/Benchmark.dbench4/chart_config.json | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/engine/tests/Benchmark.dbench4/chart_config.json b/engine/tests/Benchmark.dbench4/chart_config.json
index ee256a2..cd5f799 100644
--- a/engine/tests/Benchmark.dbench4/chart_config.json
+++ b/engine/tests/Benchmark.dbench4/chart_config.json
@@ -1,3 +1,4 @@
 {
-        "dbench":["Throughput"]
+    "chart_type": "measure_plot",
+    "measures": ["default.dbench.Throughput"]
 }
-- 
2.7.4

[Fuego] [PATCH 2/3] test.yaml: fix spaces

[Daniel Sangorrin]

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
---
 engine/tests/Benchmark.dbench4/test.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/engine/tests/Benchmark.dbench4/test.yaml b/engine/tests/Benchmark.dbench4/test.yaml
index c18ed97..8f261ef 100644
--- a/engine/tests/Benchmark.dbench4/test.yaml
+++ b/engine/tests/Benchmark.dbench4/test.yaml
@@ -10,8 +10,8 @@ fuego_release: 1
 type: Benchmark
 tags: ['disk', 'storage', 'performance']
 tarball_src:
-	- dbench-4.00.tar.gz: https://github.com/sahlberg/dbench (commit: 53ed08d)
-	- zlib-1.2.11.tar.gz: http://zlib.net/zlib-1.2.11.tar.gz
+    - dbench-4.00.tar.gz: https://github.com/sahlberg/dbench (commit: 53ed08d)
+    - zlib-1.2.11.tar.gz: http://zlib.net/zlib-1.2.11.tar.gz
 gitrepo: https://github.com/sahlberg/dbench
 params:
     - MOUNT_BLOCKDEV:
-- 
2.7.4

[Fuego] [PATCH 3/3] dbench4: add compatibility with files on the target

[Daniel Sangorrin]

Zhong reported a problem when dbench was on the target
but client.txt was absent. This patch adds the ability
to specify a loadfile using an absolute path, and
the ability to check whether a loadfile is on the target
or not.

By adding the loadfile parameter to the spec, we can also
now specify other loadfiles such as smb.txt or scsi.txt,
although I haven't tested that properly because it needs
some setup.

I noticed that Debian's dbench is 4.0 but does not have a
backend (-B parameter). I added a variable to specify that
and if it is not specified I try to guess the best one
from the loadfile.

TODO: add support for socketio

Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
Reported-by: Lu Zhong <zhongl.fnst@cn.fujitsu.com>
---
 engine/tests/Benchmark.dbench4/fuego_test.sh | 70 ++++++++++++++++++++++------
 engine/tests/Benchmark.dbench4/spec.json     |  4 +-
 engine/tests/Benchmark.dbench4/test.yaml     | 12 +++++
 3 files changed, 72 insertions(+), 14 deletions(-)

diff --git a/engine/tests/Benchmark.dbench4/fuego_test.sh b/engine/tests/Benchmark.dbench4/fuego_test.sh
index 535bdd9..14942d3 100755
--- a/engine/tests/Benchmark.dbench4/fuego_test.sh
+++ b/engine/tests/Benchmark.dbench4/fuego_test.sh
@@ -7,6 +7,7 @@ function test_pre_check {
     assert_define BENCHMARK_DBENCH_TIMELIMIT
     assert_define BENCHMARK_DBENCH_NPROCS
 
+    # check if dbench 4+ exists on the target
     is_on_target_path dbench PROGRAM_DBENCH
     if [ ! -z "$PROGRAM_DBENCH" ]; then
         help=$(cmd "dbench --help") || true
@@ -19,6 +20,26 @@ function test_pre_check {
             echo "dbench found on the target (version $version)."
         fi
     fi
+
+    # check if the loadfile exists on the target
+    LOADFILE=${BENCHMARK_DBENCH_LOADFILE:-client.txt}
+    LOADFILE_ONTARGET=""
+    if is_abs_path $LOADFILE; then
+        if $(cmd "test -e $LOADFILE"); then
+            LOADFILE_ONTARGET=$LOADFILE
+            echo "$LOADFILE_ONTARGET found on the target."
+        else
+            echo "Warning: $LOADFILE not found on the target, using Fuego's default."
+            LOADFILE_ONTARGET=""
+            LOADFILE="client.txt"
+        fi
+    else
+        # check if the file is on the target board
+        if $(cmd "test -e /usr/share/dbench/$LOADFILE"); then
+            LOADFILE_ONTARGET="/usr/share/dbench/$LOADFILE"
+            echo "$LOADFILE_ONTARGET found on the target."
+        fi
+    fi
 }
 
 function test_build {
@@ -41,29 +62,52 @@ function test_build {
 
 function test_deploy {
     if [ -z "$PROGRAM_DBENCH" ]; then
-        put dbench loadfiles/client.txt $BOARD_TESTDIR/fuego.$TESTDIR/
+        echo "Deploying dbench."
+        put dbench $BOARD_TESTDIR/fuego.$TESTDIR/
+        PROGRAM_DBENCH="$BOARD_TESTDIR/fuego.$TESTDIR/dbench"
+    fi
+
+    if [ -z "$LOADFILE_ONTARGET" ]; then
+        echo "Deploying loadfiles/$LOADFILE."
+        put loadfiles/$LOADFILE $BOARD_TESTDIR/fuego.$TESTDIR/
+        LOADFILE_ONTARGET="$BOARD_TESTDIR/fuego.$TESTDIR/$LOADFILE"
     fi
 }
 
 function test_run {
+    # check if the backend needs to be specified
+    HELP=$(cmd "$PROGRAM_DBENCH --help") || true
+    HELP_BACKEND=$(echo $HELP | grep backend) || true
+    if [ ! -z "$HELP_BACKEND" ]; then
+        echo "This dbench supports backends"
+        if [ -z "$BENCHMARK_DBENCH_BACKEND" ]; then
+            echo "Warning: backend not specified, trying to guess one."
+            case $(basename $LOADFILE) in
+                client.txt) BENCHMARK_DBENCH_BACKEND="fileio" ;;
+                iscsi*) BENCHMARK_DBENCH_BACKEND="iscsi" ;;
+                scsi.txt) BENCHMARK_DBENCH_BACKEND="scsi" ;;
+                smb*) BENCHMARK_DBENCH_BACKEND="smb" ;;
+                nfs*) BENCHMARK_DBENCH_BACKEND="nfs" ;;
+                *) BENCHMARK_DBENCH_BACKEND="sockio" ;;
+            esac
+        fi
+        echo "Using backend $BENCHMARK_DBENCH_BACKEND"
+        BACKEND="-B $BENCHMARK_DBENCH_BACKEND"
+    else
+        echo "This dbench does not support backends"
+        BACKEND=""
+    fi
+
     hd_test_mount_prepare $BENCHMARK_DBENCH_MOUNT_BLOCKDEV \
         $BENCHMARK_DBENCH_MOUNT_POINT
-    if [ -z "$PROGRAM_DBENCH" ]; then
-        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
-            ./dbench -B fileio -c ./client.txt \
-            -D $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
-            -t $BENCHMARK_DBENCH_TIMELIMIT \
-            $BENCHMARK_DBENCH_EXTRAPARAMS \
-            $BENCHMARK_DBENCH_NPROCS; \
-            sync"
-    else
-        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
-            dbench -D $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
+    # TODO: timelimit does not seem to work like a timeout as expected
+    report "$PROGRAM_DBENCH \
             -t $BENCHMARK_DBENCH_TIMELIMIT \
+            $BACKEND -c $LOADFILE_ONTARGET \
+            -D $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
             $BENCHMARK_DBENCH_EXTRAPARAMS \
             $BENCHMARK_DBENCH_NPROCS; \
             sync"
-    fi
     hd_test_clean_umount $BENCHMARK_DBENCH_MOUNT_BLOCKDEV \
         $BENCHMARK_DBENCH_MOUNT_POINT
 }
diff --git a/engine/tests/Benchmark.dbench4/spec.json b/engine/tests/Benchmark.dbench4/spec.json
index c855f36..c7df4b3 100644
--- a/engine/tests/Benchmark.dbench4/spec.json
+++ b/engine/tests/Benchmark.dbench4/spec.json
@@ -5,7 +5,9 @@
             "MOUNT_BLOCKDEV":"ROOT",
             "MOUNT_POINT":"$BOARD_TESTDIR/work",
             "TIMELIMIT":"30",
-            "NPROCS":"2"
+            "NPROCS":"2",
+            "LOADFILE":"client.txt",
+            "BACKEND":"fileio"
         },
         "sync": {
             "MOUNT_BLOCKDEV":"ROOT",
diff --git a/engine/tests/Benchmark.dbench4/test.yaml b/engine/tests/Benchmark.dbench4/test.yaml
index 8f261ef..10a407a 100644
--- a/engine/tests/Benchmark.dbench4/test.yaml
+++ b/engine/tests/Benchmark.dbench4/test.yaml
@@ -30,6 +30,18 @@ params:
         description: Number of parallel threads.
         example: 2
         optional: no
+    - LOADFILE:
+        description: |
+            One of the loadfiles under the loadfiles/ folder. If an absolute
+            path is specified it will be interpreted as a file in the target
+            filesystem to be used as load file.
+        example: client.txt, nfs.txt, scsi.txt, /usr/share/client.txt
+        default: client.txt
+        optional: yes
+    - BACKEND:
+        description: the dbench backend
+        example: fileio, sockio, nfs, scsi, iscsi, smb
+        optional: yes
     - EXTRAPARAMS:
         description: |
                 Additional parameters such as
-- 
2.7.4

Re: [Fuego] [PATCH 1/3] dbench4: fix chart_config error

[Tim.Bird]

Looks good. Applied.
Thanks,
 -- Tim

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> Sent: Tuesday, October 30, 2018 2:59 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 1/3] dbench4: fix chart_config error
> 
> I was getting this error:
> !!! ERROR: Missing chart_type in chart config file (/fuego-
> core/engine/tests/Benchmark.dbench4/chart_config.json)
> Please add 'chart_type' entry in the file
> chart config not found. Using default values.
> 
> TODO: if this is a change in the API or a regression, other
> may need changes as well.
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  engine/tests/Benchmark.dbench4/chart_config.json | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/engine/tests/Benchmark.dbench4/chart_config.json
> b/engine/tests/Benchmark.dbench4/chart_config.json
> index ee256a2..cd5f799 100644
> --- a/engine/tests/Benchmark.dbench4/chart_config.json
> +++ b/engine/tests/Benchmark.dbench4/chart_config.json
> @@ -1,3 +1,4 @@
>  {
> -        "dbench":["Throughput"]
> +    "chart_type": "measure_plot",
> +    "measures": ["default.dbench.Throughput"]
>  }
> --
> 2.7.4
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] [PATCH 2/3] test.yaml: fix spaces

[Tim.Bird]

Applied. Thanks.
 -- Tim

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org [mailto:fuego-
> bounces@lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> Sent: Tuesday, October 30, 2018 2:59 AM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 2/3] test.yaml: fix spaces
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> ---
>  engine/tests/Benchmark.dbench4/test.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/engine/tests/Benchmark.dbench4/test.yaml
> b/engine/tests/Benchmark.dbench4/test.yaml
> index c18ed97..8f261ef 100644
> --- a/engine/tests/Benchmark.dbench4/test.yaml
> +++ b/engine/tests/Benchmark.dbench4/test.yaml
> @@ -10,8 +10,8 @@ fuego_release: 1
>  type: Benchmark
>  tags: ['disk', 'storage', 'performance']
>  tarball_src:
> -	- dbench-4.00.tar.gz: https://github.com/sahlberg/dbench (commit:
> 53ed08d)
> -	- zlib-1.2.11.tar.gz: http://zlib.net/zlib-1.2.11.tar.gz
> +    - dbench-4.00.tar.gz: https://github.com/sahlberg/dbench (commit:
> 53ed08d)
> +    - zlib-1.2.11.tar.gz: http://zlib.net/zlib-1.2.11.tar.gz
>  gitrepo: https://github.com/sahlberg/dbench
>  params:
>      - MOUNT_BLOCKDEV:
> --
> 2.7.4
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] [PATCH 3/3] dbench4: add compatibility with files on the target

[Tim.Bird]

> -----Original Message-----
> From: Daniel Sangorrin on October 30, 2018 2:59 AM
> 
> Zhong reported a problem when dbench was on the target
> but client.txt was absent. This patch adds the ability
> to specify a loadfile using an absolute path, and
> the ability to check whether a loadfile is on the target
> or not.
> 
> By adding the loadfile parameter to the spec, we can also
> now specify other loadfiles such as smb.txt or scsi.txt,
> although I haven't tested that properly because it needs
> some setup.
> 
> I noticed that Debian's dbench is 4.0 but does not have a
> backend (-B parameter). I added a variable to specify that
> and if it is not specified I try to guess the best one
> from the loadfile.
> 
> TODO: add support for socketio
> 
> Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> Reported-by: Lu Zhong <zhongl.fnst@cn.fujitsu.com>
> ---
>  engine/tests/Benchmark.dbench4/fuego_test.sh | 70
> ++++++++++++++++++++++------
>  engine/tests/Benchmark.dbench4/spec.json     |  4 +-
>  engine/tests/Benchmark.dbench4/test.yaml     | 12 +++++
>  3 files changed, 72 insertions(+), 14 deletions(-)
> 
> diff --git a/engine/tests/Benchmark.dbench4/fuego_test.sh
> b/engine/tests/Benchmark.dbench4/fuego_test.sh
> index 535bdd9..14942d3 100755
> --- a/engine/tests/Benchmark.dbench4/fuego_test.sh
> +++ b/engine/tests/Benchmark.dbench4/fuego_test.sh
> @@ -7,6 +7,7 @@ function test_pre_check {
>      assert_define BENCHMARK_DBENCH_TIMELIMIT
>      assert_define BENCHMARK_DBENCH_NPROCS
> 
> +    # check if dbench 4+ exists on the target
>      is_on_target_path dbench PROGRAM_DBENCH
>      if [ ! -z "$PROGRAM_DBENCH" ]; then
>          help=$(cmd "dbench --help") || true
> @@ -19,6 +20,26 @@ function test_pre_check {
>              echo "dbench found on the target (version $version)."
>          fi
>      fi
> +
> +    # check if the loadfile exists on the target
> +    LOADFILE=${BENCHMARK_DBENCH_LOADFILE:-client.txt}
> +    LOADFILE_ONTARGET=""
> +    if is_abs_path $LOADFILE; then
I'm not familiar with "is_abs_path" .  Is this a shell function
or a program?

> +        if $(cmd "test -e $LOADFILE"); then
> +            LOADFILE_ONTARGET=$LOADFILE
> +            echo "$LOADFILE_ONTARGET found on the target."
> +        else
> +            echo "Warning: $LOADFILE not found on the target, using Fuego's
> default."
> +            LOADFILE_ONTARGET=""
> +            LOADFILE="client.txt"
> +        fi
> +    else
> +        # check if the file is on the target board
> +        if $(cmd "test -e /usr/share/dbench/$LOADFILE"); then
> +            LOADFILE_ONTARGET="/usr/share/dbench/$LOADFILE"
> +            echo "$LOADFILE_ONTARGET found on the target."
> +        fi
> +    fi
>  }
> 
>  function test_build {
> @@ -41,29 +62,52 @@ function test_build {
> 
>  function test_deploy {
>      if [ -z "$PROGRAM_DBENCH" ]; then
> -        put dbench loadfiles/client.txt $BOARD_TESTDIR/fuego.$TESTDIR/
> +        echo "Deploying dbench."
> +        put dbench $BOARD_TESTDIR/fuego.$TESTDIR/
> +        PROGRAM_DBENCH="$BOARD_TESTDIR/fuego.$TESTDIR/dbench"
> +    fi
> +
> +    if [ -z "$LOADFILE_ONTARGET" ]; then
> +        echo "Deploying loadfiles/$LOADFILE."
> +        put loadfiles/$LOADFILE $BOARD_TESTDIR/fuego.$TESTDIR/
> +        LOADFILE_ONTARGET="$BOARD_TESTDIR/fuego.$TESTDIR/$LOADFILE"
>      fi
>  }
> 
>  function test_run {
> +    # check if the backend needs to be specified
> +    HELP=$(cmd "$PROGRAM_DBENCH --help") || true
> +    HELP_BACKEND=$(echo $HELP | grep backend) || true
Couldn't this just be: $(cmd "$PROGRAM_DBENCH --help" | grep backend) || true ?

> +    if [ ! -z "$HELP_BACKEND" ]; then
if [ -n "$HELP_BACKEND" ]; then

> +        echo "This dbench supports backends"
> +        if [ -z "$BENCHMARK_DBENCH_BACKEND" ]; then
> +            echo "Warning: backend not specified, trying to guess one."
> +            case $(basename $LOADFILE) in
> +                client.txt) BENCHMARK_DBENCH_BACKEND="fileio" ;;
> +                iscsi*) BENCHMARK_DBENCH_BACKEND="iscsi" ;;
> +                scsi.txt) BENCHMARK_DBENCH_BACKEND="scsi" ;;
> +                smb*) BENCHMARK_DBENCH_BACKEND="smb" ;;
> +                nfs*) BENCHMARK_DBENCH_BACKEND="nfs" ;;
> +                *) BENCHMARK_DBENCH_BACKEND="sockio" ;;
> +            esac
> +        fi
> +        echo "Using backend $BENCHMARK_DBENCH_BACKEND"
> +        BACKEND="-B $BENCHMARK_DBENCH_BACKEND"
> +    else
> +        echo "This dbench does not support backends"
> +        BACKEND=""
> +    fi
> +
>      hd_test_mount_prepare $BENCHMARK_DBENCH_MOUNT_BLOCKDEV \
>          $BENCHMARK_DBENCH_MOUNT_POINT
> -    if [ -z "$PROGRAM_DBENCH" ]; then
> -        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
> -            ./dbench -B fileio -c ./client.txt \
> -            -D $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
> -            -t $BENCHMARK_DBENCH_TIMELIMIT \
> -            $BENCHMARK_DBENCH_EXTRAPARAMS \
> -            $BENCHMARK_DBENCH_NPROCS; \
> -            sync"
> -    else
> -        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
> -            dbench -D $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
> +    # TODO: timelimit does not seem to work like a timeout as expected
> +    report "$PROGRAM_DBENCH \
>              -t $BENCHMARK_DBENCH_TIMELIMIT \
> +            $BACKEND -c $LOADFILE_ONTARGET \
> +            -D $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
>              $BENCHMARK_DBENCH_EXTRAPARAMS \
>              $BENCHMARK_DBENCH_NPROCS; \
>              sync"
> -    fi
>      hd_test_clean_umount $BENCHMARK_DBENCH_MOUNT_BLOCKDEV \
>          $BENCHMARK_DBENCH_MOUNT_POINT
>  }
> diff --git a/engine/tests/Benchmark.dbench4/spec.json
> b/engine/tests/Benchmark.dbench4/spec.json
> index c855f36..c7df4b3 100644
> --- a/engine/tests/Benchmark.dbench4/spec.json
> +++ b/engine/tests/Benchmark.dbench4/spec.json
> @@ -5,7 +5,9 @@
>              "MOUNT_BLOCKDEV":"ROOT",
>              "MOUNT_POINT":"$BOARD_TESTDIR/work",
>              "TIMELIMIT":"30",
> -            "NPROCS":"2"
> +            "NPROCS":"2",
> +            "LOADFILE":"client.txt",
> +            "BACKEND":"fileio"
>          },
>          "sync": {
>              "MOUNT_BLOCKDEV":"ROOT",
> diff --git a/engine/tests/Benchmark.dbench4/test.yaml
> b/engine/tests/Benchmark.dbench4/test.yaml
> index 8f261ef..10a407a 100644
> --- a/engine/tests/Benchmark.dbench4/test.yaml
> +++ b/engine/tests/Benchmark.dbench4/test.yaml
> @@ -30,6 +30,18 @@ params:
>          description: Number of parallel threads.
>          example: 2
>          optional: no
> +    - LOADFILE:
> +        description: |
> +            One of the loadfiles under the loadfiles/ folder. If an absolute
> +            path is specified it will be interpreted as a file in the target
> +            filesystem to be used as load file.
> +        example: client.txt, nfs.txt, scsi.txt, /usr/share/client.txt
> +        default: client.txt
> +        optional: yes
> +    - BACKEND:
> +        description: the dbench backend
> +        example: fileio, sockio, nfs, scsi, iscsi, smb
> +        optional: yes
>      - EXTRAPARAMS:
>          description: |
>                  Additional parameters such as
> --
> 2.7.4

Looks good overall.  I like the approach.  I'm just a bit 
worried about the use of is_abs_path.  The other changes
I suggested are cosmetic.

Let me know.
 -- Tim

Re: [Fuego] [PATCH 3/3] dbench4: add compatibility with files on the target

[Daniel Sangorrin]

> -----Original Message-----
> From: Tim.Bird@sony.com <Tim.Bird@sony.com>
> > -----Original Message-----
> > From: Daniel Sangorrin on October 30, 2018 2:59 AM
> >
> > Zhong reported a problem when dbench was on the target
> > but client.txt was absent. This patch adds the ability
> > to specify a loadfile using an absolute path, and
> > the ability to check whether a loadfile is on the target
> > or not.
> >
> > By adding the loadfile parameter to the spec, we can also
> > now specify other loadfiles such as smb.txt or scsi.txt,
> > although I haven't tested that properly because it needs
> > some setup.
> >
> > I noticed that Debian's dbench is 4.0 but does not have a
> > backend (-B parameter). I added a variable to specify that
> > and if it is not specified I try to guess the best one
> > from the loadfile.
> >
> > TODO: add support for socketio
> >
> > Signed-off-by: Daniel Sangorrin <daniel.sangorrin@toshiba.co.jp>
> > Reported-by: Lu Zhong <zhongl.fnst@cn.fujitsu.com>
> > ---
> >  engine/tests/Benchmark.dbench4/fuego_test.sh | 70
> > ++++++++++++++++++++++------
> >  engine/tests/Benchmark.dbench4/spec.json     |  4 +-
> >  engine/tests/Benchmark.dbench4/test.yaml     | 12 +++++
> >  3 files changed, 72 insertions(+), 14 deletions(-)
> >
> > diff --git a/engine/tests/Benchmark.dbench4/fuego_test.sh
> > b/engine/tests/Benchmark.dbench4/fuego_test.sh
> > index 535bdd9..14942d3 100755
> > --- a/engine/tests/Benchmark.dbench4/fuego_test.sh
> > +++ b/engine/tests/Benchmark.dbench4/fuego_test.sh
> > @@ -7,6 +7,7 @@ function test_pre_check {
> >      assert_define BENCHMARK_DBENCH_TIMELIMIT
> >      assert_define BENCHMARK_DBENCH_NPROCS
> >
> > +    # check if dbench 4+ exists on the target
> >      is_on_target_path dbench PROGRAM_DBENCH
> >      if [ ! -z "$PROGRAM_DBENCH" ]; then
> >          help=$(cmd "dbench --help") || true
> > @@ -19,6 +20,26 @@ function test_pre_check {
> >              echo "dbench found on the target (version $version)."
> >          fi
> >      fi
> > +
> > +    # check if the loadfile exists on the target
> > +    LOADFILE=${BENCHMARK_DBENCH_LOADFILE:-client.txt}
> > +    LOADFILE_ONTARGET=""
> > +    if is_abs_path $LOADFILE; then
> I'm not familiar with "is_abs_path" .  Is this a shell function
> or a program?

Oouch sorry, it seems that I hadn't merged that function into my repository. It should be there now, I sent it to the list as well.

> > +        if $(cmd "test -e $LOADFILE"); then
> > +            LOADFILE_ONTARGET=$LOADFILE
> > +            echo "$LOADFILE_ONTARGET found on the target."
> > +        else
> > +            echo "Warning: $LOADFILE not found on the target, using Fuego's
> > default."
> > +            LOADFILE_ONTARGET=""
> > +            LOADFILE="client.txt"
> > +        fi
> > +    else
> > +        # check if the file is on the target board
> > +        if $(cmd "test -e /usr/share/dbench/$LOADFILE"); then
> > +            LOADFILE_ONTARGET="/usr/share/dbench/$LOADFILE"
> > +            echo "$LOADFILE_ONTARGET found on the target."
> > +        fi
> > +    fi
> >  }
> >
> >  function test_build {
> > @@ -41,29 +62,52 @@ function test_build {
> >
> >  function test_deploy {
> >      if [ -z "$PROGRAM_DBENCH" ]; then
> > -        put dbench loadfiles/client.txt $BOARD_TESTDIR/fuego.$TESTDIR/
> > +        echo "Deploying dbench."
> > +        put dbench $BOARD_TESTDIR/fuego.$TESTDIR/
> > +        PROGRAM_DBENCH="$BOARD_TESTDIR/fuego.$TESTDIR/dbench"
> > +    fi
> > +
> > +    if [ -z "$LOADFILE_ONTARGET" ]; then
> > +        echo "Deploying loadfiles/$LOADFILE."
> > +        put loadfiles/$LOADFILE $BOARD_TESTDIR/fuego.$TESTDIR/
> > +
> LOADFILE_ONTARGET="$BOARD_TESTDIR/fuego.$TESTDIR/$LOADFILE"
> >      fi
> >  }
> >
> >  function test_run {
> > +    # check if the backend needs to be specified
> > +    HELP=$(cmd "$PROGRAM_DBENCH --help") || true
> > +    HELP_BACKEND=$(echo $HELP | grep backend) || true
> Couldn't this just be: $(cmd "$PROGRAM_DBENCH --help" | grep backend) || true ?

Yes, indeed. I was having problems because I didn't put true, but when I solved I forgot to put them into one sentence.

> > +    if [ ! -z "$HELP_BACKEND" ]; then
> if [ -n "$HELP_BACKEND" ]; then

OK

> > +        echo "This dbench supports backends"
> > +        if [ -z "$BENCHMARK_DBENCH_BACKEND" ]; then
> > +            echo "Warning: backend not specified, trying to guess one."
> > +            case $(basename $LOADFILE) in
> > +                client.txt) BENCHMARK_DBENCH_BACKEND="fileio" ;;
> > +                iscsi*) BENCHMARK_DBENCH_BACKEND="iscsi" ;;
> > +                scsi.txt) BENCHMARK_DBENCH_BACKEND="scsi" ;;
> > +                smb*) BENCHMARK_DBENCH_BACKEND="smb" ;;
> > +                nfs*) BENCHMARK_DBENCH_BACKEND="nfs" ;;
> > +                *) BENCHMARK_DBENCH_BACKEND="sockio" ;;
> > +            esac
> > +        fi
> > +        echo "Using backend $BENCHMARK_DBENCH_BACKEND"
> > +        BACKEND="-B $BENCHMARK_DBENCH_BACKEND"
> > +    else
> > +        echo "This dbench does not support backends"
> > +        BACKEND=""
> > +    fi
> > +
> >      hd_test_mount_prepare $BENCHMARK_DBENCH_MOUNT_BLOCKDEV \
> >          $BENCHMARK_DBENCH_MOUNT_POINT
> > -    if [ -z "$PROGRAM_DBENCH" ]; then
> > -        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
> > -            ./dbench -B fileio -c ./client.txt \
> > -            -D $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
> > -            -t $BENCHMARK_DBENCH_TIMELIMIT \
> > -            $BENCHMARK_DBENCH_EXTRAPARAMS \
> > -            $BENCHMARK_DBENCH_NPROCS; \
> > -            sync"
> > -    else
> > -        report "cd $BOARD_TESTDIR/fuego.$TESTDIR; \
> > -            dbench -D
> $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
> > +    # TODO: timelimit does not seem to work like a timeout as expected
> > +    report "$PROGRAM_DBENCH \
> >              -t $BENCHMARK_DBENCH_TIMELIMIT \
> > +            $BACKEND -c $LOADFILE_ONTARGET \
> > +            -D $BENCHMARK_DBENCH_MOUNT_POINT/fuego.$TESTDIR \
> >              $BENCHMARK_DBENCH_EXTRAPARAMS \
> >              $BENCHMARK_DBENCH_NPROCS; \
> >              sync"
> > -    fi
> >      hd_test_clean_umount $BENCHMARK_DBENCH_MOUNT_BLOCKDEV \
> >          $BENCHMARK_DBENCH_MOUNT_POINT
> >  }
> > diff --git a/engine/tests/Benchmark.dbench4/spec.json
> > b/engine/tests/Benchmark.dbench4/spec.json
> > index c855f36..c7df4b3 100644
> > --- a/engine/tests/Benchmark.dbench4/spec.json
> > +++ b/engine/tests/Benchmark.dbench4/spec.json
> > @@ -5,7 +5,9 @@
> >              "MOUNT_BLOCKDEV":"ROOT",
> >              "MOUNT_POINT":"$BOARD_TESTDIR/work",
> >              "TIMELIMIT":"30",
> > -            "NPROCS":"2"
> > +            "NPROCS":"2",
> > +            "LOADFILE":"client.txt",
> > +            "BACKEND":"fileio"
> >          },
> >          "sync": {
> >              "MOUNT_BLOCKDEV":"ROOT",
> > diff --git a/engine/tests/Benchmark.dbench4/test.yaml
> > b/engine/tests/Benchmark.dbench4/test.yaml
> > index 8f261ef..10a407a 100644
> > --- a/engine/tests/Benchmark.dbench4/test.yaml
> > +++ b/engine/tests/Benchmark.dbench4/test.yaml
> > @@ -30,6 +30,18 @@ params:
> >          description: Number of parallel threads.
> >          example: 2
> >          optional: no
> > +    - LOADFILE:
> > +        description: |
> > +            One of the loadfiles under the loadfiles/ folder. If an absolute
> > +            path is specified it will be interpreted as a file in the target
> > +            filesystem to be used as load file.
> > +        example: client.txt, nfs.txt, scsi.txt, /usr/share/client.txt
> > +        default: client.txt
> > +        optional: yes
> > +    - BACKEND:
> > +        description: the dbench backend
> > +        example: fileio, sockio, nfs, scsi, iscsi, smb
> > +        optional: yes
> >      - EXTRAPARAMS:
> >          description: |
> >                  Additional parameters such as
> > --
> > 2.7.4
> 
> Looks good overall.  I like the approach.  I'm just a bit
> worried about the use of is_abs_path.  The other changes
> I suggested are cosmetic.
> 
> Let me know.

Sorry about not sending is_abs_path.
Thanks for the review!
Daniel