Am 10.07.2013 um 19:09 hat Eric Blake geschrieben: > On 07/10/2013 07:51 AM, Kevin Wolf wrote: > > One of the major reasons for doing something new for -blockdev and > > blockdev-add was that the old block layer code parses filenames instead > > of just taking them literally. So we should really leave it untouched > > when it's passing using the new interfaces (like -drive > > file.filename=...). > > > > This allows opening relative file names that contain a colon. > > Will a protocol prefix ever contain a '/'? Would it be desirable to > state that relative file names containing a colon should be specified as > './file:name', with the '/' serving as the escape that means relative > file rather than attempting to use protocol './file:', even when using > legacy options?
In fact, that already works, but it's kind of non-obvious magic (see path_has_protocol), whereas file.filename should works in a consistent way. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > > --- > > block.c | 17 ++++++++++++----- > > block/sheepdog.c | 2 +- > > include/block/block.h | 3 ++- > > qemu-img.c | 4 ++-- > > tests/qemu-iotests/051 | 12 ++++++++++++ > > tests/qemu-iotests/051.out | 14 ++++++++++++++ > > 6 files changed, 43 insertions(+), 9 deletions(-) > > > > > @@ -813,7 +817,10 @@ int bdrv_file_open(BlockDriverState **pbs, const char > > *filename, > > drv = bdrv_find_whitelisted_format(drvname, !(flags & > > BDRV_O_RDWR)); > > qdict_del(options, "driver"); > > } else if (filename) { > > - drv = bdrv_find_protocol(filename); > > + drv = bdrv_find_protocol(filename, allow_protocol_prefix); > > + if (!drv) { > > + qerror_report(ERROR_CLASS_GENERIC_ERROR, "Unknown protocol"); > > + } > > If you want to allow './' as a forceful non-protocol escape even in > legacy parsing, then this code may need tweaking. Otherwise, I think > the code looks fine. > > Reviewed-by: Eric Blake <ebl...@redhat.com> > > > +++ b/tests/qemu-iotests/051 > > @@ -149,6 +149,18 @@ echo > > run_qemu -drive file=$TEST_IMG,file.driver=file > > run_qemu -drive file=$TEST_IMG,file.driver=qcow2 > > > > +echo > > +echo === Parsing protocol from file name === > > +echo > > + > > +# Protocol strings are supposed to be parsed from traditional option > > strings, > > +# but not when using driver-specific options. We can distinguish them by > > the > > +# error message for non-existing files. > > Is it also worth testing that we successfully open a file name with a > colon from driver-specific options, or is that harder to do portably > (since windows doesn't allow : in file names except for the drive prefix)? The hard thing is that $TEST_DIR and $TEST_IMG are absolute paths and I would need relative ones to test this. And I don't want to create files outside the test directory. We don't really care about portability in qemu-iotests (and there's always '_supported_os Linux'). Kevin