[Qemu-devel] [PATCH 0/2] block: JSON filenames and relative backing files

[Qemu-devel] [PATCH 0/2] block: JSON filenames and relative backing files

[Max Reitz]

Sometimes, qemu does not have a filename to work with (it then generates
a JSON filename), so it does not know which directory to use for a
backing file specified by a relative filename.

In this case, qemu should not somehow try to append the backing file's
name to the JSON object, but rather just print an error and bail out.


Max Reitz (2):
  block: JSON filenames and relative backing files
  iotests: Add test for relative backing file names

 block.c                    | 19 +++++++---
 block/qapi.c               |  7 +++-
 include/block/block.h      |  2 +-
 tests/qemu-iotests/110     | 87 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/110.out | 15 ++++++++
 tests/qemu-iotests/group   |  1 +
 6 files changed, 125 insertions(+), 6 deletions(-)
 create mode 100755 tests/qemu-iotests/110
 create mode 100644 tests/qemu-iotests/110.out

-- 
1.9.3

[Qemu-devel] [PATCH 1/2] block: JSON filenames and relative backing files

[Max Reitz]

When using a relative backing file name, qemu needs to know the
directory of the top image file. For JSON filenames, such a directory
cannot be easily determined (e.g. how do you determine the directory of
a qcow2 BDS directly on top of a quorum BDS?). Therefore, do not allow
relative filenames for the backing file of BDSs only having a JSON
filename.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
Just by the way, the reason for using bs->exact_filename in
bdrv_get_full_backing_filename() instead of just testing whether
bs->filename is prefixed by "json:" is that in the future we might have
cases where bs->filename contains a JSON filename, but
bs->exact_filename is set to a non-JSON filename (because there are some
rather vital options, which radically change performance or something
like that, so we want them to be included in bs->filename if they were
specified; but we can still generate a plain filename which results in
the same data read and written, but just in some very different way or
something like that).
Actually, I might write a follow-up patch which makes
bdrv_refresh_filename() always generate an exact_filename if somehow
possible, even if unknown options were specified. This would then be
very useful for this function, but on the other hand, it would no longer
fit the definition of exact_filename (in order to follow that
definition, we have to be certain that we don't omit any vital options
which really change the data read and written).
---
 block.c               | 19 +++++++++++++++----
 block/qapi.c          |  7 ++++++-
 include/block/block.h |  2 +-
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 88f6d9b..6ccc94b 100644
--- a/block.c
+++ b/block.c
@@ -303,12 +303,17 @@ void path_combine(char *dest, int dest_size,
     }
 }
 
-void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz)
+void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz, Error **errp)
 {
-    if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
+    if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file) ||
+        path_is_absolute(bs->backing_file)) {
         pstrcpy(dest, sz, bs->backing_file);
+    } else if (!bs->exact_filename[0] ||
+               strstart(bs->exact_filename, "json:", NULL)) {
+        error_setg(errp, "Cannot use relative backing file names for '%s'",
+                   bs->filename);
     } else {
-        path_combine(dest, sz, bs->filename, bs->backing_file);
+        path_combine(dest, sz, bs->exact_filename, bs->backing_file);
     }
 }
 
@@ -1197,7 +1202,13 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
         QDECREF(options);
         goto free_exit;
     } else {
-        bdrv_get_full_backing_filename(bs, backing_filename, PATH_MAX);
+        bdrv_get_full_backing_filename(bs, backing_filename, PATH_MAX, &local_err);
+        if (local_err) {
+            ret = -EINVAL;
+            error_propagate(errp, local_err);
+            QDECREF(options);
+            goto free_exit;
+        }
     }
 
     if (!bs->drv || !bs->drv->supports_backing) {
diff --git a/block/qapi.c b/block/qapi.c
index 1301144..77baa36 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -229,7 +229,12 @@ void bdrv_query_image_info(BlockDriverState *bs,
         info->backing_filename = g_strdup(backing_filename);
         info->has_backing_filename = true;
         bdrv_get_full_backing_filename(bs, backing_filename2,
-                                       sizeof(backing_filename2));
+                                       sizeof(backing_filename2), &err);
+        if (err) {
+            error_propagate(errp, err);
+            qapi_free_ImageInfo(info);
+            return;
+        }
 
         if (strcmp(backing_filename, backing_filename2) != 0) {
             info->full_backing_filename =
diff --git a/include/block/block.h b/include/block/block.h
index 341054d..3abc7df 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -387,7 +387,7 @@ const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
                                char *filename, int filename_size);
 void bdrv_get_full_backing_filename(BlockDriverState *bs,
-                                    char *dest, size_t sz);
+                                    char *dest, size_t sz, Error **errp);
 int bdrv_is_snapshot(BlockDriverState *bs);
 
 int path_is_absolute(const char *path);
