[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL v2 17/34] block/qdev: Allow node name for drive prope
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PULL v2 17/34] block/qdev: Allow node name for drive properties |
Date: |
Wed, 13 Jul 2016 14:50:24 +0200 |
If a node name instead of a BlockBackend name is specified as the driver
for a guest device, an anonymous BlockBackend is created now.
The order of operations in release_drive() must be reversed in order to
avoid a use-after-free bug because now blk_detach_dev() frees the last
reference if an anonymous BlockBackend is used.
usb-storage uses a hack where it forwards its BlockBackend as a property
to another device that it internally creates. This hack must be updated
so that it doesn't drop its original BB before it can be passed to the
other device. This used to work because we always had the monitor
reference around, but with node-names the device reference is the only
one now.
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
---
hw/core/qdev-properties-system.c | 39 +++++++++++++++++++++++++++++++++------
hw/usb/dev-storage.c | 5 ++++-
2 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 65d9fa9..ab1bc5e 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -72,12 +72,21 @@ static void parse_drive(DeviceState *dev, const char *str,
void **ptr,
const char *propname, Error **errp)
{
BlockBackend *blk;
+ bool blk_created = false;
blk = blk_by_name(str);
if (!blk) {
+ BlockDriverState *bs = bdrv_lookup_bs(NULL, str, NULL);
+ if (bs) {
+ blk = blk_new();
+ blk_insert_bs(blk, bs);
+ blk_created = true;
+ }
+ }
+ if (!blk) {
error_setg(errp, "Property '%s.%s' can't find value '%s'",
object_get_typename(OBJECT(dev)), propname, str);
- return;
+ goto fail;
}
if (blk_attach_dev(blk, dev) < 0) {
DriveInfo *dinfo = blk_legacy_dinfo(blk);
@@ -91,9 +100,16 @@ static void parse_drive(DeviceState *dev, const char *str,
void **ptr,
error_setg(errp, "Drive '%s' is already in use by another device",
str);
}
- return;
+ goto fail;
}
+
*ptr = blk;
+
+fail:
+ if (blk_created) {
+ /* If we need to keep a reference, blk_attach_dev() took it */
+ blk_unref(blk);
+ }
}
static void release_drive(Object *obj, const char *name, void *opaque)
@@ -103,8 +119,8 @@ static void release_drive(Object *obj, const char *name,
void *opaque)
BlockBackend **ptr = qdev_get_prop_ptr(dev, prop);
if (*ptr) {
- blk_detach_dev(*ptr, dev);
blockdev_auto_del(*ptr);
+ blk_detach_dev(*ptr, dev);
}
}
@@ -127,7 +143,7 @@ static void set_drive(Object *obj, Visitor *v, const char
*name, void *opaque,
PropertyInfo qdev_prop_drive = {
.name = "str",
- .description = "ID of a drive to use as a backend",
+ .description = "Node name or ID of a block device to use as a backend",
.get = get_drive,
.set = set_drive,
.release = release_drive,
@@ -362,8 +378,19 @@ PropertyInfo qdev_prop_vlan = {
void qdev_prop_set_drive(DeviceState *dev, const char *name,
BlockBackend *value, Error **errp)
{
- object_property_set_str(OBJECT(dev), value ? blk_name(value) : "",
- name, errp);
+ const char *ref = "";
+
+ if (value) {
+ ref = blk_name(value);
+ if (!*ref) {
+ BlockDriverState *bs = blk_bs(value);
+ if (bs) {
+ ref = bdrv_get_node_name(bs);
+ }
+ }
+ }
+
+ object_property_set_str(OBJECT(dev), ref, name, errp);
}
void qdev_prop_set_chr(DeviceState *dev, const char *name,
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 4d605b8..78038a2 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -609,10 +609,12 @@ static void usb_msd_realize_storage(USBDevice *dev, Error
**errp)
* a SCSI bus that can serve only a single device, which it
* creates automatically. But first it needs to detach from its
* blockdev, or else scsi_bus_legacy_add_drive() dies when it
- * attaches again.
+ * attaches again. We also need to take another reference so that
+ * blk_detach_dev() doesn't free blk while we still need it.
*
* The hack is probably a bad idea.
*/
+ blk_ref(blk);
blk_detach_dev(blk, &s->dev.qdev);
s->conf.blk = NULL;
@@ -623,6 +625,7 @@ static void usb_msd_realize_storage(USBDevice *dev, Error
**errp)
scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable,
s->conf.bootindex, dev->serial,
&err);
+ blk_unref(blk);
if (!scsi_dev) {
error_propagate(errp, err);
return;
--
1.8.3.1
- [Qemu-devel] [PULL v2 06/34] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror', (continued)
- [Qemu-devel] [PULL v2 06/34] mirror: Add 'job-id' parameter to 'blockdev-mirror' and 'drive-mirror', Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 07/34] backup: Add 'job-id' parameter to 'blockdev-backup' and 'drive-backup', Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 08/34] stream: Add 'job-id' parameter to 'block-stream', Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 10/34] qemu-img: Set the ID of the block job in img_commit(), Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 05/34] blockjob: Add 'job_id' parameter to block_job_create(), Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 13/34] raw-posix: Use qemu_dup, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 11/34] blockjob: Update description of the 'device' field in the QMP API, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 12/34] osdep: Introduce qemu_dup, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 15/34] test-coroutine: prepare for the next patch, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 09/34] commit: Add 'job-id' parameter to 'block-commit', Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 17/34] block/qdev: Allow node name for drive properties,
Kevin Wolf <=
- [Qemu-devel] [PULL v2 19/34] commit: Fix use of error handling policy, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 14/34] coroutine: use QSIMPLEQ instead of QTAILQ, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 18/34] block/qdev: Allow configuring WCE with qdev properties, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 20/34] block/qdev: Allow configuring rerror/werror with qdev properties, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 16/34] coroutine: move entry argument to qemu_coroutine_create, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 25/34] qemu-io: Use correct range limitations, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 26/34] qcow2: Fix qcow2_get_cluster_offset(), Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 24/34] qcow2: Avoid making the L1 table too big, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 28/34] vmdk: fix metadata write regression, Kevin Wolf, 2016/07/13
- [Qemu-devel] [PULL v2 22/34] block: Remove BB options from blockdev-add, Kevin Wolf, 2016/07/13