diff mbox

[v7,2/2] block/vxhs.c: Add qemu-iotests for new block device type "vxhs"

Message ID 1486527494-3387-2-git-send-email-Ashish.Mittal@veritas.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ashish Mittal Feb. 8, 2017, 4:18 a.m. UTC 
From: Ashish Mittal <ashish.mittal@veritas.com>

These changes use a vxhs test server that is a part of the following
repository:
https://github.com/VeritasHyperScale/libqnio.git

Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com>
---
v7 changelog:
(1) No changes.

v6 changelog:
(1) Added iotests for VxHS block device.

 tests/qemu-iotests/common        |  6 ++++++
 tests/qemu-iotests/common.config | 13 +++++++++++++
 tests/qemu-iotests/common.filter |  1 +
 tests/qemu-iotests/common.rc     | 19 +++++++++++++++++++
 4 files changed, 39 insertions(+)

Comments

Stefan Hajnoczi Feb. 13, 2017, 2:43 p.m. UTC | #1 
On Tue, Feb 07, 2017 at 08:18:14PM -0800, Ashish Mittal wrote:
> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
> index f6384fb..c7a80c0 100644
> --- a/tests/qemu-iotests/common.config
> +++ b/tests/qemu-iotests/common.config
> @@ -105,6 +105,10 @@ if [ -z "$QEMU_NBD_PROG" ]; then
>      export QEMU_NBD_PROG="`set_prog_path qemu-nbd`"
>  fi
>  
> +if [ -z "$QEMU_VXHS_PROG" ]; then
> +    export QEMU_VXHS_PROG="`set_prog_path qnio_server /usr/local/bin`"

Did you test this with /usr/local/bin/qnio_server?

I think it will evaluate to QEMU_VXHS_PROG=/usr/local/bin when qnio_server
isn't found in PATH.  You probably wanted /usr/local/bin/qnio_server instead.

I suggest dropping the second argument completely and letting the user set PATH
themselves.  No existing set_prog_path caller uses the second argument.

# $1 = prog to look for, $2* = default pathnames if not found in $PATH
set_prog_path()
{
    p=`command -v $1 2> /dev/null`
    if [ -n "$p" -a -x "$p" ]; then
        echo $p
        return 0
    fi
    p=$1

    shift
    for f; do
        if [ -x $f ]; then
            echo $f
            return 0
        fi
    done

    echo ""
    return 1
}

> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 3213765..06a3164 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -89,6 +89,9 @@ else
>          TEST_IMG=$TEST_DIR/t.$IMGFMT
>      elif [ "$IMGPROTO" = "archipelago" ]; then
>          TEST_IMG="archipelago:at.$IMGFMT"
> +    elif [ "$IMGPROTO" = "vxhs" ]; then
> +        TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
> +        TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT"
>      else
>          TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
>      fi
> @@ -175,6 +178,12 @@ _make_test_img()
>          eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  $TEST_IMG_FILE &"
>          sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
>      fi
> +
> +    # Start QNIO server on image directory for vxhs protocol
> +    if [ $IMGPROTO = "vxhs" ]; then
> +        eval "$QEMU_VXHS -d  $TEST_DIR &"
> +        sleep 1 # Wait for server to come up.

This is a pre-existing problem and you don't need to fix it now:

We should replace sleep 1 with a function that probes the TCP port until the
connection can be established or a timeout is reached.  The netcat (nc) utility
is often used for this.