-- 
1.9.3

[Qemu-devel] [PATCH 2/2] iotests: Add test for relative backing file names

[Max Reitz]

Sometimes, qemu does not have a filename to work with, so it does not
know which directory to use for a backing file specified by a relative
filename. Add a test which tests that qemu exits with an appropriate
error message.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/110     | 87 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/110.out | 15 ++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 103 insertions(+)
 create mode 100755 tests/qemu-iotests/110
 create mode 100644 tests/qemu-iotests/110.out

diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110
new file mode 100755
index 0000000..39b1814
--- /dev/null
+++ b/tests/qemu-iotests/110
@@ -0,0 +1,87 @@
+#!/bin/bash
+#
+# Test case for relative backing file names in complex BDS trees
+#
+# Copyright (C) 2014 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"
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# Any format supporting backing files
+_supported_fmt qed qcow qcow2 vmdk
+_supported_proto file
+_supported_os Linux
+_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
+
+TEST_IMG_REL=$(basename "$TEST_IMG")
+
+echo
+echo '=== Reconstructable filename ==='
+echo
+
+TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+_make_test_img -b "$TEST_IMG_REL.base" 64M
+# qemu should be able to reconstruct the filename, so relative backing names
+# should work
+TEST_IMG="json:{'driver':'$IMGFMT','file':{'driver':'file','filename':'$TEST_IMG'}}" \
+    _img_info | _filter_img_info
+
+echo
+echo '=== Non-reconstructable filename ==='
+echo
+
+# Across blkdebug without a config file, you cannot reconstruct filenames, so
+# qemu is incapable of knowing the directory of the top image
+TEST_IMG="json:{
+    'driver': '$IMGFMT',
+    'file': {
+        'driver': 'blkdebug',
+        'image': {
+            'driver': 'file',
+            'filename': '$TEST_IMG'
+        },
+        'set-state': [
+            {
+                'event': 'read_aio',
+                'new_state': 42
+            }
+        ]
+    }
+}" _img_info | _filter_img_info
+
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/110.out b/tests/qemu-iotests/110.out
new file mode 100644
index 0000000..09000aa
--- /dev/null
+++ b/tests/qemu-iotests/110.out
@@ -0,0 +1,15 @@
+QA output created by 110
+
+=== Reconstructable filename ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='t.IMGFMT.base' 
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
+
+=== Non-reconstructable filename ===
+
+qemu-img: Cannot use relative backing file names for 'json:{"driver": "IMGFMT", "file": {"set-state": [{"new_state": 42, "state": 0, "event": "read_aio"}], "image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug"}}'
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 9bbd5d3..c21b7fe 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -109,3 +109,4 @@
 105 rw auto quick
 107 rw auto quick
 108 rw auto quick
+110 rw auto backing quick
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/2] block: JSON filenames and relative backing files

[Eric Blake]

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

On 10/23/2014 08:56 AM, Max Reitz wrote:
> When using a relative backing file name, qemu needs to know the
> directory of the top image file. For JSON filenames, such a directory
> cannot be easily determined (e.g. how do you determine the directory of
> a qcow2 BDS directly on top of a quorum BDS?). Therefore, do not allow
> relative filenames for the backing file of BDSs only having a JSON
> filename.
> 

Are JSON names the only case where we want to do this, or should we
widen it to all non-file protocol names?  Then again, the use of
relative names is currently being used as a NICE way on glusterfs setups
to hide whether two files are both being accessed via gluster protocol
or both being accessed via file protocol (if the gluster volume is also
mapped to the local file system such as via NFS) - that is, referring to
'base.img' as the backing file of 'top.img' works whether you opened
'/path/to/top.img' or 'gluster://host/volume/path/to/top.img', when
base.img and top.img are in the same directory of the gluster volume.

> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> Just by the way, the reason for using bs->exact_filename in
> bdrv_get_full_backing_filename() instead of just testing whether
> bs->filename is prefixed by "json:" is that in the future we might have
> cases where bs->filename contains a JSON filename, but
> bs->exact_filename is set to a non-JSON filename (because there are some
> rather vital options, which radically change performance or something
> like that, so we want them to be included in bs->filename if they were
> specified; but we can still generate a plain filename which results in
> the same data read and written, but just in some very different way or
> something like that).
> Actually, I might write a follow-up patch which makes
> bdrv_refresh_filename() always generate an exact_filename if somehow
> possible, even if unknown options were specified. This would then be
> very useful for this function, but on the other hand, it would no longer
> fit the definition of exact_filename (in order to follow that
> definition, we have to be certain that we don't omit any vital options
> which really change the data read and written).
> ---
>  block.c               | 19 +++++++++++++++----
>  block/qapi.c          |  7 ++++++-
>  include/block/block.h |  2 +-
>  3 files changed, 22 insertions(+), 6 deletions(-)
> 

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

