[Qemu-devel] [PATCH] iotests: Use configured python

[Qemu-devel] [PATCH] iotests: Use configured python

[Max Reitz]

Currently, QEMU's iotests rely on /usr/bin/env to start the correct
Python (that is, at least Python 2.4, but not 3). On systems where
Python 3 is the default, the user has no clean way of making the iotests
use the correct binary.

This commit makes the iotests use the Python selected by configure.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 configure                |  6 ++++++
 tests/qemu-iotests/031   |  9 +++++----
 tests/qemu-iotests/036   |  7 ++++---
 tests/qemu-iotests/039   | 19 ++++++++++---------
 tests/qemu-iotests/054   |  3 ++-
 tests/qemu-iotests/060   | 21 +++++++++++----------
 tests/qemu-iotests/061   | 25 +++++++++++++------------
 tests/qemu-iotests/065   |  2 +-
 tests/qemu-iotests/083   |  3 ++-
 tests/qemu-iotests/check | 18 ++++++++++++++++--
 10 files changed, 70 insertions(+), 43 deletions(-)

diff --git a/configure b/configure
index 2fbec59..93654d1 100755
--- a/configure
+++ b/configure
@@ -4766,6 +4766,12 @@ if test "$gcov" = "yes" ; then
   echo "GCOV=$gcov_tool" >> $config_host_mak
 fi
 
+iotests_common_env="tests/qemu-iotests/common.env"
+
+echo "# Automatically generated by configure - do not modify" > $iotests_common_env
+echo >> $iotests_common_env
+echo "PYTHON='$python'" >> $iotests_common_env
+
 # use included Linux headers
 if test "$linux" = "yes" ; then
   mkdir -p linux-headers
diff --git a/tests/qemu-iotests/031 b/tests/qemu-iotests/031
index 1d920ea..5aefb88 100755
--- a/tests/qemu-iotests/031
+++ b/tests/qemu-iotests/031
@@ -35,6 +35,7 @@ _cleanup()
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # get standard environment, filters and checks
+. ./common.env
 . ./common.rc
 . ./common.filter
 . ./common.pattern
@@ -56,22 +57,22 @@ for IMGOPTS in "compat=0.10" "compat=1.1"; do
     echo === Create image with unknown header extension ===
     echo
     _make_test_img 64M
-    ./qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
-    ./qcow2.py "$TEST_IMG" dump-header
+    $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
+    $PYTHON qcow2.py "$TEST_IMG" dump-header
     _check_test_img
 
     echo
     echo === Rewrite header with no backing file ===
     echo
     $QEMU_IMG rebase -u -b "" "$TEST_IMG"
-    ./qcow2.py "$TEST_IMG" dump-header
+    $PYTHON qcow2.py "$TEST_IMG" dump-header
     _check_test_img
 
     echo
     echo === Add a backing file and format ===
     echo
     $QEMU_IMG rebase -u -b "/some/backing/file/path" -F host_device "$TEST_IMG"
-    ./qcow2.py "$TEST_IMG" dump-header
+    $PYTHON qcow2.py "$TEST_IMG" dump-header
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
index 03b6aa9..29c35d1 100755
--- a/tests/qemu-iotests/036
+++ b/tests/qemu-iotests/036
@@ -38,6 +38,7 @@ _cleanup()
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # get standard environment, filters and checks
+. ./common.env
 . ./common.rc
 . ./common.filter
 . ./common.pattern
@@ -53,15 +54,15 @@ IMGOPTS="compat=1.1"
 echo === Create image with unknown autoclear feature bit ===
 echo
 _make_test_img 64M
-./qcow2.py "$TEST_IMG" set-feature-bit autoclear 63
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit autoclear 63
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 
 echo
 echo === Repair image ===
 echo
 _check_test_img -r all
 
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 
 # success, all done
 echo "*** done"
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index b9cbe99..b7b7030 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -38,6 +38,7 @@ _cleanup()
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # get standard environment, filters and checks
+. ./common.env
 . ./common.rc
 . ./common.filter
 
@@ -58,7 +59,7 @@ _make_test_img $size
 $QEMU_IO -c "write -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io
 
 # The dirty bit must not be set
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 _check_test_img
 
 echo
@@ -73,7 +74,7 @@ $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" | _filter_qemu_io
 ulimit -c "$old_ulimit"
 
 # The dirty bit must be set
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 _check_test_img
 
 echo
@@ -82,7 +83,7 @@ echo "== Read-only access must still work =="
 $QEMU_IO -r -c "read -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io
 
 # The dirty bit must be set
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 echo
 echo "== Repairing the image file must succeed =="
@@ -90,7 +91,7 @@ echo "== Repairing the image file must succeed =="
 _check_test_img -r all
 
 # The dirty bit must not be set
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 echo
 echo "== Data should still be accessible after repair =="
@@ -109,12 +110,12 @@ $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" | _filter_qemu_io
 ulimit -c "$old_ulimit"
 
 # The dirty bit must be set
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io
 
 # The dirty bit must not be set
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 echo
 echo "== Creating an image file with lazy_refcounts=off =="
@@ -128,7 +129,7 @@ $QEMU_IO -c "write -P 0x5a 0 512" -c "abort" "$TEST_IMG" | _filter_qemu_io
 ulimit -c "$old_ulimit"
 
 # The dirty bit must not be set since lazy_refcounts=off
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 _check_test_img
 
 echo
@@ -144,8 +145,8 @@ $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IMG commit "$TEST_IMG"
 
 # The dirty bit must not be set
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
-./qcow2.py "$TEST_IMG".base dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG".base dump-header | grep incompatible_features
 
 _check_test_img
 TEST_IMG="$TEST_IMG".base _check_test_img
diff --git a/tests/qemu-iotests/054 b/tests/qemu-iotests/054
index c8b7082..a5ebf99 100755
--- a/tests/qemu-iotests/054
+++ b/tests/qemu-iotests/054
@@ -35,6 +35,7 @@ _cleanup()
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # get standard environment, filters and checks
+. ./common.env
 . ./common.rc
 . ./common.filter
 
@@ -49,7 +50,7 @@ _make_test_img $((1024*1024))T
 echo
 echo "creating too large image (1 EB) using qcow2.py"
 _make_test_img 4G
-./qcow2.py "$TEST_IMG" set-header size $((1024 ** 6))
+$PYTHON qcow2.py "$TEST_IMG" set-header size $((1024 ** 6))
 _check_test_img
 
 # success, all done
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index f0116aa..5447b27 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -35,6 +35,7 @@ _cleanup()
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # get standard environment, filters and checks
+. ./common.env
 . ./common.rc
 . ./common.filter
 
@@ -68,13 +69,13 @@ poke_file "$TEST_IMG" "$l1_offset" "\x80\x00\x00\x00\x00\x03\x00\x00"
 _check_test_img
 
 # The corrupt bit should not be set anyway
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 # Try to write something, thereby forcing the corrupt bit to be set
 $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io
 
 # The corrupt bit must now be set
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 # Try to open the image R/W (which should fail)
 $QEMU_IO -c "$OPEN_RW" -c "read 0 512" 2>&1 | _filter_qemu_io \
@@ -99,19 +100,19 @@ poke_file "$TEST_IMG" "$(($rb_offset+8))" "\x00\x01"
 # Redirect new data cluster onto refcount block
 poke_file "$TEST_IMG" "$l2_offset" "\x80\x00\x00\x00\x00\x02\x00\x00"
 _check_test_img
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 # Try to fix it
 _check_test_img -r all
 
 # The corrupt bit should be cleared
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 # Look if it's really really fixed
 $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 echo
 echo "=== Testing cluster data reference into inactive L2 table ==="
@@ -124,13 +125,13 @@ $QEMU_IO -c "$OPEN_RW" -c "write -P 2 0 512" | _filter_qemu_io
 poke_file "$TEST_IMG" "$l2_offset_after_snapshot" \
                       "\x80\x00\x00\x00\x00\x04\x00\x00"
 _check_test_img
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 $QEMU_IO -c "$OPEN_RW" -c "write -P 3 0 512" | _filter_qemu_io
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 _check_test_img -r all
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 $QEMU_IO -c "$OPEN_RW" -c "write -P 4 0 512" | _filter_qemu_io
-./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 # Check data
 $QEMU_IO -c "$OPEN_RO" -c "read -P 4 0 512" | _filter_qemu_io
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index d3a6b38..0de7897 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -35,6 +35,7 @@ _cleanup()
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # get standard environment, filters and checks
+. ./common.env
 . ./common.rc
 . ./common.filter
 
@@ -48,9 +49,9 @@ echo "=== Testing version downgrade with zero expansion ==="
 echo
 IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
 $QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
 
@@ -59,9 +60,9 @@ echo "=== Testing dirty version downgrade ==="
 echo
 IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
 $QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _filter_qemu_io
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
 
@@ -69,11 +70,11 @@ echo
 echo "=== Testing version downgrade with unknown compat/autoclear flags ==="
 echo
 IMGOPTS="compat=1.1" _make_test_img 64M
-./qcow2.py "$TEST_IMG" set-feature-bit compatible 42
-./qcow2.py "$TEST_IMG" set-feature-bit autoclear 42
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit compatible 42
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit autoclear 42
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 _check_test_img
 
 echo
@@ -81,9 +82,9 @@ echo "=== Testing version upgrade and resize ==="
 echo
 IMGOPTS="compat=0.10" _make_test_img 64M
 $QEMU_IO -c "write -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IMG amend -o "compat=1.1,lazy_refcounts=on,size=128M" "$TEST_IMG"
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IO -c "read -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
 
@@ -92,9 +93,9 @@ echo "=== Testing dirty lazy_refcounts=off ==="
 echo
 IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
 $QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _filter_qemu_io
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IMG amend -o "lazy_refcounts=off" "$TEST_IMG"
-./qcow2.py "$TEST_IMG" dump-header
+$PYTHON qcow2.py "$TEST_IMG" dump-header
 $QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
 
diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index ab5445f..e89b61d 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -1,4 +1,4 @@
-#!/usr/bin/env python2
+#!/usr/bin/env python
 #
 # Test for additional information emitted by qemu-img info on qcow2
 # images
diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
index f764534..6a52c96 100755
--- a/tests/qemu-iotests/083
+++ b/tests/qemu-iotests/083
@@ -29,6 +29,7 @@ tmp=/tmp/$$
 status=1	# failure is the default!
 
 # get standard environment, filters and checks
+. ./common.env
 . ./common.rc
 . ./common.filter
 
@@ -81,7 +82,7 @@ EOF
 		nbd_url="nbd:127.0.0.1:$port:exportname=foo"
 	fi
 
-	./nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" 2>&1 >/dev/null &
+	$PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" 2>&1 >/dev/null &
 	wait_for_tcp_port "127.0.0.1:$port"
 	$QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | filter_nbd
 
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index e2ed5a9..ca2ee43 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -34,6 +34,13 @@ timestamp=${TIMESTAMP:=false}
 # generic initialization
 iam=check
 
+# we need common.env
+if ! . ./common.env
+then
+    echo "$iam: failed to source common.env"
+    exit 1
+fi
+
 # we need common.config
 if ! . ./common.config
 then