sleep 1 is not reliable and may fail on a heavily loaded machine like the
Travis-CI build machines that are used.
Ashish Mittal Feb. 16, 2017, 2:59 a.m. UTC | #2 
On Mon, Feb 13, 2017 at 6:43 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Feb 07, 2017 at 08:18:14PM -0800, Ashish Mittal wrote:
>> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
>> index f6384fb..c7a80c0 100644
>> --- a/tests/qemu-iotests/common.config
>> +++ b/tests/qemu-iotests/common.config
>> @@ -105,6 +105,10 @@ if [ -z "$QEMU_NBD_PROG" ]; then
>>      export QEMU_NBD_PROG="`set_prog_path qemu-nbd`"
>>  fi
>>
>> +if [ -z "$QEMU_VXHS_PROG" ]; then
>> +    export QEMU_VXHS_PROG="`set_prog_path qnio_server /usr/local/bin`"
>
> Did you test this with /usr/local/bin/qnio_server?
>
> I think it will evaluate to QEMU_VXHS_PROG=/usr/local/bin when qnio_server
> isn't found in PATH.  You probably wanted /usr/local/bin/qnio_server instead.
>
> I suggest dropping the second argument completely and letting the user set PATH
> themselves.  No existing set_prog_path caller uses the second argument.
>
> # $1 = prog to look for, $2* = default pathnames if not found in $PATH
> set_prog_path()
> {
>     p=`command -v $1 2> /dev/null`
>     if [ -n "$p" -a -x "$p" ]; then
>         echo $p
>         return 0
>     fi
>     p=$1
>
>     shift
>     for f; do
>         if [ -x $f ]; then
>             echo $f
>             return 0
>         fi
>     done
>
>     echo ""
>     return 1
> }
>
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 3213765..06a3164 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -89,6 +89,9 @@ else
>>          TEST_IMG=$TEST_DIR/t.$IMGFMT
>>      elif [ "$IMGPROTO" = "archipelago" ]; then
>>          TEST_IMG="archipelago:at.$IMGFMT"
>> +    elif [ "$IMGPROTO" = "vxhs" ]; then
>> +        TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> +        TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT"
>>      else
>>          TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
>>      fi
>> @@ -175,6 +178,12 @@ _make_test_img()
>>          eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  $TEST_IMG_FILE &"
>>          sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
>>      fi
>> +
>> +    # Start QNIO server on image directory for vxhs protocol
>> +    if [ $IMGPROTO = "vxhs" ]; then
>> +        eval "$QEMU_VXHS -d  $TEST_DIR &"
>> +        sleep 1 # Wait for server to come up.
>
> This is a pre-existing problem and you don't need to fix it now:
>
> We should replace sleep 1 with a function that probes the TCP port until the
> connection can be established or a timeout is reached.  The netcat (nc) utility
> is often used for this.
>
> sleep 1 is not reliable and may fail on a heavily loaded machine like the
> Travis-CI build machines that are used.
Ashish Mittal Feb. 16, 2017, 3:11 a.m. UTC | #3 
Sorry, pressed the "send" button instead of "expand text" on the
previous email ...

On Mon, Feb 13, 2017 at 6:43 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Feb 07, 2017 at 08:18:14PM -0800, Ashish Mittal wrote:
>> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
>> index f6384fb..c7a80c0 100644
>> --- a/tests/qemu-iotests/common.config
>> +++ b/tests/qemu-iotests/common.config
>> @@ -105,6 +105,10 @@ if [ -z "$QEMU_NBD_PROG" ]; then
>>      export QEMU_NBD_PROG="`set_prog_path qemu-nbd`"
>>  fi
>>
>> +if [ -z "$QEMU_VXHS_PROG" ]; then
>> +    export QEMU_VXHS_PROG="`set_prog_path qnio_server /usr/local/bin`"
>
> Did you test this with /usr/local/bin/qnio_server?
>
> I think it will evaluate to QEMU_VXHS_PROG=/usr/local/bin when qnio_server
> isn't found in PATH.  You probably wanted /usr/local/bin/qnio_server instead.
>
> I suggest dropping the second argument completely and letting the user set PATH
> themselves.  No existing set_prog_path caller uses the second argument.
>

You're right. Will drop the second argument.

> # $1 = prog to look for, $2* = default pathnames if not found in $PATH
> set_prog_path()
> {
>     p=`command -v $1 2> /dev/null`
>     if [ -n "$p" -a -x "$p" ]; then
>         echo $p
>         return 0
>     fi
>     p=$1
>
>     shift
>     for f; do
>         if [ -x $f ]; then
>             echo $f
>             return 0
>         fi
>     done
>
>     echo ""
>     return 1
> }
>
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index 3213765..06a3164 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -89,6 +89,9 @@ else
>>          TEST_IMG=$TEST_DIR/t.$IMGFMT
>>      elif [ "$IMGPROTO" = "archipelago" ]; then
>>          TEST_IMG="archipelago:at.$IMGFMT"
>> +    elif [ "$IMGPROTO" = "vxhs" ]; then
>> +        TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
>> +        TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT"
>>      else
>>          TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
>>      fi
>> @@ -175,6 +178,12 @@ _make_test_img()
>>          eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  $TEST_IMG_FILE &"
>>          sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
>>      fi
>> +
>> +    # Start QNIO server on image directory for vxhs protocol
>> +    if [ $IMGPROTO = "vxhs" ]; then
>> +        eval "$QEMU_VXHS -d  $TEST_DIR &"
>> +        sleep 1 # Wait for server to come up.
>
> This is a pre-existing problem and you don't need to fix it now:
>
> We should replace sleep 1 with a function that probes the TCP port until the
> connection can be established or a timeout is reached.  The netcat (nc) utility
> is often used for this.
>
> sleep 1 is not reliable and may fail on a heavily loaded machine like the
> Travis-CI build machines that are used.

