[Qemu-devel] [PATCH v3 0/2] iotests: Add test for colon handling

[Qemu-devel] [PATCH v3 0/2] iotests: Add test for colon handling

[Max Reitz]

This is a v3 for "block: Fix backing paths for filenames with colons".
Kevin reported that the test added there does not work if the test
programs are specified with relative paths (because the new test changes
its working directory), so we/I dropped the test from the queue and here
it is again.

The test itself is unchanged (except for the comment fixed as requested
by Eric), there is just a new patch here to make it work even if you
specify the test programs with relative paths. Bonus: It makes symlinked
programs work with out-of-tree builds.


git-backport-diff against v2:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/2:[down] 'iotests: Use absolute paths for executables'
002/2:[0006] [FC] 'iotests: Add test for colon handling'


Max Reitz (2):
  iotests: Use absolute paths for executables
  iotests: Add test for colon handling

 tests/qemu-iotests/126           | 105 +++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/126.out       |  23 +++++++++
 tests/qemu-iotests/common.config |   6 +++
 tests/qemu-iotests/group         |   1 +
 4 files changed, 135 insertions(+)
 create mode 100755 tests/qemu-iotests/126
 create mode 100644 tests/qemu-iotests/126.out

-- 
2.9.4

[Qemu-devel] [PATCH v3 1/2] iotests: Use absolute paths for executables

[Max Reitz]

A user may specify a relative path for accessing qemu, qemu-img, etc.
through environment variables ($QEMU_PROG and friends) or a symlink.

If a test decides to change its working directory, relative paths will
cease to work, however. Work around this by making all of the paths to
programs that should undergo testing absolute. Besides "realpath", we
also have to use "which" to support programs in $PATH.

As a side effect, this fixes specifying these programs as symlinks for
out-of-tree builds: Before, you would have to create two symlinks, one
in the build and one in the source tree (the first one for common.config
to find, the second one for the iotest to use). Now it is sufficient to
create one in the build tree because common.config will resolve it.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/common.config | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index d1b45f5..08aac56 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
     export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
 fi
 
