* [Qemu-devel] [PATCH v2 1/3] block: check full backing filename when searching protocol filenames
2017-01-26 1:08 [Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames Jeff Cody
@ 2017-01-26 1:08 ` Jeff Cody
2017-01-26 1:08 ` [Qemu-devel] [PATCH v2 2/3] qemu-iotests: Don't create fifos / pidfiles with protocol paths Jeff Cody
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Jeff Cody @ 2017-01-26 1:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, eblake
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH v2 2/3] qemu-iotests: Don't create fifos / pidfiles with protocol paths
2017-01-26 1:08 [Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames Jeff Cody
2017-01-26 1:08 ` [Qemu-devel] [PATCH v2 1/3] " Jeff Cody
@ 2017-01-26 1:08 ` Jeff Cody
2017-01-26 1:08 ` [Qemu-devel] [PATCH v2 3/3] qemu-iotest: test to lookup protocol-based image with relative backing Jeff Cody
2017-02-01 2:43 ` [Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames Max Reitz
3 siblings, 0 replies; 5+ messages in thread
From: Jeff Cody @ 2017-01-26 1:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, eblake
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH v2 3/3] qemu-iotest: test to lookup protocol-based image with relative backing
2017-01-26 1:08 [Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames Jeff Cody
2017-01-26 1:08 ` [Qemu-devel] [PATCH v2 1/3] " Jeff Cody
2017-01-26 1:08 ` [Qemu-devel] [PATCH v2 2/3] qemu-iotests: Don't create fifos / pidfiles with protocol paths Jeff Cody
@ 2017-01-26 1:08 ` Jeff Cody
2017-02-01 2:43 ` [Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames Max Reitz
3 siblings, 0 replies; 5+ messages in thread
From: Jeff Cody @ 2017-01-26 1:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, eblake
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames
2017-01-26 1:08 [Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames Jeff Cody
` (2 preceding siblings ...)
2017-01-26 1:08 ` [Qemu-devel] [PATCH v2 3/3] qemu-iotest: test to lookup protocol-based image with relative backing Jeff Cody
@ 2017-02-01 2:43 ` Max Reitz
3 siblings, 0 replies; 5+ messages in thread
From: Max Reitz @ 2017-02-01 2:43 UTC (permalink / raw)
To: Jeff Cody, qemu-devel; +Cc: qemu-block, kwolf, eblake
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread