diff mbox

[v2,3/3] iotests: Add test for colon handling

Message ID 20170522195217.12991-4-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Max Reitz May 22, 2017, 7:52 p.m. UTC 
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

Comments

Eric Blake May 22, 2017, 8:06 p.m. UTC | #1 
On 05/22/2017 02:52 PM, Max Reitz wrote:
> 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
> 

> +# 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 to use a relative path with a protocol prefix. This

s/to/the/

> +#  may always give you weird results because in one sense, qemu considers such
> +#  paths absolute, whereas in another, they are still relative.)

Should we tighten qemu to forbid the use of a protocol prefix with a
non-absolute path?  But that can be a subsequent patch, I don't see it
as a reason to hold up this one.

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz May 22, 2017, 8:17 p.m. UTC | #2 
On 2017-05-22 22:06, Eric Blake wrote:
> On 05/22/2017 02:52 PM, Max Reitz wrote:
>> 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
>>
> 
>> +# 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 to use a relative path with a protocol prefix. This
> 
> s/to/the/

Then it will also have to be s/use a/use of a/.

>> +#  may always give you weird results because in one sense, qemu considers such
>> +#  paths absolute, whereas in another, they are still relative.)
> 
> Should we tighten qemu to forbid the use of a protocol prefix with a
> non-absolute path?  But that can be a subsequent patch, I don't see it
> as a reason to hold up this one.

Hm. I'd rather not do this. It could be considered a bug fix (and it
would make patch 2 obsolete, so it would definitely have a use there),
but it would break compatibility.

The whole filename handling in qemu is a mess, and that's mostly because
users expect it to be a mess. It would be great if we didn't have to
handle filenames at all, or at least only absolute filenames, but that
would not be something that users like. And having little weird things
like these in some corner cases (it's not like many people are using an
explicit "file:" prefix anyway) is better than not supporting relative
filenames at all...

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

Thanks!

Max
diff mbox

Patch

diff --git a/tests/qemu-iotests/126 b/tests/qemu-iotests/126
new file mode 100755
index 0000000..3a2a43a
--- /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 to use 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