+export QEMU_PROG=$(realpath "$(which "$QEMU_PROG")")
+export QEMU_IMG_PROG=$(realpath "$(which "$QEMU_IMG_PROG")")
+export QEMU_IO_PROG=$(realpath "$(which "$QEMU_IO_PROG")")
+export QEMU_NBD_PROG=$(realpath "$(which "$QEMU_NBD_PROG")")
+export QEMU_VXHS_PROG=$(realpath "$(which "$QEMU_VXHS_PROG")")
+
 _qemu_wrapper()
 {
     (
-- 
2.9.4

Re: [Qemu-devel] [PATCH v3 1/2] iotests: Use absolute paths for executables

[Eric Blake]

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

On 05/29/2017 10:23 AM, Max Reitz wrote:
> A user may specify a relative path for accessing qemu, qemu-img, etc.
> through environment variables ($QEMU_PROG and friends) or a symlink.
> 
> If a test decides to change its working directory, relative paths will
> cease to work, however. Work around this by making all of the paths to
> programs that should undergo testing absolute. Besides "realpath", we
> also have to use "which" to support programs in $PATH.

'type -p' is more portable than 'which' - especially since our scripts
are bash scripts, and type is a bash builtin while which is not.

> 
> As a side effect, this fixes specifying these programs as symlinks for
> out-of-tree builds: Before, you would have to create two symlinks, one
> in the build and one in the source tree (the first one for common.config
> to find, the second one for the iotest to use). Now it is sufficient to
> create one in the build tree because common.config will resolve it.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/common.config | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
> index d1b45f5..08aac56 100644
> --- a/tests/qemu-iotests/common.config
> +++ b/tests/qemu-iotests/common.config
> @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
>      export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
>  fi
>  
> +export QEMU_PROG=$(realpath "$(which "$QEMU_PROG")")
> +export QEMU_IMG_PROG=$(realpath "$(which "$QEMU_IMG_PROG")")
> +export QEMU_IO_PROG=$(realpath "$(which "$QEMU_IO_PROG")")
> +export QEMU_NBD_PROG=$(realpath "$(which "$QEMU_NBD_PROG")")
> +export QEMU_VXHS_PROG=$(realpath "$(which "$QEMU_VXHS_PROG")")

If you switch all of these to $(realpath -- "$(type -p "$QEMU_...")"),
you can add:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v3 1/2] iotests: Use absolute paths for executables

[Max Reitz]

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

On 2017-05-29 17:42, Eric Blake wrote:
> On 05/29/2017 10:23 AM, Max Reitz wrote:
>> A user may specify a relative path for accessing qemu, qemu-img, etc.
>> through environment variables ($QEMU_PROG and friends) or a symlink.
>>
>> If a test decides to change its working directory, relative paths will
>> cease to work, however. Work around this by making all of the paths to
>> programs that should undergo testing absolute. Besides "realpath", we
>> also have to use "which" to support programs in $PATH.
> 
> 'type -p' is more portable than 'which' - especially since our scripts
> are bash scripts, and type is a bash builtin while which is not.
> 
>>
>> As a side effect, this fixes specifying these programs as symlinks for
>> out-of-tree builds: Before, you would have to create two symlinks, one
>> in the build and one in the source tree (the first one for common.config
>> to find, the second one for the iotest to use). Now it is sufficient to
>> create one in the build tree because common.config will resolve it.
>>
>> Reported-by: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/common.config | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
>> index d1b45f5..08aac56 100644
>> --- a/tests/qemu-iotests/common.config
>> +++ b/tests/qemu-iotests/common.config
>> @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
>>      export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
>>  fi
>>  
>> +export QEMU_PROG=$(realpath "$(which "$QEMU_PROG")")
>> +export QEMU_IMG_PROG=$(realpath "$(which "$QEMU_IMG_PROG")")
>> +export QEMU_IO_PROG=$(realpath "$(which "$QEMU_IO_PROG")")
>> +export QEMU_NBD_PROG=$(realpath "$(which "$QEMU_NBD_PROG")")
>> +export QEMU_VXHS_PROG=$(realpath "$(which "$QEMU_VXHS_PROG")")
> 
> If you switch all of these to $(realpath -- "$(type -p "$QEMU_...")"),
> you can add:

I'd love to, but this is what type -p outputs for me:

$ type -p qemu-img
qemu-img is /usr/bin/qemu-img

So I would need to parse the result (and it depends on the locale). If
that is indeed so, I'd rather stay with which, to be honest...

Max


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

Re: [Qemu-devel] [PATCH v3 1/2] iotests: Use absolute paths for executables

[Eric Blake]

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

On 05/29/2017 10:46 AM, Max Reitz wrote:

>> If you switch all of these to $(realpath -- "$(type -p "$QEMU_...")"),
>> you can add:
> 
> I'd love to, but this is what type -p outputs for me:
> 
> $ type -p qemu-img
> qemu-img is /usr/bin/qemu-img

Huh? That's plain 'type' output.  Are you sure you're testing 'type -p'?

$ PATH=$PATH    # to forcefully clear bash's cache
$ type qemu-img
qemu-img is /usr/bin/qemu-img
$ type -p qemu-img
/usr/bin/qemu-img
$ qemu-img --help >/dev/null   # to repopulate qemu-img into the cache
$ type qemu-img
qemu-img is hashed (/usr/bin/qemu-img)
$ type -p qemu-img
/usr/bin/qemu-img

> 
> So I would need to parse the result (and it depends on the locale). If
> that is indeed so, I'd rather stay with which, to be honest...

Plain 'type' does have to be parsed, but 'type -p' is required to be
machine-usable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v3 1/2] iotests: Use absolute paths for executables

[Max Reitz]

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

On 2017-05-29 17:55, Eric Blake wrote:
> On 05/29/2017 10:46 AM, Max Reitz wrote:
> 
>>> If you switch all of these to $(realpath -- "$(type -p "$QEMU_...")"),
>>> you can add:
>>
>> I'd love to, but this is what type -p outputs for me:
>>
>> $ type -p qemu-img
>> qemu-img is /usr/bin/qemu-img
> 
> Huh? That's plain 'type' output.  Are you sure you're testing 'type -p'?
> 
> $ PATH=$PATH    # to forcefully clear bash's cache
> $ type qemu-img
> qemu-img is /usr/bin/qemu-img
> $ type -p qemu-img
> /usr/bin/qemu-img
> $ qemu-img --help >/dev/null   # to repopulate qemu-img into the cache
> $ type qemu-img
> qemu-img is hashed (/usr/bin/qemu-img)
> $ type -p qemu-img
> /usr/bin/qemu-img
> 
>>
>> So I would need to parse the result (and it depends on the locale). If
>> that is indeed so, I'd rather stay with which, to be honest...
> 
> Plain 'type' does have to be parsed, but 'type -p' is required to be
> machine-usable.

Oops. I tested it (both with -p and without) on zsh, then on bash, and I
forgot the -p on bash. Well, I'm going to trust you, then. O:-)

Max


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

[Qemu-devel] [PATCH v3 2/2] iotests: Add test for colon handling