@@ -215,9 +222,16 @@ do
 
         start=`_wallclock`
         $timestamp && echo -n "        ["`date "+%T"`"]"
-        [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
+
+        if [ "$(head -n 1 $seq)" == "#!/usr/bin/env python" ]; then
+            run_command="$PYTHON $seq"
+        else
+            [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
+            run_command="./$seq"
+        fi
+
         MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
-                ./$seq >$tmp.out 2>&1
+                $run_command >$tmp.out 2>&1
         sts=$?
         $timestamp && _timestamp
         stop=`_wallclock`
-- 
1.9.2

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Stefan Hajnoczi]

On Sat, May 03, 2014 at 04:47:08PM +0200, Max Reitz wrote:
> @@ -56,22 +57,22 @@ for IMGOPTS in "compat=0.10" "compat=1.1"; do
>      echo === Create image with unknown header extension ===
>      echo
>      _make_test_img 64M
> -    ./qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
> -    ./qcow2.py "$TEST_IMG" dump-header
> +    $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
> +    $PYTHON qcow2.py "$TEST_IMG" dump-header

Please use "$PYTHON" to humor the people who like to put spaces in their
path names.

> @@ -215,9 +222,16 @@ do
>  
>          start=`_wallclock`
>          $timestamp && echo -n "        ["`date "+%T"`"]"
> -        [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
> +
> +        if [ "$(head -n 1 $seq)" == "#!/usr/bin/env python" ]; then
> +            run_command="$PYTHON $seq"

The code generally uses the older `` notation instead of $().  Please
use ``.

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Peter Maydell]

On 5 May 2014 13:26, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Sat, May 03, 2014 at 04:47:08PM +0200, Max Reitz wrote:
>> @@ -56,22 +57,22 @@ for IMGOPTS in "compat=0.10" "compat=1.1"; do
>>      echo === Create image with unknown header extension ===
>>      echo
>>      _make_test_img 64M
>> -    ./qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
>> -    ./qcow2.py "$TEST_IMG" dump-header
>> +    $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
>> +    $PYTHON qcow2.py "$TEST_IMG" dump-header
>
> Please use "$PYTHON" to humor the people who like to put spaces in their
> path names.

Won't that cause problems if configure sets $PYTHON to "python -B" ?

>> @@ -215,9 +222,16 @@ do
>>
>>          start=`_wallclock`
>>          $timestamp && echo -n "        ["`date "+%T"`"]"
>> -        [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
>> +
>> +        if [ "$(head -n 1 $seq)" == "#!/usr/bin/env python" ]; then
>> +            run_command="$PYTHON $seq"
>
> The code generally uses the older `` notation instead of $().  Please
> use ``.

$() is nicer though...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On 05/05/2014 06:26 AM, Stefan Hajnoczi wrote:

>> -        [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
>> +
>> +        if [ "$(head -n 1 $seq)" == "#!/usr/bin/env python" ]; then
>> +            run_command="$PYTHON $seq"
> 
> The code generally uses the older `` notation instead of $().  Please
> use ``.

I'd rather scrub the sources to get rid of all `` and use $() instead.
`` is obsolete.  Sounds like a good beginner's project, if someone wants
a summer of code warmup...

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Max Reitz]

On 05.05.2014 14:26, Stefan Hajnoczi wrote:
> On Sat, May 03, 2014 at 04:47:08PM +0200, Max Reitz wrote:
>> @@ -56,22 +57,22 @@ for IMGOPTS in "compat=0.10" "compat=1.1"; do
>>       echo === Create image with unknown header extension ===
>>       echo
>>       _make_test_img 64M
>> -    ./qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
>> -    ./qcow2.py "$TEST_IMG" dump-header
>> +    $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
>> +    $PYTHON qcow2.py "$TEST_IMG" dump-header
> Please use "$PYTHON" to humor the people who like to put spaces in their
> path names.

Following on Peter's explanation, me using ./configure --python=python2 
results in PYTHON='python2 -B', which probably won't work so well with 
quotes around it.

>> @@ -215,9 +222,16 @@ do
>>   
>>           start=`_wallclock`
>>           $timestamp && echo -n "        ["`date "+%T"`"]"
>> -        [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
>> +
>> +        if [ "$(head -n 1 $seq)" == "#!/usr/bin/env python" ]; then
>> +            run_command="$PYTHON $seq"
> The code generally uses the older `` notation instead of $().  Please
> use ``.

If I'd send a v2 with ``, Eric would probably want me to send a v3 with 
$(). ;-)

I personally don't really care what to use, but so far nobody has picked 
on me for using $(), whereas Eric once criticized my use of `` (which I 
had taken over from other tests).

Max

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

On 05/05/2014 10:25 AM, Max Reitz wrote:
>> The code generally uses the older `` notation instead of $().  Please
>> use ``.
> 
> If I'd send a v2 with ``, Eric would probably want me to send a v3 with
> $(). ;-)

I won't make you resend if you are consistent with other code in the
same file.  But I also won't object to someone tackling a generic
cleanup series to nuke ALL use of `` in the codebase.

> 
> I personally don't really care what to use, but so far nobody has picked
> on me for using $(), whereas Eric once criticized my use of `` (which I
> had taken over from other tests).

Consistency trumps aesthetics; I can point out obsolete usages, but
won't reject a commit that is self-consistent in the use of that construct.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Stefan Hajnoczi]

On Mon, May 05, 2014 at 06:25:38PM +0200, Max Reitz wrote:
> On 05.05.2014 14:26, Stefan Hajnoczi wrote:
> >On Sat, May 03, 2014 at 04:47:08PM +0200, Max Reitz wrote:
> >>@@ -56,22 +57,22 @@ for IMGOPTS in "compat=0.10" "compat=1.1"; do
> >>      echo === Create image with unknown header extension ===
> >>      echo
> >>      _make_test_img 64M
> >>-    ./qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
> >>-    ./qcow2.py "$TEST_IMG" dump-header
> >>+    $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
> >>+    $PYTHON qcow2.py "$TEST_IMG" dump-header
> >Please use "$PYTHON" to humor the people who like to put spaces in their
> >path names.
> 
> Following on Peter's explanation, me using ./configure
> --python=python2 results in PYTHON='python2 -B', which probably
> won't work so well with quotes around it.

You have a point, let's take the patch as-is.

> >>@@ -215,9 +222,16 @@ do
> >>          start=`_wallclock`
> >>          $timestamp && echo -n "        ["`date "+%T"`"]"
> >>-        [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
> >>+
> >>+        if [ "$(head -n 1 $seq)" == "#!/usr/bin/env python" ]; then
> >>+            run_command="$PYTHON $seq"
> >The code generally uses the older `` notation instead of $().  Please
> >use ``.
> 
> If I'd send a v2 with ``, Eric would probably want me to send a v3
> with $(). ;-)
> 
> I personally don't really care what to use, but so far nobody has
> picked on me for using $(), whereas Eric once criticized my use of
> `` (which I had taken over from other tests).

Personally I'm a $() man.  Just pointed it out for consistency but it
seems nobody really likes `` anyway :-).

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Kevin Wolf]

Am 03.05.2014 um 16:47 hat Max Reitz geschrieben:
> Currently, QEMU's iotests rely on /usr/bin/env to start the correct
> Python (that is, at least Python 2.4, but not 3). On systems where
> Python 3 is the default, the user has no clean way of making the iotests
> use the correct binary.
> 
> This commit makes the iotests use the Python selected by configure.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Max, apparently this breaks out-of-tree build from a clean directory:

[16:11] <rth> ../qemu/configure: line 4773:
tests/qemu-iotests/common.env: No such file or directory
[16:11] <rth> eh?
[16:13] <bonzini> kwolf, stefanha: i think you broke srcdir != builddir
[16:14] <pm215> really? my pre-push test suites are all out-of-tree
build (though not ditto from clean)
[16:14] <bonzini> pm215: needs to be clean
[16:14] <bonzini> pm215: tests/qemu-iotests is created at line 5165

Could you post a follow-up to fix this, please?

Kevin

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> Currently, QEMU's iotests rely on /usr/bin/env to start the correct
> Python (that is, at least Python 2.4, but not 3). On systems where
> Python 3 is the default, the user has no clean way of making the iotests
> use the correct binary.
>
> This commit makes the iotests use the Python selected by configure.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

I'm afraid this broke qemu-iotests in a separate build tree:

    ./check: line 38: ./common.env: No such file or directory

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Max Reitz]

On 14.05.2014 14:33, Markus Armbruster wrote:
> Max Reitz <mreitz@redhat.com> writes:
>
>> Currently, QEMU's iotests rely on /usr/bin/env to start the correct
>> Python (that is, at least Python 2.4, but not 3). On systems where
>> Python 3 is the default, the user has no clean way of making the iotests
>> use the correct binary.
>>
>> This commit makes the iotests use the Python selected by configure.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> I'm afraid this broke qemu-iotests in a separate build tree:
>
>      ./check: line 38: ./common.env: No such file or directory

As I already replied to Peter, I see two (or maybe three) ways to fix this:

The first is, we use the correct path to common.env. This would however 
result in modification of the source tree although this is probably not 
what the user intends with an out-of-tree build. On the other hand, this 
would just work.

The second is, we do not create common.env for out-of-tree builds and 
add a default common.env to the repository ("PYTHON = python" should 
probably suffice). This would not introduce any regressions, however, 
the iotests would remain problematic on systems with Python 3 being the 
default when using out-of-tree builds. As I guess that out-of-tree 
builds are actually recommended, this would mean that the better 
solution might be to revert my patch and instead change every "python" 
occurrence in the iotests' Shebangs to "python2", as kind of a third way 
to go. However, for this I'm not sure whether all systems which are 
supposed to be supported by qemu actually have a "python2" 
executable/symlink. I guess so, but then again...

So, either we fix it but try to write to the source tree without the 
user intending to modify it; or we fix it for in-tree builds only; or we 
drop the magic and just use "python2" in the iotests' Shebangs.

Max

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Fam Zheng]

On Thu, 05/15 01:41, Max Reitz wrote:
> On 14.05.2014 14:33, Markus Armbruster wrote:
> >Max Reitz <mreitz@redhat.com> writes:
> >
> >>Currently, QEMU's iotests rely on /usr/bin/env to start the correct
> >>Python (that is, at least Python 2.4, but not 3). On systems where
> >>Python 3 is the default, the user has no clean way of making the iotests
> >>use the correct binary.
> >>
> >>This commit makes the iotests use the Python selected by configure.
> >>
> >>Signed-off-by: Max Reitz <mreitz@redhat.com>
> >I'm afraid this broke qemu-iotests in a separate build tree:
> >
> >     ./check: line 38: ./common.env: No such file or directory
> 
> As I already replied to Peter, I see two (or maybe three) ways to fix this:
> 
> The first is, we use the correct path to common.env. This would however
> result in modification of the source tree although this is probably not what
> the user intends with an out-of-tree build. On the other hand, this would
> just work.
> 
> The second is, we do not create common.env for out-of-tree builds and add a
> default common.env to the repository ("PYTHON = python" should probably
> suffice). This would not introduce any regressions, however, the iotests
> would remain problematic on systems with Python 3 being the default when
> using out-of-tree builds. As I guess that out-of-tree builds are actually
> recommended, this would mean that the better solution might be to revert my
> patch and instead change every "python" occurrence in the iotests' Shebangs
> to "python2", as kind of a third way to go. However, for this I'm not sure
> whether all systems which are supposed to be supported by qemu actually have
> a "python2" executable/symlink. I guess so, but then again...
> 
> So, either we fix it but try to write to the source tree without the user
> intending to modify it; or we fix it for in-tree builds only; or we drop the
> magic and just use "python2" in the iotests' Shebangs.

Why can't we just tell ./check the path to common.env with an env var, like how
we tell ./check the path to qemu-img with QEMU_IMG_PROG?

Fam

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> On 14.05.2014 14:33, Markus Armbruster wrote:
>> Max Reitz <mreitz@redhat.com> writes:
>>
>>> Currently, QEMU's iotests rely on /usr/bin/env to start the correct
>>> Python (that is, at least Python 2.4, but not 3). On systems where
>>> Python 3 is the default, the user has no clean way of making the iotests
>>> use the correct binary.
>>>
>>> This commit makes the iotests use the Python selected by configure.
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> I'm afraid this broke qemu-iotests in a separate build tree:
>>
>>      ./check: line 38: ./common.env: No such file or directory
>
> As I already replied to Peter, I see two (or maybe three) ways to fix this:
>
> The first is, we use the correct path to common.env. This would
> however result in modification of the source tree although this is
> probably not what the user intends with an out-of-tree build. On the
> other hand, this would just work.

Writing to the source tree works only when the write is exactly the same
for every build tree.  Example: autoconf writing configure.

Modifying files under git-control is right out.

> The second is, we do not create common.env for out-of-tree builds and
> add a default common.env to the repository ("PYTHON = python" should
> probably suffice). This would not introduce any regressions, however,
> the iotests would remain problematic on systems with Python 3 being
> the default when using out-of-tree builds.

A difference between an in-tree and an out-of-tree build is a bug.

If plain "python" is good enough for out-of-tree builds, it should be
good enough for in-tree builds.

>                                            As I guess that out-of-tree
> builds are actually recommended,

Correct.

>                                  this would mean that the better
> solution might be to revert my patch and instead change every "python"
> occurrence in the iotests' Shebangs to "python2", as kind of a third
> way to go. However, for this I'm not sure whether all systems which
> are supposed to be supported by qemu actually have a "python2"
> executable/symlink. I guess so, but then again...

I don't know.  Try and find out :)

> So, either we fix it but try to write to the source tree without the
> user intending to modify it; or we fix it for in-tree builds only; or
> we drop the magic and just use "python2" in the iotests' Shebangs.

The problem is including generated bits, namely results of configure,
into source files.

The Autoconf way is to substitute placeholders in FOO.in producing FOO.

When you want to limit .in contents as much as possible, you factor out
the stuff that needs substitutions into some SUB.in, which you then
include into FOO.  Requires a suitable include mechanism.  In shell,
builtin source.

But then you need to find SUB from FOO.  I've seen two ways used:

1. Assume SUB is in the current directory.  Link FOO into the build tree
to make it so.

2. Require FOO to be run in a way that lets it find its source
directory.  If whatever runs FOO puts the full path into argv[0], you
can use that.  Else, require a suitable argument or environment
variable.

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

[...]
> The problem is including generated bits, namely results of configure,
> into source files.
>
> The Autoconf way is to substitute placeholders in FOO.in producing FOO.
>
> When you want to limit .in contents as much as possible, you factor out
> the stuff that needs substitutions into some SUB.in, which you then
> include into FOO.  Requires a suitable include mechanism.  In shell,
> builtin source.
>
> But then you need to find SUB from FOO.  I've seen two ways used:
>
> 1. Assume SUB is in the current directory.  Link FOO into the build tree
> to make it so.
>
> 2. Require FOO to be run in a way that lets it find its source
> directory.  If whatever runs FOO puts the full path into argv[0], you
> can use that.  Else, require a suitable argument or environment
> variable.

Uh, I left out some "obvious" details here.  Revert the role of FOO and
SUB.  Generate FOO from FOO.in into the build tree, include the real
meat from SUB.

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Max Reitz]

On 15.05.2014 08:52, Markus Armbruster wrote:
> Max Reitz <mreitz@redhat.com> writes:
>
>> On 14.05.2014 14:33, Markus Armbruster wrote:
>>> Max Reitz <mreitz@redhat.com> writes:
>>>
>>>> Currently, QEMU's iotests rely on /usr/bin/env to start the correct
>>>> Python (that is, at least Python 2.4, but not 3). On systems where
>>>> Python 3 is the default, the user has no clean way of making the iotests
>>>> use the correct binary.
>>>>
>>>> This commit makes the iotests use the Python selected by configure.
>>>>
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> I'm afraid this broke qemu-iotests in a separate build tree:
>>>
>>>       ./check: line 38: ./common.env: No such file or directory
>> As I already replied to Peter, I see two (or maybe three) ways to fix this:
>>
>> The first is, we use the correct path to common.env. This would
>> however result in modification of the source tree although this is
>> probably not what the user intends with an out-of-tree build. On the
>> other hand, this would just work.
> Writing to the source tree works only when the write is exactly the same
> for every build tree.  Example: autoconf writing configure.
>
> Modifying files under git-control is right out.
>
>> The second is, we do not create common.env for out-of-tree builds and
>> add a default common.env to the repository ("PYTHON = python" should
>> probably suffice). This would not introduce any regressions, however,
>> the iotests would remain problematic on systems with Python 3 being
>> the default when using out-of-tree builds.
> A difference between an in-tree and an out-of-tree build is a bug.
>
> If plain "python" is good enough for out-of-tree builds, it should be
> good enough for in-tree builds.
>
>>                                             As I guess that out-of-tree
>> builds are actually recommended,
> Correct.
>
>>                                   this would mean that the better
>> solution might be to revert my patch and instead change every "python"
>> occurrence in the iotests' Shebangs to "python2", as kind of a third
>> way to go. However, for this I'm not sure whether all systems which
>> are supposed to be supported by qemu actually have a "python2"
>> executable/symlink. I guess so, but then again...
> I don't know.  Try and find out :)

Okay, so there is a Python Enhancement Proposal, which suggests having 
the symlink python2:

http://legacy.python.org/dev/peps/pep-0394/

However, at least Debian seems to ignore this (or at least some Debian 
versions do).

I think I'll go with Fam's proposal, which is making common.config look 
for python itself, which then can be overwritten by an environment variable.

>> So, either we fix it but try to write to the source tree without the
>> user intending to modify it; or we fix it for in-tree builds only; or
>> we drop the magic and just use "python2" in the iotests' Shebangs.
> The problem is including generated bits, namely results of configure,
> into source files.
>
> The Autoconf way is to substitute placeholders in FOO.in producing FOO.
>
> When you want to limit .in contents as much as possible, you factor out
> the stuff that needs substitutions into some SUB.in, which you then
> include into FOO.  Requires a suitable include mechanism.  In shell,
> builtin source.
>
> But then you need to find SUB from FOO.  I've seen two ways used:
>
> 1. Assume SUB is in the current directory.  Link FOO into the build tree
> to make it so.
>
> 2. Require FOO to be run in a way that lets it find its source
> directory.  If whatever runs FOO puts the full path into argv[0], you
> can use that.  Else, require a suitable argument or environment
> variable.

Hm, doing this would probably be even more magic than my previous patch 
– which already had too much magic for myself to handle and therefore 
broke. I hope finding the correct python to use in 
tests/qemu-iotests/common.config will work out without too much hassle.

Max

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Peter Maydell]

On 15 May 2014 17:56, Max Reitz <mreitz@redhat.com> wrote:
> I think I'll go with Fam's proposal, which is making common.config look for
> python itself, which then can be overwritten by an environment variable.

That sounds wrong to me. We already have a way for the user
to tell us what python to use, which is the configure
script argument. We should just arrange to use that.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Max Reitz]

On 15.05.2014 19:08, Peter Maydell wrote:
> On 15 May 2014 17:56, Max Reitz <mreitz@redhat.com> wrote:
>> I think I'll go with Fam's proposal, which is making common.config look for
>> python itself, which then can be overwritten by an environment variable.
> That sounds wrong to me. We already have a way for the user
> to tell us what python to use, which is the configure
> script argument. We should just arrange to use that.

configure (and even make) for me does (do) not create any new file in 
the source tree. If we want to leave it at that, we will need to create 
a configuration file somewhere in the build tree which points to the 
correct version of Python and then tell the iotests where to find that 
file (as the iotests are supposed to be run in the source tree, as far 
as I know). But if we were to do that, the user could easily just 
directly tell it what Python to use.

In fact, the iotests currently have exactly that problem, but not with a 
system program, but with the generated executables. In order to use the 
iotests, one has to create symlinks pointing to the compiled qemu, 
qemu-io, qemu-img and qemu-nbd. This may be because this allows the user 
to freely specify which qemu to use, but I'd much rather guess it is 
exactly because the iotests do not know where the build tree is. 
Otherwise, it would probably default to "$BUILD_PATH/qemu-img" etc. 
instead of just "qemu-img" (i.e. the one in $PATH) if the symlink does 
not exist.

Therefore, I think the iotests have to be independent of configure, as 
they don't appear to have access to configure's results (that is, the 
location of the build tree).

So the user always has some hassle with the iotests, as those symlinks 
have to be created before they may be used. I think this is 
justification enough that we can make the search process for the correct 
Python independent of configure, as long as it is overridable by the 
user (which it would be, through an environment variable).

And finally, if I'll be doing things correctly, it will at least not be 
worse than it currently is. If the user does not specify which Python to 
use for the iotests, common.config will try python2, and if that does 
not exist, python.

I know it would be nice to use the Python which has been selected 
through configure (that is why I wrote the patch like it was in the 
first place), but in order to get the result to iotests, we either have 
to modify the original source tree (where the iotests are run from) or 
tell iotests manually where to find the build tree containing the result.

Max

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Peter Maydell]

On 15 May 2014 18:29, Max Reitz <mreitz@redhat.com> wrote:
> On 15.05.2014 19:08, Peter Maydell wrote:
>>
>> On 15 May 2014 17:56, Max Reitz <mreitz@redhat.com> wrote:
>>>
>>> I think I'll go with Fam's proposal, which is making common.config look
>>> for
>>> python itself, which then can be overwritten by an environment variable.
>>
>> That sounds wrong to me. We already have a way for the user
>> to tell us what python to use, which is the configure
>> script argument. We should just arrange to use that.
>
>
> configure (and even make) for me does (do) not create any new
> file in the source tree.

Yes, and this is correct behaviour. Files created during
build should be created in the build tree. (Consider for
instance a situation where the source tree is being used
to build two configurations simultaneously which have different
--python arguments.)

> If we want to leave it at that, we will need to create a
> configuration file somewhere in the build tree which points to the correct
> version of Python and then tell the iotests where to find that file

> (as the iotests are supposed to be run in the source tree, as far
> as I know).