-- 
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: 539 bytes --]

Re: [Qemu-devel] [PATCH 2/2] iotests: Add test for relative backing file names

[Eric Blake]

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

On 10/23/2014 08:56 AM, Max Reitz wrote:
> Sometimes, qemu does not have a filename to work with, so it does not
> know which directory to use for a backing file specified by a relative
> filename. Add a test which tests that qemu exits with an appropriate
> error message.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/110     | 87 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/110.out | 15 ++++++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 103 insertions(+)
>  create mode 100755 tests/qemu-iotests/110
>  create mode 100644 tests/qemu-iotests/110.out
> 

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

-- 
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: 539 bytes --]

Re: [Qemu-devel] [PATCH 0/2] block: JSON filenames and relative backing files

[Eric Blake]

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

On 10/23/2014 08:56 AM, Max Reitz wrote:
> Sometimes, qemu does not have a filename to work with (it then generates
> a JSON filename), so it does not know which directory to use for a
> backing file specified by a relative filename.
> 
> In this case, qemu should not somehow try to append the backing file's
> name to the JSON object, but rather just print an error and bail out.

Hmm. Makes me wonder if we should extend BlockdevOptions to allow for a
BDS to additionally track a notion of "current directory" from which any
relative names should be interpreted against.  That is, something like:

 TEST_IMG="json:{
     'driver': '$IMGFMT',
     'file': {
         'driver': 'blkdebug',
         'image': {
             'driver': 'file',
             'filename': '$TEST_IMG'
         },
+        'rel-dir': '$DIR_OF_TEST_IMG_BACK',
         'set-state': [
             {
                 'event': 'read_aio',
                 'new_state': 42
             }
         ]
     }
 }" _img_info | _filter_img_info

being a way to let us access the (otherwise relative-only) backing file
encoded behind the blkdebug wall with a notion of the correct directory
to find it in.  Of course, such an extension to BDS description (for
both command line and QMP) would be a separate series...

-- 
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: 539 bytes --]

Re: [Qemu-devel] [PATCH 1/2] block: JSON filenames and relative backing files

[Max Reitz]

On 23.10.2014 19:42, Eric Blake wrote:
> On 10/23/2014 08:56 AM, Max Reitz wrote:
>> When using a relative backing file name, qemu needs to know the
>> directory of the top image file. For JSON filenames, such a directory
>> cannot be easily determined (e.g. how do you determine the directory of
>> a qcow2 BDS directly on top of a quorum BDS?). Therefore, do not allow
>> relative filenames for the backing file of BDSs only having a JSON
>> filename.
>>
> Are JSON names the only case where we want to do this, or should we
> widen it to all non-file protocol names?

It'll probably work for HTTP, NFS of course and I can see it working for 
NBD, too, if one is crazy enough to do that (and you're mentioning 
glusterfs). In general, I think all filenames have some normal 
unix-path-like sequence as their tail, so relative filenames can work 
there (and maybe there are even people using it already for all kinds of 
non-file protocols).

> Then again, the use of
> relative names is currently being used as a NICE way on glusterfs setups
> to hide whether two files are both being accessed via gluster protocol
> or both being accessed via file protocol (if the gluster volume is also
> mapped to the local file system such as via NFS) - that is, referring to
> 'base.img' as the backing file of 'top.img' works whether you opened
> '/path/to/top.img' or 'gluster://host/volume/path/to/top.img', when
> base.img and top.img are in the same directory of the gluster volume.
>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> Just by the way, the reason for using bs->exact_filename in
>> bdrv_get_full_backing_filename() instead of just testing whether
>> bs->filename is prefixed by "json:" is that in the future we might have
>> cases where bs->filename contains a JSON filename, but
>> bs->exact_filename is set to a non-JSON filename (because there are some
>> rather vital options, which radically change performance or something
>> like that, so we want them to be included in bs->filename if they were
>> specified; but we can still generate a plain filename which results in
>> the same data read and written, but just in some very different way or
>> something like that).
>> Actually, I might write a follow-up patch which makes
>> bdrv_refresh_filename() always generate an exact_filename if somehow
>> possible, even if unknown options were specified. This would then be
>> very useful for this function, but on the other hand, it would no longer
>> fit the definition of exact_filename (in order to follow that
>> definition, we have to be certain that we don't omit any vital options
>> which really change the data read and written).
>> ---
>>   block.c               | 19 +++++++++++++++----
>>   block/qapi.c          |  7 ++++++-
>>   include/block/block.h |  2 +-
>>   3 files changed, 22 insertions(+), 6 deletions(-)
>>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Max