Will skip this one for now.

Thanks,
Ashish
diff mbox

Patch

diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index b6274be..318a81d 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -158,6 +158,7 @@  check options
     -nfs                test nfs
     -archipelago        test archipelago
     -luks               test luks
+    -vxhs               test vxhs
     -xdiff              graphical mode diff
     -nocache            use O_DIRECT on backing file
     -misalign           misalign memory allocations
@@ -261,6 +262,11 @@  testlist options
             xpand=false
             ;;
 
+        -vxhs)
+            IMGPROTO=vxhs
+            xpand=false
+            ;;
+
         -ssh)
             IMGPROTO=ssh
             xpand=false
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index f6384fb..c7a80c0 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -105,6 +105,10 @@  if [ -z "$QEMU_NBD_PROG" ]; then
     export QEMU_NBD_PROG="`set_prog_path qemu-nbd`"
 fi
 
+if [ -z "$QEMU_VXHS_PROG" ]; then
+    export QEMU_VXHS_PROG="`set_prog_path qnio_server /usr/local/bin`"
+fi
+
 _qemu_wrapper()
 {
     (
@@ -156,10 +160,19 @@  _qemu_nbd_wrapper()
     )
 }
 
+_qemu_vxhs_wrapper()
+{
+    (
+        echo $BASHPID > "${TEST_DIR}/qemu-vxhs.pid"
+        exec "$QEMU_VXHS_PROG" $QEMU_VXHS_OPTIONS "$@"
+    )
+}
+
 export QEMU=_qemu_wrapper
 export QEMU_IMG=_qemu_img_wrapper
 export QEMU_IO=_qemu_io_wrapper
 export QEMU_NBD=_qemu_nbd_wrapper
+export QEMU_VXHS=_qemu_vxhs_wrapper
 
 QEMU_IMG_EXTRA_ARGS=
 if [ "$IMGOPTSSYNTAX" = "true" ]; then
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 240ed06..a8a4d0e 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -123,6 +123,7 @@  _filter_img_info()
         -e "s#$TEST_DIR#TEST_DIR#g" \
         -e "s#$IMGFMT#IMGFMT#g" \
         -e 's#nbd://127.0.0.1:10810$#TEST_DIR/t.IMGFMT#g' \
+        -e 's#json.*vdisk-id.*vxhs"}}#TEST_DIR/t.IMGFMT#' \
         -e "/encrypted: yes/d" \
         -e "/cluster_size: [0-9]\\+/d" \
         -e "/table_size: [0-9]\\+/d" \
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 3213765..06a3164 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -89,6 +89,9 @@  else
         TEST_IMG=$TEST_DIR/t.$IMGFMT
     elif [ "$IMGPROTO" = "archipelago" ]; then
         TEST_IMG="archipelago:at.$IMGFMT"
+    elif [ "$IMGPROTO" = "vxhs" ]; then
+        TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
+        TEST_IMG="vxhs://127.0.0.1:9999/t.$IMGFMT"
     else
         TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
     fi
@@ -175,6 +178,12 @@  _make_test_img()
         eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 -f $IMGFMT  $TEST_IMG_FILE &"
         sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
     fi
+
+    # Start QNIO server on image directory for vxhs protocol
+    if [ $IMGPROTO = "vxhs" ]; then
+        eval "$QEMU_VXHS -d  $TEST_DIR &"
+        sleep 1 # Wait for server to come up.
+    fi
 }
 
 _rm_test_img()
@@ -201,6 +210,16 @@  _cleanup_test_img()
             fi
             rm -f "$TEST_IMG_FILE"
             ;;
+        vxhs)
+            if [ -f "${TEST_DIR}/qemu-vxhs.pid" ]; then
+                local QEMU_VXHS_PID
+                read QEMU_VXHS_PID < "${TEST_DIR}/qemu-vxhs.pid"
+                kill ${QEMU_VXHS_PID} >/dev/null 2>&1
+                rm -f "${TEST_DIR}/qemu-vxhs.pid"
+            fi
+            rm -f "$TEST_IMG_FILE"
+            ;;
+
         file)
             _rm_test_img "$TEST_DIR/t.$IMGFMT"
             _rm_test_img "$TEST_DIR/t.$IMGFMT.orig"