From 1a838745b80ec6bcb3f78ce38aa1ccf1efc9f937 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Sun, 9 Feb 2014 10:02:27 +0100 Subject: [PATCH 01/11] sdhci: Drop unnecessary #include Signed-off-by: Kevin Wolf Reviewed-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- hw/sd/sdhci.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 0906a1d62b..a0b90baf6c 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -26,7 +26,6 @@ #include "sysemu/blockdev.h" #include "sysemu/dma.h" #include "qemu/timer.h" -#include "block/block_int.h" #include "qemu/bitops.h" #include "sdhci.h" From ee13ed1cbc5f7f848e417f587c93ca1f36d83eb0 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Sun, 9 Feb 2014 09:52:32 +0100 Subject: [PATCH 02/11] blockdev: Remove 'type' parameter from blockdev_init() blockdev-add doesn't know about the device that the backend will be attached to, this is a legacy -drive concept. Move the remaining checks that use it to drive_init(). [Fam Zheng suggested line-wrapping to 80 chars as required by the coding standard. I have fixed this. --Stefan] Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- blockdev.c | 47 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/blockdev.c b/blockdev.c index 36ceece9ff..d5f21f07b7 100644 --- a/blockdev.c +++ b/blockdev.c @@ -308,7 +308,6 @@ typedef enum { MEDIA_DISK, MEDIA_CDROM } DriveMediaType; /* Takes the ownership of bs_opts */ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, - BlockInterfaceType type, Error **errp) { const char *buf; @@ -437,11 +436,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, on_write_error = BLOCKDEV_ON_ERROR_ENOSPC; if ((buf = qemu_opt_get(opts, "werror")) != NULL) { - if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) { - error_setg(errp, "werror is not supported by this bus type"); - goto early_err; - } - on_write_error = parse_block_error_action(buf, 0, &error); if (error_is_set(&error)) { error_propagate(errp, error); @@ -451,11 +445,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, on_read_error = BLOCKDEV_ON_ERROR_REPORT; if ((buf = qemu_opt_get(opts, "rerror")) != NULL) { - if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI && type != IF_NONE) { - error_report("rerror is not supported by this bus type"); - goto early_err; - } - on_read_error = parse_block_error_action(buf, 1, &error); if (error_is_set(&error)) { error_propagate(errp, error); @@ -469,7 +458,6 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, dinfo->bdrv = bdrv_new(dinfo->id); dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0; dinfo->bdrv->read_only = ro; - dinfo->type = type; dinfo->refcount = 1; if (serial != NULL) { dinfo->serial = g_strdup(serial); @@ -608,6 +596,14 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "read-only", .type = QEMU_OPT_BOOL, .help = "open drive file as read-only", + },{ + .name = "rerror", + .type = QEMU_OPT_STRING, + .help = "read error action", + },{ + .name = "werror", + .type = QEMU_OPT_STRING, + .help = "write error action", },{ .name = "copy-on-read", .type = QEMU_OPT_BOOL, @@ -629,6 +625,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) int cyls, heads, secs, translation; int max_devs, bus_id, unit_id, index; const char *devaddr; + const char *werror, *rerror; bool read_only = false; bool copy_on_read; const char *filename; @@ -872,8 +869,29 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) filename = qemu_opt_get(legacy_opts, "file"); + /* Check werror/rerror compatibility with if=... */ + werror = qemu_opt_get(legacy_opts, "werror"); + if (werror != NULL) { + if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && + type != IF_NONE) { + error_report("werror is not supported by this bus type"); + goto fail; + } + qdict_put(bs_opts, "werror", qstring_from_str(werror)); + } + + rerror = qemu_opt_get(legacy_opts, "rerror"); + if (rerror != NULL) { + if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI && + type != IF_NONE) { + error_report("rerror is not supported by this bus type"); + goto fail; + } + qdict_put(bs_opts, "rerror", qstring_from_str(rerror)); + } + /* Actual block device init: Functionality shared with blockdev-add */ - dinfo = blockdev_init(filename, bs_opts, type, &local_err); + dinfo = blockdev_init(filename, bs_opts, &local_err); if (dinfo == NULL) { if (error_is_set(&local_err)) { qerror_report_err(local_err); @@ -893,6 +911,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->secs = secs; dinfo->trans = translation; + dinfo->type = type; dinfo->bus = bus_id; dinfo->unit = unit_id; dinfo->devaddr = devaddr; @@ -2276,7 +2295,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) qdict_flatten(qdict); - blockdev_init(NULL, qdict, IF_NONE, &local_err); + blockdev_init(NULL, qdict, &local_err); if (error_is_set(&local_err)) { error_propagate(errp, local_err); goto fail; From 28f106afb35a86aa01e1907ef7632e015fabce02 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Tue, 4 Feb 2014 14:12:44 -0500 Subject: [PATCH 03/11] block: Add notes to iSCSI's .bdrv_open and .bdrv_reopen_prepare iSCSI currently does not need to do any actions to support the current usage of bdrv_reopen(). However, it is important to note a couple of things: 1.) A connection will not be re-established to an iSCSI target, and 2.) If iscsi_open() is changed to parse 'flags', then iscsi_reopen_prepare() may need to be more than a stub. In light of the above, this commit adds comments above both of the functions to bring attention to these facts. Signed-off-by: Jeff Cody Signed-off-by: Stefan Hajnoczi --- block/iscsi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 8d0f9667c5..c97c04060d 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1099,6 +1099,10 @@ fail: /* * We support iscsi url's on the form * iscsi://[%@][:]// + * + * Note: flags are currently not used by iscsi_open. If this function + * is changed such that flags are used, please examine iscsi_reopen_prepare() + * to see if needs to be changed as well. */ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) @@ -1336,11 +1340,13 @@ static int iscsi_refresh_limits(BlockDriverState *bs) return 0; } -/* We have nothing to do for iSCSI reopen, stub just returns - * success */ +/* Since iscsi_open() ignores bdrv_flags, there is nothing to do here in + * prepare. Note that this will not re-establish a connection with an iSCSI + * target - it is effectively a NOP. */ static int iscsi_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue, Error **errp) { + /* NOP */ return 0; } From 39a611a3e035e148257af314a522a6cd169c2d0e Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Wed, 12 Feb 2014 14:46:24 -0500 Subject: [PATCH 04/11] block: Don't throw away errno via error_setg There are a handful of places in the block layer where a failure path has a valid -errno value, yet error_setg() is used. Those instances should instead use error_setg_errno(), to preserve as much error information as possible. This patch replaces those instances with error_setg_errno(), so that errno is passed up the stack in the error message. Reported-By: Kevin Wolf Signed-off-by: Jeff Cody Reviewed-by: Eric Blake Reviewed-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- block/mirror.c | 13 +++++++++---- block/qcow2-snapshot.c | 8 +++++--- block/vmdk.c | 6 +++--- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 2a4333474e..26d18e44d7 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -633,6 +633,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, { int64_t length, base_length; int orig_base_flags; + int ret; orig_base_flags = bdrv_get_flags(base); @@ -642,19 +643,23 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, length = bdrv_getlength(bs); if (length < 0) { - error_setg(errp, "Unable to determine length of %s", bs->filename); + error_setg_errno(errp, -length, + "Unable to determine length of %s", bs->filename); goto error_restore_flags; } base_length = bdrv_getlength(base); if (base_length < 0) { - error_setg(errp, "Unable to determine length of %s", base->filename); + error_setg_errno(errp, -base_length, + "Unable to determine length of %s", base->filename); goto error_restore_flags; } if (length > base_length) { - if (bdrv_truncate(base, length) < 0) { - error_setg(errp, "Top image %s is larger than base image %s, and " + ret = bdrv_truncate(base, length); + if (ret < 0) { + error_setg_errno(errp, -ret, + "Top image %s is larger than base image %s, and " "resize of base image failed", bs->filename, base->filename); goto error_restore_flags; diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index ad8bf3dcd9..2fc6320aa1 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -606,7 +606,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs, s->nb_snapshots--; ret = qcow2_write_snapshots(bs); if (ret < 0) { - error_setg(errp, "Failed to remove snapshot from snapshot list"); + error_setg_errno(errp, -ret, + "Failed to remove snapshot from snapshot list"); return ret; } @@ -624,7 +625,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, ret = qcow2_update_snapshot_refcount(bs, sn.l1_table_offset, sn.l1_size, -1); if (ret < 0) { - error_setg(errp, "Failed to free the cluster and L1 table"); + error_setg_errno(errp, -ret, "Failed to free the cluster and L1 table"); return ret; } qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t), @@ -633,7 +634,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs, /* must update the copied flag on the current cluster offsets */ ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0); if (ret < 0) { - error_setg(errp, "Failed to update snapshot status in disk"); + error_setg_errno(errp, -ret, + "Failed to update snapshot status in disk"); return ret; } diff --git a/block/vmdk.c b/block/vmdk.c index e809e2ef46..ff6f5ee911 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1502,7 +1502,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, if (flat) { ret = bdrv_truncate(bs, filesize); if (ret < 0) { - error_setg(errp, "Could not truncate file"); + error_setg_errno(errp, -ret, "Could not truncate file"); } goto exit; } @@ -1562,7 +1562,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, ret = bdrv_truncate(bs, le64_to_cpu(header.grain_offset) << 9); if (ret < 0) { - error_setg(errp, "Could not truncate file"); + error_setg_errno(errp, -ret, "Could not truncate file"); goto exit; } @@ -1846,7 +1846,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, if (desc_offset == 0) { ret = bdrv_truncate(new_bs, desc_len); if (ret < 0) { - error_setg(errp, "Could not truncate file"); + error_setg_errno(errp, -ret, "Could not truncate file"); } } exit: From e001807847ba40d29450031377b84acd10066b61 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Wed, 12 Feb 2014 16:30:52 -0500 Subject: [PATCH 05/11] block: qemu-iotests - fix test 070 (vhdx) VHDX test 070 failed, due to different output from qemu-io / qemu when opening an image read-only that contains a log file. Filter the output, and update the expected results to match the correct output. Signed-off-by: Jeff Cody Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests/070 | 3 ++- tests/qemu-iotests/070.out | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070 index 41bf100701..f84d2cb4ed 100755 --- a/tests/qemu-iotests/070 +++ b/tests/qemu-iotests/070 @@ -56,7 +56,8 @@ _use_sample_img iotest-dirtylog-10G-4M.vhdx.bz2 echo echo "=== Verify open image read-only fails, due to dirty log ===" -$QEMU_IO -r -c "read -pP 0xa5 0 18M" "$TEST_IMG" 2>&1 | grep -o "Permission denied" +$QEMU_IO -r -c "read -pP 0xa5 0 18M" "$TEST_IMG" 2>&1 | _filter_testdir \ + | _filter_qemu_io echo "=== Verify open image replays log ===" $QEMU_IO -c "read -pP 0xa5 0 18M" "$TEST_IMG" | _filter_qemu_io diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out index 9db8ff2650..0352adced4 100644 --- a/tests/qemu-iotests/070.out +++ b/tests/qemu-iotests/070.out @@ -1,7 +1,9 @@ QA output created by 070 === Verify open image read-only fails, due to dirty log === -Permission denied +qemu-io: can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed. To replay the log, execute: + qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx': Operation not permitted + no file open, try 'help open' === Verify open image replays log === read 18874368/18874368 bytes at offset 0 18 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) From 18968ca1a33458f3978394499d2b70e4c32c5ad6 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Wed, 12 Feb 2014 16:30:53 -0500 Subject: [PATCH 06/11] block: qemu-iotests - add vhdx log replay tests for qemu-img VHDX logs can now be replayed via 'qemu-img check -r all'. Add tests to verify that the log replay is successful when using qemu-img. Signed-off-by: Jeff Cody Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests/070 | 10 ++++++++++ tests/qemu-iotests/070.out | 11 +++++++++++ 2 files changed, 21 insertions(+) diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070 index f84d2cb4ed..ce71fa4a22 100755 --- a/tests/qemu-iotests/070 +++ b/tests/qemu-iotests/070 @@ -62,6 +62,16 @@ $QEMU_IO -r -c "read -pP 0xa5 0 18M" "$TEST_IMG" 2>&1 | _filter_testdir \ echo "=== Verify open image replays log ===" $QEMU_IO -c "read -pP 0xa5 0 18M" "$TEST_IMG" | _filter_qemu_io +# extract fresh sample image again +_use_sample_img iotest-dirtylog-10G-4M.vhdx.bz2 + +echo "=== Verify qemu-img check -r all replays log ===" +$QEMU_IMG check -r all "$TEST_IMG" 2>&1 | _filter_testdir | _filter_qemu + +echo "=== Verify open image read-only succeeds after log replay ===" +$QEMU_IO -r -c "read -pP 0xa5 0 18M" "$TEST_IMG" 2>&1 | _filter_testdir \ + | _filter_qemu_io + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out index 0352adced4..922d62cb51 100644 --- a/tests/qemu-iotests/070.out +++ b/tests/qemu-iotests/070.out @@ -7,4 +7,15 @@ qemu-io: can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file === Verify open image replays log === read 18874368/18874368 bytes at offset 0 18 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +=== Verify qemu-img check -r all replays log === +The following inconsistencies were found and repaired: + + 0 leaked clusters + 1 corruptions + +Double checking the fixed image now... +No errors were found on the image. +=== Verify open image read-only succeeds after log replay === +read 18874368/18874368 bytes at offset 0 +18 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) *** done From 2aa4a86f59c7093e03d2ec18c5d5f08c957d1a78 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 13 Feb 2014 10:05:31 +0800 Subject: [PATCH 07/11] qemu-iotests: Don't run 005 on vmdk split formats There would be too many extents that VMDK driver can't open all of them: 005 0s ... - output mismatch (see 005.out.bad) --- 005.out 2013-12-24 09:27:27.608181030 +0800 +++ 005.out.bad 2014-02-13 10:00:15.282184557 +0800 @@ -4,10 +4,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=5368709120000 small read -read 4096/4096 bytes at offset 1024 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +qemu-io: can't open device /tmp/qemu-iotests/t.vmdk: Could not open '/tmp/qemu-iotests/t-s1016.vmdk': Too many open files +no file open, try 'help open' small write -wrote 4096/4096 bytes at offset 8192 -4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +qemu-io: can't open device /tmp/qemu-iotests/t.vmdk: Could not open '/tmp/qemu-iotests/t-s1016.vmdk': Too many open files +no file open, try 'help open' *** done So disable the two subformats. Signed-off-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests/005 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005 index 9abcb84e4b..ba1236dfbf 100755 --- a/tests/qemu-iotests/005 +++ b/tests/qemu-iotests/005 @@ -44,6 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt generic _supported_proto generic _supported_os Linux +_unsupported_imgopts "subformat=twoGbMaxExtentFlat" \ + "subformat=twoGbMaxExtentSparse" # vpc is limited to 127GB, so we can't test it here if [ "$IMGFMT" = "vpc" ]; then From cc67f4d1f9645e8e6d90aee84ca19162d661f082 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Thu, 13 Feb 2014 09:23:38 -0500 Subject: [PATCH 08/11] block: mirror - use local_err to avoid NULL errp When starting a block job, commit_active_start() relies on whether *errp is set by mirror_start_job. This allows it to determine if the mirror job start failed, so that it can clean up any changes to open flags from the bdrv_reopen(). If errp is NULL, then it will not be able to determine if mirror_start_job failed or not. To avoid this, use a local Error variable, and then propagate the error (if any) to errp. Reported-by: Markus Armbruster Signed-off-by: Jeff Cody Reviewed-by: Kevin Wolf Reviewed-by: Eric Blake Signed-off-by: Stefan Hajnoczi --- block/mirror.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 26d18e44d7..e683959570 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -634,6 +634,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, int64_t length, base_length; int orig_base_flags; int ret; + Error *local_err = NULL; orig_base_flags = bdrv_get_flags(base); @@ -668,9 +669,10 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base, bdrv_ref(base); mirror_start_job(bs, base, speed, 0, 0, - on_error, on_error, cb, opaque, errp, + on_error, on_error, cb, opaque, &local_err, &commit_active_job_driver, false, base); - if (error_is_set(errp)) { + if (error_is_set(&local_err)) { + error_propagate(errp, local_err); goto error_restore_flags; } From 57b6bdf37c64985cf02b8737c550d52759059c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Thu, 13 Feb 2014 17:22:33 +0100 Subject: [PATCH 09/11] blockdev: Fix wrong usage of QDECREF causing snapshoted quorum to crash on close. As bdrv_open() documentation states: "The reference to the QDict belongs to the block layer * after the call (even on failure), so if the caller intends to reuse the * dictionary, it needs to use QINCREF() before calling bdrv_open." the optional options dict will not be reused after bdrv_open() and should belong to the block layer so remove the extra QDECREF(options). Signed-off-by: Benoit Canet Reviewed-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- blockdev.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index d5f21f07b7..ccd6a72e92 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1329,8 +1329,6 @@ static void external_snapshot_prepare(BlkTransactionState *common, if (ret != 0) { error_propagate(errp, local_err); } - - QDECREF(options); } static void external_snapshot_commit(BlkTransactionState *common) From dd67fa5052fecf661369540d5f104720f57900a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Wed, 12 Feb 2014 17:15:06 +0100 Subject: [PATCH 10/11] block: Relax bdrv_lookup_bs constraints. The following patch will reuse bdrv_lookup_bs in order to open images by references so the rules of usage of bdrv_lookup_bs must be relaxed a bit. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Stefan Hajnoczi --- block.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/block.c b/block.c index 636aa117c3..ba2aecf87a 100644 --- a/block.c +++ b/block.c @@ -3574,30 +3574,26 @@ BlockDriverState *bdrv_lookup_bs(const char *device, { BlockDriverState *bs = NULL; - if ((!device && !node_name) || (device && node_name)) { - error_setg(errp, "Use either device or node-name but not both"); - return NULL; - } - if (device) { bs = bdrv_find(device); - if (!bs) { - error_set(errp, QERR_DEVICE_NOT_FOUND, device); - return NULL; + if (bs) { + return bs; } - - return bs; } - bs = bdrv_find_node(node_name); + if (node_name) { + bs = bdrv_find_node(node_name); - if (!bs) { - error_set(errp, QERR_DEVICE_NOT_FOUND, node_name); - return NULL; + if (bs) { + return bs; + } } - return bs; + error_setg(errp, "Cannot find device=%s nor node_name=%s", + device ? device : "", + node_name ? node_name : ""); + return NULL; } BlockDriverState *bdrv_next(BlockDriverState *bs) From 0c5e94ee8339e1aa49020466eba232e6f7c31a0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Canet?= Date: Wed, 12 Feb 2014 17:15:07 +0100 Subject: [PATCH 11/11] block: Open by reference will try device then node_name. Since we introduced node_name for named bs of the graph modify the opening by reference to use it as a fallback. This patch also enforce the separation of the device id and graph node namespaces. Signed-off-by: Benoit Canet Reviewed-by: Max Reitz Signed-off-by: Stefan Hajnoczi --- block.c | 10 ++++++++-- blockdev.c | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index ba2aecf87a..8f718f9f29 100644 --- a/block.c +++ b/block.c @@ -796,6 +796,13 @@ static int bdrv_assign_node_name(BlockDriverState *bs, return -EINVAL; } + /* takes care of avoiding namespaces collisions */ + if (bdrv_find(node_name)) { + error_setg(errp, "node-name=%s is conflicting with a device id", + node_name); + return -EINVAL; + } + /* takes care of avoiding duplicates node names */ if (bdrv_find_node(node_name)) { error_setg(errp, "Duplicate node name"); @@ -977,9 +984,8 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, } QDECREF(options); - bs = bdrv_find(reference); + bs = bdrv_lookup_bs(reference, reference, errp); if (!bs) { - error_setg(errp, "Cannot find block device '%s'", reference); return -ENODEV; } bdrv_ref(bs); diff --git a/blockdev.c b/blockdev.c index ccd6a72e92..dfb5ec7529 100644 --- a/blockdev.c +++ b/blockdev.c @@ -452,6 +452,12 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, } } + if (bdrv_find_node(qemu_opts_id(opts))) { + error_setg(errp, "device id=%s is conflicting with a node-name", + qemu_opts_id(opts)); + goto early_err; + } + /* init */ dinfo = g_malloc0(sizeof(*dinfo)); dinfo->id = g_strdup(qemu_opts_id(opts));