Re: [Qemu-devel] [PATCH 0/2] block: JSON filenames and relative backing files

[Stefan Hajnoczi]

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

On Thu, Oct 23, 2014 at 04:56:13PM +0200, Max Reitz wrote:
> Sometimes, qemu does not have a filename to work with (it then generates
> a JSON filename), so it does not know which directory to use for a
> backing file specified by a relative filename.
> 
> In this case, qemu should not somehow try to append the backing file's
> name to the JSON object, but rather just print an error and bail out.
> 
> 
> Max Reitz (2):
>   block: JSON filenames and relative backing files
>   iotests: Add test for relative backing file names
> 
>  block.c                    | 19 +++++++---
>  block/qapi.c               |  7 +++-
>  include/block/block.h      |  2 +-
>  tests/qemu-iotests/110     | 87 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/110.out | 15 ++++++++
>  tests/qemu-iotests/group   |  1 +
>  6 files changed, 125 insertions(+), 6 deletions(-)
>  create mode 100755 tests/qemu-iotests/110
>  create mode 100644 tests/qemu-iotests/110.out
> 
> -- 
> 1.9.3
> 
> 

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

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 1/2] block: JSON filenames and relative backing files

[Kevin Wolf]

Am 23.10.2014 um 20:00 hat Max Reitz geschrieben:
> On 23.10.2014 19:42, Eric Blake wrote:
> >On 10/23/2014 08:56 AM, Max Reitz wrote:
> >>When using a relative backing file name, qemu needs to know the
> >>directory of the top image file. For JSON filenames, such a directory
> >>cannot be easily determined (e.g. how do you determine the directory of
> >>a qcow2 BDS directly on top of a quorum BDS?). Therefore, do not allow
> >>relative filenames for the backing file of BDSs only having a JSON
> >>filename.
> >>
> >Are JSON names the only case where we want to do this, or should we
> >widen it to all non-file protocol names?
> 
> It'll probably work for HTTP, NFS of course and I can see it working
> for NBD, too, if one is crazy enough to do that (and you're
> mentioning glusterfs). In general, I think all filenames have some
> normal unix-path-like sequence as their tail, so relative filenames
> can work there (and maybe there are even people using it already for
> all kinds of non-file protocols).

Perhaps make path_combine() a BlockDriver callback so that each block
driver can implement relative paths the way they make most sense for it,
or NULL to forbid them? We can still keep a common implementation for
"normal" paths in block.c and reference that in raw-posix, curl, etc.

Not sure how tricky it will become to integrate any of this relative
file name handling nicely in a future, completely QDict-based
bdrv_open()...

Kevin

Re: [Qemu-devel] [PATCH 0/2] block: JSON filenames and relative backing files

[Stefan Hajnoczi]

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

On Thu, Oct 23, 2014 at 04:56:13PM +0200, Max Reitz wrote:
> Sometimes, qemu does not have a filename to work with (it then generates
> a JSON filename), so it does not know which directory to use for a
> backing file specified by a relative filename.
> 
> In this case, qemu should not somehow try to append the backing file's
> name to the JSON object, but rather just print an error and bail out.
> 
> 
> Max Reitz (2):
>   block: JSON filenames and relative backing files
>   iotests: Add test for relative backing file names
> 
>  block.c                    | 19 +++++++---
>  block/qapi.c               |  7 +++-
>  include/block/block.h      |  2 +-
>  tests/qemu-iotests/110     | 87 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/110.out | 15 ++++++++
>  tests/qemu-iotests/group   |  1 +
>  6 files changed, 125 insertions(+), 6 deletions(-)
>  create mode 100755 tests/qemu-iotests/110
>  create mode 100644 tests/qemu-iotests/110.out

Unfortunately this breaks qemu-iotests check -vmdk 110 so I had to drop
the patches.

