* [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons @ 2017-05-22 19:52 Max Reitz 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 1/3] " Max Reitz ` (3 more replies) 0 siblings, 4 replies; 8+ messages in thread From: Max Reitz @ 2017-05-22 19:52 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake There are (at least) two issues with filenames that contain colons when trying to use relative backing filenames with them, for each of which there is a patch in this series. The first patch fixes an issue in the general block layer (path_combine() does not have the same opinion on what constitutes a protocol prefix as path_has_protocl()), the second fixes an issue in file-* (when stripping off the optional "file:" prefix we should not create a filename that seems to have another valid protocol prefix). The third patch adds a test. Bonus info: The following is something I did not change, although it is weird: $ mkdir foo && cd foo $ ../qemu-img create -f qcow2 file:image:base.qcow2 64M Formatting 'file:image:base.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ ../qemu-img create -f qcow2 -b file:image:base.qcow2 file:image:top.qcow2 Formatting 'file:image:top.qcow2', fmt=qcow2 size=67108864 backing_file=file:image:base.qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ cd .. $ ./qemu-img info foo/image:top.qcow2 image: foo/image:top.qcow2 file format: qcow2 virtual size: 64M (67108864 bytes) disk size: 196K cluster_size: 65536 backing file: file:image:base.qcow2 Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false $ ./qemu-io foo/image:top.qcow2 can't open device foo/image:top.qcow2: Could not open backing file: Could not open './image:base.qcow2': No such file or directory Patch 3 adds a comment to the test which explains this behavior in a bit more detail, but the gist is this: qemu generally thinks that protocol-prefixed filenames are absolute filenames; but from a file-posix perspective, "file:foo" is still a relative filename. I think the above behavior is what we want and that it's the best we can do, but it still is weird, so I just wanted to mention it. v2: - Kept patch 1 unchanged - Added patch 2 - Extended patch 3 (did NOT change the patch title) 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/3:[----] [--] 'block: Fix backing paths for filenames with colons' 002/3:[down] 'block/file-*: *_parse_filename() and colons' 003/3:[0036] [FC] 'iotests: Add test for colon handling' Max Reitz (3): block: Fix backing paths for filenames with colons block/file-*: *_parse_filename() and colons iotests: Add test for colon handling include/block/block_int.h | 3 ++ block.c | 50 ++++++++++++++++++--- block/file-posix.c | 17 ++------ block/file-win32.c | 12 +----- tests/qemu-iotests/126 | 105 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/126.out | 23 ++++++++++ tests/qemu-iotests/group | 1 + 7 files changed, 182 insertions(+), 29 deletions(-) create mode 100755 tests/qemu-iotests/126 create mode 100644 tests/qemu-iotests/126.out -- 2.9.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] block: Fix backing paths for filenames with colons 2017-05-22 19:52 [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons Max Reitz @ 2017-05-22 19:52 ` Max Reitz 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 2/3] block/file-*: *_parse_filename() and colons Max Reitz ` (2 subsequent siblings) 3 siblings, 0 replies; 8+ messages in thread From: Max Reitz @ 2017-05-22 19:52 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake path_combine() naturally tries to preserve a protocol prefix. However, it recognizes such a prefix by scanning for the first colon; which is different from what path_has_protocol() does: There only is a protocol prefix if there is a colon before the first slash. A protocol prefix that is not recognized by path_has_protocol() is none, and should thus not be taken as one. Case in point, before this patch: $ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2 qemu-img: ./top:image.qcow2: Could not open './top:backing.qcow2': No such file or directory Afterwards: $ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2 qemu-img: ./top:image.qcow2: Could not open './backing.qcow2': No such file or directory Reported-by: yangyang <yangyang@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- block.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 50ba264..b72b872 100644 --- a/block.c +++ b/block.c @@ -163,11 +163,16 @@ void path_combine(char *dest, int dest_size, if (path_is_absolute(filename)) { pstrcpy(dest, dest_size, filename); } else { - p = strchr(base_path, ':'); - if (p) - p++; - else - p = base_path; + const char *protocol_stripped = NULL; + + if (path_has_protocol(base_path)) { + protocol_stripped = strchr(base_path, ':'); + if (protocol_stripped) { + protocol_stripped++; + } + } + p = protocol_stripped ?: base_path; + p1 = strrchr(base_path, '/'); #ifdef _WIN32 { -- 2.9.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] block/file-*: *_parse_filename() and colons 2017-05-22 19:52 [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons Max Reitz 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 1/3] " Max Reitz @ 2017-05-22 19:52 ` Max Reitz 2017-05-22 20:02 ` Eric Blake 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 3/3] iotests: Add test for colon handling Max Reitz 2017-05-26 17:30 ` [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons Max Reitz 3 siblings, 1 reply; 8+ messages in thread From: Max Reitz @ 2017-05-22 19:52 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake The file drivers' *_parse_filename() implementations just strip the optional protocol prefix off the filename. However, for e.g. "file:foo:bar", this would lead to "foo:bar" being stored as the BDS's filename which looks like it should be managed using the "foo" protocol. This is especially troublesome if you then try to resolve a backing filename based on "foo:bar". This issue can only occur if the stripped part is a relative filename ("file:/foo:bar" will be shortened to "/foo:bar" and having a slash before the first colon means that "/foo" is not recognized as a protocol part). Therefore, we can easily fix it by prepending "./" to such filenames. Before this patch: $ ./qemu-img create -f qcow2 backing.qcow2 64M Formatting 'backing.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ ./qemu-img create -f qcow2 -b backing.qcow2 file:top:image.qcow2 Formatting 'file:top:image.qcow2', fmt=qcow2 size=67108864 backing_file=backing.qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ ./qemu-io file:top:image.qcow2 can't open device file:top:image.qcow2: Could not open backing file: Unknown protocol 'top' After this patch: $ ./qemu-io file:top:image.qcow2 [no error] Signed-off-by: Max Reitz <mreitz@redhat.com> --- include/block/block_int.h | 3 +++ block.c | 35 +++++++++++++++++++++++++++++++++++ block/file-posix.c | 17 +++-------------- block/file-win32.c | 12 ++---------- 4 files changed, 43 insertions(+), 24 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index 8d3724c..e5eb473 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -682,6 +682,9 @@ int get_tmp_filename(char *filename, int size); BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size, const char *filename); +void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, + QDict *options); + /** * bdrv_add_before_write_notifier: diff --git a/block.c b/block.c index b72b872..fa1d06d 100644 --- a/block.c +++ b/block.c @@ -197,6 +197,41 @@ void path_combine(char *dest, int dest_size, } } +/* + * Helper function for bdrv_parse_filename() implementations to remove optional + * protocol prefixes (especially "file:") from a filename and for putting the + * stripped filename into the options QDict if there is such a prefix. + */ +void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, + QDict *options) +{ + if (strstart(filename, prefix, &filename)) { + /* Stripping the explicit protocol prefix may result in a protocol + * prefix being (wrongly) detected (if the filename contains a colon) */ + if (path_has_protocol(filename)) { + QString *fat_filename; + + /* This means there is some colon before the first slash; therefore, + * this cannot be an absolute path */ + assert(!path_is_absolute(filename)); + + /* And we can thus fix the protocol detection issue by prefixing it + * by "./" */ + fat_filename = qstring_from_str("./"); + qstring_append(fat_filename, filename); + + assert(!path_has_protocol(qstring_get_str(fat_filename))); + + qdict_put(options, "filename", fat_filename); + } else { + /* If no protocol prefix was detected, we can use the shortened + * filename as-is */ + qdict_put_str(options, "filename", filename); + } + } +} + + /* Returns whether the image file is opened as read-only. Note that this can * return false and writing to the image file is still not possible because the * image is inactivated. */ diff --git a/block/file-posix.c b/block/file-posix.c index 4354d49..de2d3a2 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -381,12 +381,7 @@ static void raw_parse_flags(int bdrv_flags, int *open_flags) static void raw_parse_filename(const char *filename, QDict *options, Error **errp) { - /* The filename does not have to be prefixed by the protocol name, since - * "file" is the default protocol; therefore, the return value of this - * function call can be ignored. */ - strstart(filename, "file:", &filename); - - qdict_put_str(options, "filename", filename); + bdrv_parse_filename_strip_prefix(filename, "file:", options); } static QemuOptsList raw_runtime_opts = { @@ -2395,10 +2390,7 @@ static int check_hdev_writable(BDRVRawState *s) static void hdev_parse_filename(const char *filename, QDict *options, Error **errp) { - /* The prefix is optional, just as for "file". */ - strstart(filename, "host_device:", &filename); - - qdict_put_str(options, "filename", filename); + bdrv_parse_filename_strip_prefix(filename, "host_device:", options); } static bool hdev_is_sg(BlockDriverState *bs) @@ -2697,10 +2689,7 @@ static BlockDriver bdrv_host_device = { static void cdrom_parse_filename(const char *filename, QDict *options, Error **errp) { - /* The prefix is optional, just as for "file". */ - strstart(filename, "host_cdrom:", &filename); - - qdict_put_str(options, "filename", filename); + bdrv_parse_filename_strip_prefix(filename, "host_cdrom:", options); } #endif diff --git a/block/file-win32.c b/block/file-win32.c index 8f14f0b..ef2910b 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -276,12 +276,7 @@ static void raw_parse_flags(int flags, bool use_aio, int *access_flags, static void raw_parse_filename(const char *filename, QDict *options, Error **errp) { - /* The filename does not have to be prefixed by the protocol name, since - * "file" is the default protocol; therefore, the return value of this - * function call can be ignored. */ - strstart(filename, "file:", &filename); - - qdict_put_str(options, "filename", filename); + bdrv_parse_filename_strip_prefix(filename, "file:", options); } static QemuOptsList raw_runtime_opts = { @@ -671,10 +666,7 @@ static int hdev_probe_device(const char *filename) static void hdev_parse_filename(const char *filename, QDict *options, Error **errp) { - /* The prefix is optional, just as for "file". */ - strstart(filename, "host_device:", &filename); - - qdict_put_str(options, "filename", filename); + bdrv_parse_filename_strip_prefix(filename, "host_device:", options); } static int hdev_open(BlockDriverState *bs, QDict *options, int flags, -- 2.9.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/3] block/file-*: *_parse_filename() and colons 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 2/3] block/file-*: *_parse_filename() and colons Max Reitz @ 2017-05-22 20:02 ` Eric Blake 0 siblings, 0 replies; 8+ messages in thread From: Eric Blake @ 2017-05-22 20:02 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: qemu-devel, Kevin Wolf [-- Attachment #1: Type: text/plain, Size: 926 bytes --] On 05/22/2017 02:52 PM, Max Reitz wrote: > The file drivers' *_parse_filename() implementations just strip the > optional protocol prefix off the filename. However, for e.g. > "file:foo:bar", this would lead to "foo:bar" being stored as the BDS's > filename which looks like it should be managed using the "foo" protocol. > This is especially troublesome if you then try to resolve a backing > filename based on "foo:bar". > > This issue can only occur if the stripped part is a relative filename > ("file:/foo:bar" will be shortened to "/foo:bar" and having a slash > before the first colon means that "/foo" is not recognized as a protocol > part). Therefore, we can easily fix it by prepending "./" to such > filenames. Slick. 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] iotests: Add test for colon handling 2017-05-22 19:52 [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons Max Reitz 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 1/3] " Max Reitz 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 2/3] block/file-*: *_parse_filename() and colons Max Reitz @ 2017-05-22 19:52 ` Max Reitz 2017-05-22 20:06 ` Eric Blake 2017-05-26 17:30 ` [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons Max Reitz 3 siblings, 1 reply; 8+ messages in thread From: Max Reitz @ 2017-05-22 19:52 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake 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..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 -- 2.9.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] iotests: Add test for colon handling 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 3/3] iotests: Add test for colon handling Max Reitz @ 2017-05-22 20:06 ` Eric Blake 2017-05-22 20:17 ` Max Reitz 0 siblings, 1 reply; 8+ messages in thread From: Eric Blake @ 2017-05-22 20:06 UTC (permalink / raw) To: Max Reitz, qemu-block; +Cc: qemu-devel, Kevin Wolf [-- Attachment #1: Type: text/plain, Size: 1569 bytes --] 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> -- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] iotests: Add test for colon handling 2017-05-22 20:06 ` Eric Blake @ 2017-05-22 20:17 ` Max Reitz 0 siblings, 0 replies; 8+ messages in thread From: Max Reitz @ 2017-05-22 20:17 UTC (permalink / raw) To: Eric Blake, qemu-block; +Cc: qemu-devel, Kevin Wolf [-- Attachment #1: Type: text/plain, Size: 2219 bytes --] 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 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 498 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons 2017-05-22 19:52 [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons Max Reitz ` (2 preceding siblings ...) 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 3/3] iotests: Add test for colon handling Max Reitz @ 2017-05-26 17:30 ` Max Reitz 3 siblings, 0 replies; 8+ messages in thread From: Max Reitz @ 2017-05-26 17:30 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, Kevin Wolf, Eric Blake [-- Attachment #1: Type: text/plain, Size: 708 bytes --] On 2017-05-22 21:52, Max Reitz wrote: > There are (at least) two issues with filenames that contain colons when > trying to use relative backing filenames with them, for each of which > there is a patch in this series. > > The first patch fixes an issue in the general block layer > (path_combine() does not have the same opinion on what constitutes a > protocol prefix as path_has_protocl()), the second fixes an issue in > file-* (when stripping off the optional "file:" prefix we should not > create a filename that seems to have another valid protocol prefix). > > The third patch adds a test. Applied to my block tree, with the spelling in patch 3 fixed as indicated by Eric. Max [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 498 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-05-26 17:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-22 19:52 [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons Max Reitz 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 1/3] " Max Reitz 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 2/3] block/file-*: *_parse_filename() and colons Max Reitz 2017-05-22 20:02 ` Eric Blake 2017-05-22 19:52 ` [Qemu-devel] [PATCH v2 3/3] iotests: Add test for colon handling Max Reitz 2017-05-22 20:06 ` Eric Blake 2017-05-22 20:17 ` Max Reitz 2017-05-26 17:30 ` [Qemu-devel] [PATCH v2 0/3] block: Fix backing paths for filenames with colons Max Reitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).