This is the bit I think should be changed. I was surprised to
find that the iotests were not run with their current working
directory set to the build directory, in fact.

> In fact, the iotests currently have exactly that problem, but not with a
> system program, but with the generated executables. In order to use the
> iotests, one has to create symlinks pointing to the compiled qemu, qemu-io,
> qemu-img and qemu-nbd. This may be because this allows the user to freely
> specify which qemu to use, but I'd much rather guess it is exactly because
> the iotests do not know where the build tree is. Otherwise, it would
> probably default to "$BUILD_PATH/qemu-img" etc. instead of just "qemu-img"
> (i.e. the one in $PATH) if the symlink does not exist.

This also sounds like something that would be simply fixed by
making the current working directory be in the build tree,
not the source tree.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Markus Armbruster]

Max Reitz <mreitz@redhat.com> writes:

> On 15.05.2014 08:52, Markus Armbruster wrote:
>> Max Reitz <mreitz@redhat.com> writes:
>>
>>> On 14.05.2014 14:33, Markus Armbruster wrote:
>>>> Max Reitz <mreitz@redhat.com> writes:
>>>>
>>>>> Currently, QEMU's iotests rely on /usr/bin/env to start the correct
>>>>> Python (that is, at least Python 2.4, but not 3). On systems where
>>>>> Python 3 is the default, the user has no clean way of making the iotests
>>>>> use the correct binary.
>>>>>
>>>>> This commit makes the iotests use the Python selected by configure.
>>>>>
>>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>> I'm afraid this broke qemu-iotests in a separate build tree:
>>>>
>>>>       ./check: line 38: ./common.env: No such file or directory
>>> As I already replied to Peter, I see two (or maybe three) ways to fix this:
>>>
>>> The first is, we use the correct path to common.env. This would
>>> however result in modification of the source tree although this is
>>> probably not what the user intends with an out-of-tree build. On the
>>> other hand, this would just work.
>> Writing to the source tree works only when the write is exactly the same
>> for every build tree.  Example: autoconf writing configure.
>>
>> Modifying files under git-control is right out.
>>
>>> The second is, we do not create common.env for out-of-tree builds and
>>> add a default common.env to the repository ("PYTHON = python" should
>>> probably suffice). This would not introduce any regressions, however,
>>> the iotests would remain problematic on systems with Python 3 being
>>> the default when using out-of-tree builds.
>> A difference between an in-tree and an out-of-tree build is a bug.
>>
>> If plain "python" is good enough for out-of-tree builds, it should be
>> good enough for in-tree builds.
>>
>>>                                             As I guess that out-of-tree
>>> builds are actually recommended,
>> Correct.
>>
>>>                                   this would mean that the better
>>> solution might be to revert my patch and instead change every "python"
>>> occurrence in the iotests' Shebangs to "python2", as kind of a third
>>> way to go. However, for this I'm not sure whether all systems which
>>> are supposed to be supported by qemu actually have a "python2"
>>> executable/symlink. I guess so, but then again...
>> I don't know.  Try and find out :)
>
> Okay, so there is a Python Enhancement Proposal, which suggests having
> the symlink python2:
>
> http://legacy.python.org/dev/peps/pep-0394/
>
> However, at least Debian seems to ignore this (or at least some Debian
> versions do).

Fools :)

> I think I'll go with Fam's proposal, which is making common.config
> look for python itself, which then can be overwritten by an
> environment variable.

Namely "tell ./check the path to common.env with an env var, like how we
tell ./check the path to qemu-img with QEMU_IMG_PROG".  I don't like how
we tell check where to find the stuff we build.  But you can declare
that a separate problem, and fix your "where is common.env" problem with
the current method, as Fam suggests.

>>> So, either we fix it but try to write to the source tree without the
>>> user intending to modify it; or we fix it for in-tree builds only; or
>>> we drop the magic and just use "python2" in the iotests' Shebangs.
>> The problem is including generated bits, namely results of configure,
>> into source files.
>>
>> The Autoconf way is to substitute placeholders in FOO.in producing FOO.
>>
>> When you want to limit .in contents as much as possible, you factor out
>> the stuff that needs substitutions into some SUB.in, which you then
>> include into FOO.  Requires a suitable include mechanism.  In shell,
>> builtin source.
>>
>> But then you need to find SUB from FOO.  I've seen two ways used:
>>
>> 1. Assume SUB is in the current directory.  Link FOO into the build tree
>> to make it so.
>>
>> 2. Require FOO to be run in a way that lets it find its source
>> directory.  If whatever runs FOO puts the full path into argv[0], you
>> can use that.  Else, require a suitable argument or environment
>> variable.
>
> Hm, doing this would probably be even more magic than my previous
> patch – which already had too much magic for myself to handle and
> therefore broke. I hope finding the correct python to use in
> tests/qemu-iotests/common.config will work out without too much
> hassle.

It's low-level magic at most :)

The root stupid idea is to run stuff in the source tree.  Since a source
tree can have many build trees, finding the correct build tree can't be
automated.

If you run stuff in the build tree, there is exactly one source tree,
and putting a pointer to it in the build tree is trivial.  In fact, we
already have one: try "make -pn | grep ^SRC_PATH".

I'd generate a suitable script into the build tree that sets up
necessary variables, then sources $SRC_PATH/qemu-iotests/check.

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Max Reitz]