110 0s ... - output mismatch (see 110.out.bad)
--- /home/stefanha/qemu/tests/qemu-iotests/110.out	2014-11-03 09:48:42.157955288 +0000
+++ 110.out.bad	2014-11-03 10:11:59.469231898 +0000
@@ -3,13 +3,26 @@
 === Reconstructable filename ===
 
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
+qemu-img: TEST_DIR/t.IMGFMT: Could not open 't.IMGFMT.base': No such file or directory
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='t.IMGFMT.base' 
-image: TEST_DIR/t.IMGFMT
-file format: IMGFMT
-virtual size: 64M (67108864 bytes)
-backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
+qemu-img: Could not open 'json:{'driver':'IMGFMT','file':{'driver':'file','filename':'TEST_DIR/t.IMGFMT'}}': Could not open 'TEST_DIR/t.IMGFMT': No such file or directory
 
 === Non-reconstructable filename ===
 
-qemu-img: Cannot use relative backing file names for 'json:{"driver": "IMGFMT", "file": {"set-state": [{"new_state": 42, "state": 0, "event": "read_aio"}], "image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug"}}'
+qemu-img: Could not open 'json:{
+    'driver': 'IMGFMT',
+    'file': {
+        'driver': 'blkdebug',
+        'image': {
+            'driver': 'file',
+            'filename': 'TEST_DIR/t.IMGFMT'
+        },
+        'set-state': [
+            {
+                'event': 'read_aio',
+                'new_state': 42
+            }
+        ]
+    }
+}': Could not open 'TEST_DIR/t.IMGFMT': No such file or directory

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 0/2] block: JSON filenames and relative backing files

[Max Reitz]

On 2014-11-03 at 12:41, Stefan Hajnoczi wrote:
> On Thu, Oct 23, 2014 at 04:56:13PM +0200, Max Reitz wrote:
>> Sometimes, qemu does not have a filename to work with (it then generates
>> a JSON filename), so it does not know which directory to use for a
>> backing file specified by a relative filename.
>>
>> In this case, qemu should not somehow try to append the backing file's
>> name to the JSON object, but rather just print an error and bail out.
>>
>>
>> Max Reitz (2):
>>    block: JSON filenames and relative backing files
>>    iotests: Add test for relative backing file names
>>
>>   block.c                    | 19 +++++++---
>>   block/qapi.c               |  7 +++-
>>   include/block/block.h      |  2 +-
>>   tests/qemu-iotests/110     | 87 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/110.out | 15 ++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   6 files changed, 125 insertions(+), 6 deletions(-)
>>   create mode 100755 tests/qemu-iotests/110
>>   create mode 100644 tests/qemu-iotests/110.out
> Unfortunately this breaks qemu-iotests check -vmdk 110 so I had to drop
> the patches.
>
> 110 0s ... - output mismatch (see 110.out.bad)
> --- /home/stefanha/qemu/tests/qemu-iotests/110.out	2014-11-03 09:48:42.157955288 +0000
> +++ 110.out.bad	2014-11-03 10:11:59.469231898 +0000
> @@ -3,13 +3,26 @@
>   === Reconstructable filename ===
>   
>   Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
> +qemu-img: TEST_DIR/t.IMGFMT: Could not open 't.IMGFMT.base': No such file or directory
>   Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='t.IMGFMT.base'
> -image: TEST_DIR/t.IMGFMT
> -file format: IMGFMT
> -virtual size: 64M (67108864 bytes)
> -backing file: t.IMGFMT.base (actual path: TEST_DIR/t.IMGFMT.base)
> +qemu-img: Could not open 'json:{'driver':'IMGFMT','file':{'driver':'file','filename':'TEST_DIR/t.IMGFMT'}}': Could not open 'TEST_DIR/t.IMGFMT': No such file or directory
>   
>   === Non-reconstructable filename ===
>   
> -qemu-img: Cannot use relative backing file names for 'json:{"driver": "IMGFMT", "file": {"set-state": [{"new_state": 42, "state": 0, "event": "read_aio"}], "image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, "driver": "blkdebug"}}'
> +qemu-img: Could not open 'json:{
> +    'driver': 'IMGFMT',
> +    'file': {
> +        'driver': 'blkdebug',
> +        'image': {
> +            'driver': 'file',
> +            'filename': 'TEST_DIR/t.IMGFMT'
> +        },
> +        'set-state': [
> +            {
> +                'event': 'read_aio',
> +                'new_state': 42
> +            }
> +        ]
> +    }
> +}': Could not open 'TEST_DIR/t.IMGFMT': No such file or directory

Well, it doesn't break the test, the new test just doesn't work. The 
reason it doesn't work seems to be a bug in VMDK, as far as I can tell 
(although I don't know anything about VMDK, basically): In contrast to 
the rest of the block layer, it uses the backing file name directly, 
that is, relative to qemu's working directory, instead of relative to 
the image's directory.

I'll see whether I can fix that, and if I can't, I'll just send the v2 
with VMDK removed from the list of supported formats.

Max