* [Qemu-devel] [PATCH v2 1/2] block: Do not prematurely remove "filename"
2014-06-26 19:09 [Qemu-devel] [PATCH v2 0/2] block: Fix unset "filename" for certain drivers Max Reitz
@ 2014-06-26 19:09 ` Max Reitz
2014-06-26 19:46 ` Eric Blake
2014-06-26 19:09 ` [Qemu-devel] [PATCH v2 2/2] iotests: Add test for set "filename" for NBD Max Reitz
1 sibling, 1 reply; 6+ messages in thread
From: Max Reitz @ 2014-06-26 19:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
If "filename" is removed from the options QDict before entering
bdrv_open_common(), it cannot be stored in the BDS. Therefore, wait
until it has been copied there and remove it from the options only
afterwards.
This fixes "filename" in the BDS being empty for block drivers which do
not need the filename because they have parsed it already (e.g. NBD).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index ff44e76..850c5d7 100644
--- a/block.c
+++ b/block.c
@@ -881,7 +881,8 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
* Removes all processed options from *options.
*/
static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
- QDict *options, int flags, BlockDriver *drv, Error **errp)
+ QDict *options, int flags, BlockDriver *drv, bool parsed_filename,
+ Error **errp)
{
int ret, open_flags;
const char *filename;
@@ -954,6 +955,14 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
bs->filename[0] = '\0';
}
+ if (parsed_filename && !drv->bdrv_needs_filename) {
+ qdict_del(options, "filename");
+ /* The pointer "filename" may reference the just deleted QDict entry; in
+ * any case, it is no longer needed, so indicate that by setting it to
+ * NULL. */
+ filename = NULL;
+ }
+
bs->drv = drv;
bs->opaque = g_malloc0(drv->instance_size);
@@ -1020,7 +1029,7 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
{
BlockDriver *drv;
const char *drvname;
- bool parse_filename = false;
+ bool parse_filename = false, parsed_filename = false;
Error *local_err = NULL;
int ret;
@@ -1070,18 +1079,19 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
goto fail;
}
- if (!drv->bdrv_needs_filename) {
- qdict_del(*options, "filename");
- } else {
+ if (drv->bdrv_needs_filename) {
filename = qdict_get_str(*options, "filename");
}
+
+ parsed_filename = true;
}
if (!drv->bdrv_file_open) {
ret = bdrv_open(&bs, filename, NULL, *options, flags, drv, &local_err);
*options = NULL;
} else {
- ret = bdrv_open_common(bs, NULL, *options, flags, drv, &local_err);
+ ret = bdrv_open_common(bs, NULL, *options, flags, drv, parsed_filename,
+ &local_err);
}
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1471,7 +1481,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
}
/* Open the image */
- ret = bdrv_open_common(bs, file, options, flags, drv, &local_err);
+ ret = bdrv_open_common(bs, file, options, flags, drv, false, &local_err);
if (ret < 0) {
goto fail;
}
--
2.0.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 2/2] iotests: Add test for set "filename" for NBD
2014-06-26 19:09 [Qemu-devel] [PATCH v2 0/2] block: Fix unset "filename" for certain drivers Max Reitz
2014-06-26 19:09 ` [Qemu-devel] [PATCH v2 1/2] block: Do not prematurely remove "filename" Max Reitz
@ 2014-06-26 19:09 ` Max Reitz
1 sibling, 0 replies; 6+ messages in thread
From: Max Reitz @ 2014-06-26 19:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Benoît Canet, Stefan Hajnoczi, Max Reitz
Add a new test for qemu-iotests which checks whether the "filename" (and
consequently the "file") attribute is set for images which are opened
over NBD.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/097 | 72 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/097.out | 13 +++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 86 insertions(+)
create mode 100755 tests/qemu-iotests/097
create mode 100644 tests/qemu-iotests/097.out
diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
new file mode 100755
index 0000000..c471ef2
--- /dev/null
+++ b/tests/qemu-iotests/097
@@ -0,0 +1,72 @@
+#!/bin/bash
+#
+# Test case for correct filename attribute for NBD
+#
+# 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
+
+_supported_fmt generic
+_supported_proto nbd
+_supported_os Linux
+
+function do_run_qemu()
+{
+ $QEMU -nographic -qmp stdio -serial none "$@"
+}
+
+function run_qemu()
+{
+ do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp | sed -e "s#$TEST_IMG#TEST_IMG#g"
+}
+
+IMG_SIZE=128K
+
+echo
+echo '=== Testing NBD filename ("filename" and "file" should be set to TEST_IMG) ==='
+echo
+
+_make_test_img $IMG_SIZE
+
+run_qemu -drive file=$TEST_IMG,format=raw,if=none <<QMP
+{ 'execute': 'qmp_capabilities' }
+{ 'execute': 'query-block' }
+{ 'execute': 'quit' }
+QMP
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
new file mode 100644
index 0000000..6607683
--- /dev/null
+++ b/tests/qemu-iotests/097.out
@@ -0,0 +1,13 @@
+QA output created by 097
+
+=== Testing NBD filename ("filename" and "file" should be set to TEST_IMG) ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
+QMP_VERSION
+{"return": {}}
+{"return": [{"device": "none0", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 131072, "filename": "TEST_IMG", "format": "raw"}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_IMG", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 0f07440..5447660 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -99,3 +99,4 @@
090 rw auto quick
091 rw auto
092 rw auto quick
+097 rw auto
--
2.0.0
^ permalink raw reply related [flat|nested] 6+ messages in thread