[Max Reitz]

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/126     | 105 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/126.out |  23 ++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 129 insertions(+)
 create mode 100755 tests/qemu-iotests/126
 create mode 100644 tests/qemu-iotests/126.out

diff --git a/tests/qemu-iotests/126 b/tests/qemu-iotests/126
new file mode 100755
index 0000000..a2d4d6c
--- /dev/null
+++ b/tests/qemu-iotests/126
@@ -0,0 +1,105 @@
+#!/bin/bash
+#
+# Tests handling of colons in filenames (which may be confused with protocol
+# prefixes)
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1	# failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# Needs backing file support
+_supported_fmt qcow qcow2 qed vmdk
+# This is the default protocol (and we want to test the difference between
+# colons which separate a protocol prefix from the rest and colons which are
+# just part of the filename, so we cannot test protocols which require a prefix)
+_supported_proto file
+_supported_os Linux
+
+echo
+echo '=== Testing plain files ==='
+echo
+
+# A colon after a slash is not a protocol prefix separator
+TEST_IMG="$TEST_DIR/a:b.$IMGFMT" _make_test_img 64M
+_rm_test_img "$TEST_DIR/a:b.$IMGFMT"
+
+# But if you want to be really sure, you can do this
+TEST_IMG="file:$TEST_DIR/a:b.$IMGFMT" _make_test_img 64M
+_rm_test_img "$TEST_DIR/a:b.$IMGFMT"
+
+
+echo
+echo '=== Testing relative backing filename resolution ==='
+echo
+
+BASE_IMG="$TEST_DIR/image:base.$IMGFMT"
+TOP_IMG="$TEST_DIR/image:top.$IMGFMT"
+
+TEST_IMG=$BASE_IMG _make_test_img 64M
+TEST_IMG=$TOP_IMG _make_test_img -b ./image:base.$IMGFMT
+
+# The default cluster size depends on the image format
+TEST_IMG=$TOP_IMG _img_info | grep -v 'cluster_size'
+
+_rm_test_img "$BASE_IMG"
+_rm_test_img "$TOP_IMG"
+
+
+# Do another test where we access both top and base without any slash in them
+echo
+pushd "$TEST_DIR" >/dev/null
+
+BASE_IMG="base.$IMGFMT"
+TOP_IMG="file:image:top.$IMGFMT"
+
+TEST_IMG=$BASE_IMG _make_test_img 64M
+TEST_IMG=$TOP_IMG _make_test_img -b "$BASE_IMG"
+
+TEST_IMG=$TOP_IMG _img_info | grep -v 'cluster_size'
+
+_rm_test_img "$BASE_IMG"
+_rm_test_img "image:top.$IMGFMT"
+
+popd >/dev/null
+
+# Note that we could also do the same test with BASE_IMG=file:image:base.$IMGFMT
+# -- but behavior for that case is a bit strange. Protocol-prefixed paths are
+# in a sense always absolute paths, so such paths will never be combined with
+# the path of the overlay. But since "image:base.$IMGFMT" is actually a
+# relative path, it will always be evaluated relative to qemu's CWD (but not
+# relative to the overlay!). While this is more or less intended, it is still
+# pretty strange and thus not something that is tested here.
+# (The root of the issue is the use of a relative path with a protocol prefix.
+#  This may always give you weird results because in one sense, qemu considers
+#  such paths absolute, whereas in another, they are still relative.)
+
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/126.out b/tests/qemu-iotests/126.out
new file mode 100644
index 0000000..50d7308
--- /dev/null
+++ b/tests/qemu-iotests/126.out
@@ -0,0 +1,23 @@
+QA output created by 126
+
+=== Testing plain files ===
+
+Formatting 'TEST_DIR/a:b.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/a:b.IMGFMT', fmt=IMGFMT size=67108864
+
+=== Testing relative backing filename resolution ===
+
+Formatting 'TEST_DIR/image:base.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/image:top.IMGFMT', fmt=IMGFMT size=67108864 backing_file=./image:base.IMGFMT
+image: TEST_DIR/image:top.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+backing file: ./image:base.IMGFMT (actual path: TEST_DIR/./image:base.IMGFMT)
+
+Formatting 'base.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'file:image:top.IMGFMT', fmt=IMGFMT size=67108864 backing_file=base.IMGFMT
+image: ./image:top.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+backing file: base.IMGFMT (actual path: ./base.IMGFMT)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 5c8ea0f..30717cb 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -130,6 +130,7 @@
 122 rw auto
 123 rw auto quick
 124 rw auto backing
+126 rw auto backing
 128 rw auto quick
 129 rw auto quick
 130 rw auto quick
-- 
2.9.4