On 15.05.2014 19:35, Markus Armbruster wrote:
> Max Reitz <mreitz@redhat.com> writes:
>
>> On 15.05.2014 08:52, Markus Armbruster wrote:
>>> Max Reitz <mreitz@redhat.com> writes:
>>>
>>>> On 14.05.2014 14:33, Markus Armbruster wrote:
>>>>> Max Reitz <mreitz@redhat.com> writes:
>>>>>
>>>>>> Currently, QEMU's iotests rely on /usr/bin/env to start the correct
>>>>>> Python (that is, at least Python 2.4, but not 3). On systems where
>>>>>> Python 3 is the default, the user has no clean way of making the iotests
>>>>>> use the correct binary.
>>>>>>
>>>>>> This commit makes the iotests use the Python selected by configure.
>>>>>>
>>>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>>> I'm afraid this broke qemu-iotests in a separate build tree:
>>>>>
>>>>>        ./check: line 38: ./common.env: No such file or directory
>>>> As I already replied to Peter, I see two (or maybe three) ways to fix this:
>>>>
>>>> The first is, we use the correct path to common.env. This would
>>>> however result in modification of the source tree although this is
>>>> probably not what the user intends with an out-of-tree build. On the
>>>> other hand, this would just work.
>>> Writing to the source tree works only when the write is exactly the same
>>> for every build tree.  Example: autoconf writing configure.
>>>
>>> Modifying files under git-control is right out.
>>>
>>>> The second is, we do not create common.env for out-of-tree builds and
>>>> add a default common.env to the repository ("PYTHON = python" should
>>>> probably suffice). This would not introduce any regressions, however,
>>>> the iotests would remain problematic on systems with Python 3 being
>>>> the default when using out-of-tree builds.
>>> A difference between an in-tree and an out-of-tree build is a bug.
>>>
>>> If plain "python" is good enough for out-of-tree builds, it should be
>>> good enough for in-tree builds.
>>>
>>>>                                              As I guess that out-of-tree
>>>> builds are actually recommended,
>>> Correct.
>>>
>>>>                                    this would mean that the better
>>>> solution might be to revert my patch and instead change every "python"
>>>> occurrence in the iotests' Shebangs to "python2", as kind of a third
>>>> way to go. However, for this I'm not sure whether all systems which
>>>> are supposed to be supported by qemu actually have a "python2"
>>>> executable/symlink. I guess so, but then again...
>>> I don't know.  Try and find out :)
>> Okay, so there is a Python Enhancement Proposal, which suggests having
>> the symlink python2:
>>
>> http://legacy.python.org/dev/peps/pep-0394/
>>
>> However, at least Debian seems to ignore this (or at least some Debian
>> versions do).
> Fools :)
>
>> I think I'll go with Fam's proposal, which is making common.config
>> look for python itself, which then can be overwritten by an
>> environment variable.
> Namely "tell ./check the path to common.env with an env var, like how we
> tell ./check the path to qemu-img with QEMU_IMG_PROG".  I don't like how
> we tell check where to find the stuff we build.  But you can declare
> that a separate problem, and fix your "where is common.env" problem with
> the current method, as Fam suggests.
>
>>>> So, either we fix it but try to write to the source tree without the
>>>> user intending to modify it; or we fix it for in-tree builds only; or
>>>> we drop the magic and just use "python2" in the iotests' Shebangs.
>>> The problem is including generated bits, namely results of configure,
>>> into source files.
>>>
>>> The Autoconf way is to substitute placeholders in FOO.in producing FOO.
>>>
>>> When you want to limit .in contents as much as possible, you factor out
>>> the stuff that needs substitutions into some SUB.in, which you then
>>> include into FOO.  Requires a suitable include mechanism.  In shell,
>>> builtin source.
>>>
>>> But then you need to find SUB from FOO.  I've seen two ways used:
>>>
>>> 1. Assume SUB is in the current directory.  Link FOO into the build tree
>>> to make it so.
>>>
>>> 2. Require FOO to be run in a way that lets it find its source
>>> directory.  If whatever runs FOO puts the full path into argv[0], you
>>> can use that.  Else, require a suitable argument or environment
>>> variable.
>> Hm, doing this would probably be even more magic than my previous
>> patch – which already had too much magic for myself to handle and
>> therefore broke. I hope finding the correct python to use in
>> tests/qemu-iotests/common.config will work out without too much
>> hassle.
> It's low-level magic at most :)
>
> The root stupid idea is to run stuff in the source tree.  Since a source
> tree can have many build trees, finding the correct build tree can't be
> automated.
>
> If you run stuff in the build tree, there is exactly one source tree,
> and putting a pointer to it in the build tree is trivial.  In fact, we
> already have one: try "make -pn | grep ^SRC_PATH".
>
> I'd generate a suitable script into the build tree that sets up
> necessary variables, then sources $SRC_PATH/qemu-iotests/check.

And here I was, just not wanting to do "rm /usr/bin/python; ln -s 
python2 /usr/bin/python" anymore...

Okay, as this coincides mostly with what Peter says, I'll try to allow 
the iotests to be executed from the build tree (while hopefully keeping 
compatibility with the current situation).

Max

Re: [Qemu-devel] [PATCH] iotests: Use configured python

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On 05/15/2014 11:35 AM, Markus Armbruster wrote:

> 
> The root stupid idea is to run stuff in the source tree.  Since a source
> tree can have many build trees, finding the correct build tree can't be
> automated.
> 
> If you run stuff in the build tree, there is exactly one source tree,
> and putting a pointer to it in the build tree is trivial.  In fact, we
> already have one: try "make -pn | grep ^SRC_PATH".
> 
> I'd generate a suitable script into the build tree that sets up
> necessary variables, then sources $SRC_PATH/qemu-iotests/check.

In fact, this is sort of what libvirt does.  We have a 'run.in' script
template at the top directory, then create 'run' in the build tree
(whether or not it is a VPATH tree); the run script then primes any
necessary environment variables to execute commands using the build tree
as its preferred location for executables, dynamic libraries, and helper
files.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]