From ee13ed1cbc5f7f848e417f587c93ca1f36d83eb0 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Sun, 9 Feb 2014 09:52:32 +0100 Subject: 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(-) (limited to 'blockdev.c') 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; -- cgit 1.4.1 From 57b6bdf37c64985cf02b8737c550d52759059c9d Mon Sep 17 00:00:00 2001 From: Benoît Canet Date: Thu, 13 Feb 2014 17:22:33 +0100 Subject: 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(-) (limited to 'blockdev.c') 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) -- cgit 1.4.1 From 0c5e94ee8339e1aa49020466eba232e6f7c31a0a Mon Sep 17 00:00:00 2001 From: Benoît Canet Date: Wed, 12 Feb 2014 17:15:07 +0100 Subject: 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(-) (limited to 'blockdev.c') 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)); -- cgit 1.4.1