[Qemu-devel] [PATCH v4 0/6] block: Relative backing files

[Qemu-devel] [PATCH v4 0/6] block: Relative backing files

[Max Reitz]

Cover letter of v1:

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.


Part of the cover letter specific to v2 (with patch numbers fixed):

Stefan already applied v1 before realizing that the test added in the
series was actually broken for vmdk (which I somehow missed myself).
This was due to vmdk trying to open the backing file on creation in
order to determine its format; however, normally the backing file path
is interpreted relatively to the backed image's base directory, whereas
in this case vmdk directly used the user-specified filename which was
therefore interpreted relatively to qemu's working directory.

Patch 5 of this v2 (v4) fixes this. A similar issue exists directly in
bdrv_img_create() which opens the backing file in order to determine its
size in case the size of the new image has not been specified. This is
fixed by patch 4.

The function both patches use is factored out from
bdrv_get_full_backing_filename() (which we cannot use here because it
requires a BDS which does not necessarily exist during image creation)
in patch 2. Patch 3 was then modified so it modifies this new function
(bdrv_get_full_backing_filename_from_filename()) and the old
bdrv_get_full_backing_filename(), which is now more or less just a
wrapper.

And finally, I added a test to patch 6 which tests creation of a backed
image in another directory only using relative paths while omitting the
image size (which is therefore inferred from the backing file).


v4:
- Added patch 1: Patch 3 (prev. 2) failed checkpatch.pl, so do the
  obvious: Fix checkpatch.pl.
- Patch 3 (prev. 2): Fix overly long line [Fam]
- Patch 6 (prev. 5): s/shoud/should/ [Eric]


git-backport-diff against v3:

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/6:[down] 'checkpatch: Brace handling on multi-line condition'
002/6:[----] [--] 'block: Get full backing filename from string'
003/6:[0003] [FC] 'block: JSON filenames and relative backing files'
004/6:[----] [--] 'block: Relative backing file for image creation'
005/6:[----] [--] 'block/vmdk: Relative backing file for creation'
006/6:[0002] [FC] 'iotests: Add test for relative backing file names'


Max Reitz (6):
  checkpatch: Brace handling on multi-line condition
  block: Get full backing filename from string
  block: JSON filenames and relative backing files
  block: Relative backing file for image creation
  block/vmdk: Relative backing file for creation
  iotests: Add test for relative backing file names

 block.c                    | 46 ++++++++++++++++++++---
 block/qapi.c               |  7 +++-
 block/vmdk.c               | 13 ++++++-
 include/block/block.h      |  6 ++-
 scripts/checkpatch.pl      | 13 ++++++-
 tests/qemu-iotests/110     | 94 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/110.out | 19 ++++++++++
 tests/qemu-iotests/group   |  1 +
 8 files changed, 188 insertions(+), 11 deletions(-)
 create mode 100755 tests/qemu-iotests/110
 create mode 100644 tests/qemu-iotests/110.out

-- 
1.9.3

[Qemu-devel] [PATCH v4 1/6] checkpatch: Brace handling on multi-line condition

[Max Reitz]

CODING_STYLE states the following about braces around blocks:

> The opening brace is on the line that contains the control flow
> statement that introduces the new block; [...]

This is obviously impossible with multi-line conditions. Therefore,
CODING_STYLE does not make any clear statement about where to put the
opening brace after a multi-line condition.

There is a reason to prefer to place the opening brace on an own line
after such a condition while still placing it on the same line as the
"control flow statement" if possible; that reason is that the last line
of a multi-line condition is indented, in the case of "if", it is often
indented by four spaces, just as much as the first statement in the
block will be indented. This is hard to read as there is no clearly
visible distinction between condition and block. Placing the opening
brace on a separate line solves this issue.

Also, there are cases where placing the opening brace on a separate line
is the only viable option; if the previous line had nearly 80 characters
and splitting it is not desirable, the opening brace is naturally placed
on an own line.

This patch fixes checkpatch.pl to not complain about braces on own lines
if the condition introducing the block spanned more than one line, or if
the previous line had 79 or 80 characters.

