[Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames

[Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames

[Jeff Cody]

Differences from v1:

Patch 1: set local_error = NULL after freeing (Thanks Eric)
Patch 2: new patch, groundwork for qemu-iotest in patch 3
Patch 3: qemu-iotest, as requested (Thanks Max)


Jeff Cody (3):
  block: check full backing filename when searching protocol filenames
  qemu-iotests: Don't create fifos / pidfiles with protocol paths
  qemu-iotest: test to lookup protocol-based image with relative backing

 block.c                          | 13 ++++++
 tests/qemu-iotests/173           | 97 ++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/173.out       | 12 +++++
 tests/qemu-iotests/common.config |  6 ++-
 tests/qemu-iotests/common.qemu   | 10 ++---
 tests/qemu-iotests/common.rc     |  6 +--
 tests/qemu-iotests/group         |  1 +
 7 files changed, 135 insertions(+), 10 deletions(-)
 create mode 100755 tests/qemu-iotests/173
 create mode 100644 tests/qemu-iotests/173.out

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/3] block: check full backing filename when searching protocol filenames

[Jeff Cody]

In bdrv_find_backing_image(), if we are searching an image for a backing
file that contains a protocol, we currently only compare unmodified
paths.

However, some management software will change the backing filename to be
a relative filename in a path.  QEMU is able to handle this fine,
because internally it will use path_combine to put together the full
protocol URI.

However, this can lead to an inability to match an image during a QAPI
command that needs to use bdrv_find_backing_image() to find the image,
when it is searched by the full URI.

When searching for a protocol filename, if the straight comparison
fails, this patch will also compare against the full backing filename to
see if that is a match.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/block.c b/block.c
index 39ddea3..d97c009 100644
--- a/block.c
+++ b/block.c
@@ -3145,6 +3145,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
     int is_protocol = 0;
     BlockDriverState *curr_bs = NULL;
     BlockDriverState *retval = NULL;
+    Error *local_error = NULL;
 
     if (!bs || !bs->drv || !backing_file) {
         return NULL;
@@ -3165,6 +3166,18 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
                 retval = curr_bs->backing->bs;
                 break;
             }
+            /* Also check against the full backing filename for the image */
+            bdrv_get_full_backing_filename(curr_bs, backing_file_full, PATH_MAX,
+                                           &local_error);
+            if (local_error == NULL) {
+                if (strcmp(backing_file, backing_file_full) == 0) {
+                    retval = curr_bs->backing->bs;
+                    break;
+                }
+            } else {
+                error_free(local_error);
+                local_error = NULL;
+            }
         } else {
             /* If not an absolute filename path, make it relative to the current
              * image's filename path */
-- 
2.9.3

[Qemu-devel] [PATCH v2 2/3] qemu-iotests: Don't create fifos / pidfiles with protocol paths

[Jeff Cody]

Trying to create, use, and remove fifos and pidfiles on protocol paths
(e.g. nfs://localhost/scratch/qemu-nbd.pid) is obviously broken.

Use the local $TEST_DIR path before it is 'protocolized' for these
files.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 tests/qemu-iotests/common.config |  6 ++++--
 tests/qemu-iotests/common.qemu   | 10 +++++-----
 tests/qemu-iotests/common.rc     |  6 +++---
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index f6384fb..55527aa 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -109,7 +109,7 @@ _qemu_wrapper()
 {
     (
         if [ -n "${QEMU_NEED_PID}" ]; then
-            echo $BASHPID > "${TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
+            echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
         fi
         exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
     )
@@ -151,7 +151,7 @@ _qemu_io_wrapper()
 _qemu_nbd_wrapper()
 {
     (
-        echo $BASHPID > "${TEST_DIR}/qemu-nbd.pid"
+        echo $BASHPID > "${QEMU_TEST_DIR}/qemu-nbd.pid"
         exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@"
     )
 }
@@ -186,6 +186,8 @@ if [ -z "$TEST_DIR" ]; then
         TEST_DIR=`pwd`/scratch
 fi
 
+QEMU_TEST_DIR="${TEST_DIR}"
+
 if [ ! -e "$TEST_DIR" ]; then
         mkdir "$TEST_DIR"
 fi
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index e657361..4278789 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -27,8 +27,8 @@
 
 QEMU_COMM_TIMEOUT=10
 
-QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$"
-QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$"
+QEMU_FIFO_IN="${QEMU_TEST_DIR}/qmp-in-$$"
+QEMU_FIFO_OUT="${QEMU_TEST_DIR}/qmp-out-$$"
 
 QEMU_HANDLE=0
 
@@ -204,9 +204,9 @@ function _cleanup_qemu()
     for i in "${!QEMU_OUT[@]}"
     do
         local QEMU_PID
-        if [ -f "${TEST_DIR}/qemu-${i}.pid" ]; then
-            read QEMU_PID < "${TEST_DIR}/qemu-${i}.pid"
-            rm -f "${TEST_DIR}/qemu-${i}.pid"
+        if [ -f "${QEMU_TEST_DIR}/qemu-${i}.pid" ]; then
+            read QEMU_PID < "${QEMU_TEST_DIR}/qemu-${i}.pid"
+            rm -f "${QEMU_TEST_DIR}/qemu-${i}.pid"
             if [ -z "${wait}" ] && [ -n "${QEMU_PID}" ]; then
                 kill -KILL ${QEMU_PID} 2>/dev/null
             fi
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 3213765..4bb9b77 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -193,11 +193,11 @@ _cleanup_test_img()
     case "$IMGPROTO" in
 
         nbd)
-            if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then
+            if [ -f "${QEMU_TEST_DIR}/qemu-nbd.pid" ]; then
                 local QEMU_NBD_PID
-                read QEMU_NBD_PID < "${TEST_DIR}/qemu-nbd.pid"
+                read QEMU_NBD_PID < "${QEMU_TEST_DIR}/qemu-nbd.pid"
                 kill ${QEMU_NBD_PID}
-                rm -f "${TEST_DIR}/qemu-nbd.pid"
+                rm -f "${QEMU_TEST_DIR}/qemu-nbd.pid"
             fi
             rm -f "$TEST_IMG_FILE"
             ;;
-- 
2.9.3

[Qemu-devel] [PATCH v2 3/3] qemu-iotest: test to lookup protocol-based image with relative backing

[Jeff Cody]

This test uses NFS and block-stream to force a lookup of a backing
image that has a relative filename, but a full backing image name
with the protocol path intact.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 tests/qemu-iotests/173     | 97 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/173.out | 12 ++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 110 insertions(+)
 create mode 100755 tests/qemu-iotests/173
 create mode 100644 tests/qemu-iotests/173.out

diff --git a/tests/qemu-iotests/173 b/tests/qemu-iotests/173
new file mode 100755
index 0000000..bdaa092
--- /dev/null
+++ b/tests/qemu-iotests/173
@@ -0,0 +1,97 @@
+#!/bin/bash
+#
+# Test QAPI commands looking up protocol based images with relative
+# filename backing strings
+#
+# 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=jcody@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1    # failure is the default!
+
+_cleanup()
+{
+    _cleanup_qemu
+    rm  -f "${QEMU_TEST_DIR}/image.base" "${QEMU_TEST_DIR}/image.snp1"
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_proto nfs
+_supported_os Linux
+
+size=100M
+
+BASE_IMG="${TEST_DIR}/image.base"
+TOP_IMG="${TEST_DIR}/image.snp1"
+
+TEST_IMG="${BASE_IMG}" _make_test_img $size
+
+TEST_IMG="${TOP_IMG}" _make_test_img $size
+
+echo
+echo === Running QEMU, using block-stream to find backing image ===
+echo
+
+qemu_comm_method="qmp"
+_launch_qemu -drive file="${BASE_IMG}",if=virtio,id=disk2
+h=$QEMU_HANDLE
+
+_send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return"
+
+_send_qemu_cmd $h "{ 'arguments': {
+                        'device': 'disk2',
+                        'format': '${IMGFMT}',
+                        'mode': 'existing',
+                        'snapshot-file': '${TOP_IMG}',
+                        'snapshot-node-name': 'snp1'
+                     },
+                     'execute': 'blockdev-snapshot-sync'
+                   }" "return"
+
+
+_send_qemu_cmd $h "{ 'arguments': {
+                        'backing-file': 'image.base',
+                        'device': 'disk2',
+                        'image-node-name': 'snp1'
+                     },
+                     'execute': 'change-backing-file'
+                   }" "return"
+
+_send_qemu_cmd $h "{ 'arguments': {
+                        'base': '${BASE_IMG}',
+                        'device': 'disk2'
+                      },
+                      'execute': 'block-stream'
+                   }" "BLOCK_JOB_COMPLETED"
+
+_cleanup_qemu
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/173.out b/tests/qemu-iotests/173.out
new file mode 100644
index 0000000..f477a00
--- /dev/null
+++ b/tests/qemu-iotests/173.out
@@ -0,0 +1,12 @@
+QA output created by 173
+Formatting 'TEST_DIR/image.base', fmt=IMGFMT size=104857600
+Formatting 'TEST_DIR/image.snp1', fmt=IMGFMT size=104857600
+
+=== Running QEMU, using block-stream to find backing image ===
+
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk2", "len": 104857600, "offset": 104857600, "speed": 0, "type": "stream"}}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 866c1a0..5a93ba9 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -165,3 +165,4 @@
 170 rw auto quick
 171 rw auto quick
 172 auto
+173 rw auto
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames

[Max Reitz]

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

On 26.01.2017 02:08, Jeff Cody wrote:
> Differences from v1:
> 
> Patch 1: set local_error = NULL after freeing (Thanks Eric)
> Patch 2: new patch, groundwork for qemu-iotest in patch 3
> Patch 3: qemu-iotest, as requested (Thanks Max)
> 
> 
> Jeff Cody (3):
>   block: check full backing filename when searching protocol filenames
>   qemu-iotests: Don't create fifos / pidfiles with protocol paths
>   qemu-iotest: test to lookup protocol-based image with relative backing
> 
>  block.c                          | 13 ++++++
>  tests/qemu-iotests/173           | 97 ++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/173.out       | 12 +++++
>  tests/qemu-iotests/common.config |  6 ++-
>  tests/qemu-iotests/common.qemu   | 10 ++---
>  tests/qemu-iotests/common.rc     |  6 +--
>  tests/qemu-iotests/group         |  1 +
>  7 files changed, 135 insertions(+), 10 deletions(-)
>  create mode 100755 tests/qemu-iotests/173
>  create mode 100644 tests/qemu-iotests/173.out

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]