Furthermore, the warning about not having braces around a block is fixed
to mind braces not being on the last line of the condition.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 scripts/checkpatch.pl | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 053e432..5df61f9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1639,7 +1639,13 @@ sub process {
 			#print "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n";
 			#print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
 
-			if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
+			# The length of the "previous line" is checked against 80 because it
+			# includes the + at the beginning of the line (if the actual line has
+			# 79 or 80 characters, it is no longer possible to add a space and an
+			# opening brace there)
+			if ($#ctx == 0 && $ctx !~ /{\s*/ &&
+			    defined($lines[$ctx_ln - 1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/ &&
+			    defined($lines[$ctx_ln - 2]) && length($lines[$ctx_ln - 2]) < 80) {
 				ERROR("that open brace { should be on the previous line\n" .
 					"$here\n$ctx\n$rawlines[$ctx_ln - 1]\n");
 			}
@@ -2542,7 +2548,10 @@ sub process {
 
 					substr($block, 0, length($cond), '');
 
-					$seen++ if ($block =~ /^\s*{/);
+					my $spaced_block = $block;
+					$spaced_block =~ s/\n\+/ /g;
+
+					$seen++ if ($spaced_block =~ /^\s*{/);
 
                                         print "APW: cond<$cond> block<$block> allowed<$allowed>\n"
                                             if $dbg_adv_apw;
-- 
1.9.3

[Qemu-devel] [PATCH v4 2/6] block: Get full backing filename from string

[Max Reitz]

Introduce bdrv_get_full_backing_filename_from_filename(), a function
which takes the name of the backed file and a potentially relative
backing filename to produce the full (absolute) backing filename.

Use this function from bdrv_get_full_backing_filename().

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 16 ++++++++++++----
 include/block/block.h |  3 +++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 866c8b4..0c1be37 100644
--- a/block.c
+++ b/block.c
@@ -303,15 +303,23 @@ 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_from_filename(const char *backed,
+                                                  const char *backing,
+                                                  char *dest, size_t sz)
 {
-    if (bs->backing_file[0] == '\0' || path_has_protocol(bs->backing_file)) {
-        pstrcpy(dest, sz, bs->backing_file);
+    if (backing[0] == '\0' || path_has_protocol(backing)) {
+        pstrcpy(dest, sz, backing);
     } else {
-        path_combine(dest, sz, bs->filename, bs->backing_file);
+        path_combine(dest, sz, backed, backing);
     }
 }
 
+void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t sz)
+{
+    bdrv_get_full_backing_filename_from_filename(bs->filename, bs->backing_file,
+                                                 dest, sz);
+}
+
 void bdrv_register(BlockDriver *bdrv)
 {
     /* Block drivers without coroutine functions need emulation */
diff --git a/include/block/block.h b/include/block/block.h
index 610be9f..1c7ecaa 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -399,6 +399,9 @@ 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);
+void bdrv_get_full_backing_filename_from_filename(const char *backed,
+                                                  const char *backing,
+                                                  char *dest, size_t sz);
 int bdrv_is_snapshot(BlockDriverState *bs);
 
 int path_is_absolute(const char *path);
-- 
1.9.3

[Qemu-devel] [PATCH v4 3/6] 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.

Furthermore, BDS::exact_filename should be used whenever possible. If
BDS::filename is not equal to BDS::exact_filename, the former will
always be a JSON object.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 28 ++++++++++++++++++++++------
 block/qapi.c          |  7 ++++++-
 include/block/block.h |  5 +++--
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 0c1be37..0d1869c 100644
--- a/block.c
+++ b/block.c
@@ -305,19 +305,28 @@ void path_combine(char *dest, int dest_size,
 
 void bdrv_get_full_backing_filename_from_filename(const char *backed,
                                                   const char *backing,
-                                                  char *dest, size_t sz)
+                                                  char *dest, size_t sz,
+                                                  Error **errp)
 {
-    if (backing[0] == '\0' || path_has_protocol(backing)) {
+    if (backing[0] == '\0' || path_has_protocol(backing) ||
+        path_is_absolute(backing))
+    {
         pstrcpy(dest, sz, backing);
+    } else if (backed[0] == '\0' || strstart(backed, "json:", NULL)) {
+        error_setg(errp, "Cannot use relative backing file names for '%s'",
+                   backed);
     } else {
         path_combine(dest, sz, backed, backing);
     }
 }
 
-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)
 {
-    bdrv_get_full_backing_filename_from_filename(bs->filename, bs->backing_file,
-                                                 dest, sz);
+    char *backed = bs->exact_filename[0] ? bs->exact_filename : bs->filename;
+
+    bdrv_get_full_backing_filename_from_filename(backed, bs->backing_file,
+                                                 dest, sz, errp);
 }
 
 void bdrv_register(BlockDriver *bdrv)
@@ -1209,7 +1218,14 @@ 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 fa68ba7..a6fd6f7 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -214,7 +214,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 1c7ecaa..f34f63c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -398,10 +398,11 @@ 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);
 void bdrv_get_full_backing_filename_from_filename(const char *backed,
                                                   const char *backing,
-                                                  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 v4 4/6] block: Relative backing file for image creation

[Max Reitz]

Relative backing filenames are always relative to the backed image's
directory; the same applies to image creation. Therefore, if the backing
file has to be opened for determining its size (in case the size has not
been explicitly specified) its filename should be interpreted relative
to the new image's base directory and not relative to qemu's working
directory.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 block.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 0d1869c..65f31ef 100644
--- a/block.c
+++ b/block.c
@@ -5634,16 +5634,26 @@ void bdrv_img_create(const char *filename, const char *fmt,
     if (size == -1) {
         if (backing_file) {
             BlockDriverState *bs;
+            char *full_backing = g_new0(char, PATH_MAX);
             int64_t size;
             int back_flags;
 
+            bdrv_get_full_backing_filename_from_filename(filename, backing_file,
+                                                         full_backing, PATH_MAX,
+                                                         &local_err);
+            if (local_err) {
+                g_free(full_backing);
+                goto out;
+            }
+
             /* backing files always opened read-only */
             back_flags =
                 flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
 
             bs = NULL;
-            ret = bdrv_open(&bs, backing_file, NULL, NULL, back_flags,
+            ret = bdrv_open(&bs, full_backing, NULL, NULL, back_flags,
                             backing_drv, &local_err);
+            g_free(full_backing);
             if (ret < 0) {
                 goto out;
             }
-- 
1.9.3

[Qemu-devel] [PATCH v4 5/6] block/vmdk: Relative backing file for creation

[Max Reitz]

When a vmdk image is created with a backing file, it is opened to check
whether it is indeed a vmdk file by letting qemu probe it. When doing
so, the backing filename is relative to the image's base directory so it
should be interpreted accordingly.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2cbfd3e..aea8f07 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1868,8 +1868,19 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
     }
     if (backing_file) {
         BlockDriverState *bs = NULL;
-        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_NO_BACKING, NULL,
+        char *full_backing = g_new0(char, PATH_MAX);
+        bdrv_get_full_backing_filename_from_filename(filename, backing_file,
+                                                     full_backing, PATH_MAX,
+                                                     &local_err);
+        if (local_err) {
+            g_free(full_backing);
+            error_propagate(errp, local_err);
+            ret = -ENOENT;
+            goto exit;
+        }
+        ret = bdrv_open(&bs, full_backing, NULL, NULL, BDRV_O_NO_BACKING, NULL,
                         errp);
+        g_free(full_backing);
         if (ret != 0) {
             goto exit;
         }
-- 
1.9.3

[Qemu-devel] [PATCH v4 6/6] 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.

Additionally, add a test for qemu-img create with a backing filename
relative to the backed image's base directory while omitting the image
size.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/110     | 94 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/110.out | 19 ++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 114 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..a687f95
--- /dev/null
+++ b/tests/qemu-iotests/110
@@ -0,0 +1,94 @@
+#!/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
+
+echo
+echo '=== Backing name is always relative to the backed image ==='
+echo
+
+# omit the image size; it should work anyway
+_make_test_img -b "$TEST_IMG_REL.base"
+
+
+# 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..056ffec
--- /dev/null
+++ b/tests/qemu-iotests/110.out
@@ -0,0 +1,19 @@
+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"}}'
+
+=== Backing name is always relative to the backed image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='t.IMGFMT.base'
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 7dfe469..de3c643 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -111,4 +111,5 @@
 105 rw auto quick
 107 rw auto quick
 108 rw auto quick
+110 rw auto backing quick
 111 rw auto quick
-- 
1.9.3

Re: [Qemu-devel] [PATCH v4 1/6] checkpatch: Brace handling on multi-line condition

[Eric Blake]

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

On 11/26/2014 09:20 AM, Max Reitz wrote:
> CODING_STYLE states the following about braces around blocks:
> 
>> The opening brace is on the line that contains the control flow
>> statement that introduces the new block; [...]
> 
> This is obviously impossible with multi-line conditions. Therefore,
> CODING_STYLE does not make any clear statement about where to put the
> opening brace after a multi-line condition.

I'll leave review of this one to others (personally, I'm 80:20 against
applying it, but my vote is very weak if you scale it by the percentage
of actual C code contributions existing in the tree under my name - so
I'll live with majority rules).

-- 
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 v4 3/6] block: JSON filenames and relative backing files

[Eric Blake]

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

On 11/26/2014 09:20 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.
> 
> Furthermore, BDS::exact_filename should be used whenever possible. If
> BDS::filename is not equal to BDS::exact_filename, the former will
> always be a JSON object.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c               | 28 ++++++++++++++++++++++------
>  block/qapi.c          |  7 ++++++-
>  include/block/block.h |  5 +++--
>  3 files changed, 31 insertions(+), 9 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 v4 0/6] block: Relative backing files

[Max Reitz]

On 2014-11-26 at 17:20, Max Reitz wrote:
> Cover letter of v1:
>
> 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.
>
>
> Part of the cover letter specific to v2 (with patch numbers fixed):
>
> Stefan already applied v1 before realizing that the test added in the
> series was actually broken for vmdk (which I somehow missed myself).
> This was due to vmdk trying to open the backing file on creation in
> order to determine its format; however, normally the backing file path
> is interpreted relatively to the backed image's base directory, whereas
> in this case vmdk directly used the user-specified filename which was
> therefore interpreted relatively to qemu's working directory.
>
> Patch 5 of this v2 (v4) fixes this. A similar issue exists directly in
> bdrv_img_create() which opens the backing file in order to determine its
> size in case the size of the new image has not been specified. This is
> fixed by patch 4.
>
> The function both patches use is factored out from
> bdrv_get_full_backing_filename() (which we cannot use here because it
> requires a BDS which does not necessarily exist during image creation)
> in patch 2. Patch 3 was then modified so it modifies this new function
> (bdrv_get_full_backing_filename_from_filename()) and the old
> bdrv_get_full_backing_filename(), which is now more or less just a
> wrapper.
>
> And finally, I added a test to patch 6 which tests creation of a backed
> image in another directory only using relative paths while omitting the
> image size (which is therefore inferred from the backing file).
>
>
> v4:
> - Added patch 1: Patch 3 (prev. 2) failed checkpatch.pl, so do the
>    obvious: Fix checkpatch.pl.
> - Patch 3 (prev. 2): Fix overly long line [Fam]
> - Patch 6 (prev. 5): s/shoud/should/ [Eric]
>
>
> git-backport-diff against v3:
>
> 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/6:[down] 'checkpatch: Brace handling on multi-line condition'
> 002/6:[----] [--] 'block: Get full backing filename from string'
> 003/6:[0003] [FC] 'block: JSON filenames and relative backing files'
> 004/6:[----] [--] 'block: Relative backing file for image creation'
> 005/6:[----] [--] 'block/vmdk: Relative backing file for creation'
> 006/6:[0002] [FC] 'iotests: Add test for relative backing file names'
>
>
> Max Reitz (6):
>    checkpatch: Brace handling on multi-line condition
>    block: Get full backing filename from string
>    block: JSON filenames and relative backing files
>    block: Relative backing file for image creation
>    block/vmdk: Relative backing file for creation
>    iotests: Add test for relative backing file names
>
>   block.c                    | 46 ++++++++++++++++++++---
>   block/qapi.c               |  7 +++-
>   block/vmdk.c               | 13 ++++++-
>   include/block/block.h      |  6 ++-
>   scripts/checkpatch.pl      | 13 ++++++-
>   tests/qemu-iotests/110     | 94 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/110.out | 19 ++++++++++
>   tests/qemu-iotests/group   |  1 +
>   8 files changed, 188 insertions(+), 11 deletions(-)
>   create mode 100755 tests/qemu-iotests/110
>   create mode 100644 tests/qemu-iotests/110.out

Ping

Re: [Qemu-devel] [PATCH v4 0/6] block: Relative backing files

[Kevin Wolf]

Am 26.11.2014 um 17:20 hat Max Reitz geschrieben:
> Cover letter of v1:
> 
> 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.

Thanks, applied all to the block branch.

Kevin