[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 08/23] blockdev: Clean up automatic drive deletion
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PATCH 08/23] blockdev: Clean up automatic drive deletion |
Date: |
Fri, 2 Jul 2010 18:38:17 +0200 |
From: Markus Armbruster <address@hidden>
We automatically delete blockdev host parts on unplug of the guest
device. Too much magic, but we can't change that now.
The delete happens early in the guest device teardown, before the
connection to the host part is severed. Thus, the guest part's
pointer to the host part dangles for a brief time. No actual harm
comes from this, but we'll catch such dangling pointers a few commits
down the road. Clean up the dangling pointers by delaying the
automatic deletion until the guest part's pointer is gone.
Device usb-storage deliberately makes two qdev properties refer to the
same drive, because it automatically creates a second device. Again,
too much magic we can't change now. Multiple references worked okay
before, but now free_drive() dies for the second one. Zap the extra
reference.
Signed-off-by: Markus Armbruster <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
---
blockdev.c | 23 +++++++++++++++++++++++
blockdev.h | 4 ++++
hw/qdev-properties.c | 10 ++++++++++
hw/scsi-disk.c | 2 +-
hw/scsi-generic.c | 2 +-
hw/usb-msd.c | 20 ++++++++++++++++----
hw/virtio-pci.c | 2 +-
7 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index ba4f66f..4848112 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -17,6 +17,29 @@
static QTAILQ_HEAD(drivelist, DriveInfo) drives =
QTAILQ_HEAD_INITIALIZER(drives);
+/*
+ * We automatically delete the drive when a device using it gets
+ * unplugged. Questionable feature, but we can't just drop it.
+ * Device models call blockdev_mark_auto_del() to schedule the
+ * automatic deletion, and generic qdev code calls blockdev_auto_del()
+ * when deletion is actually safe.
+ */
+void blockdev_mark_auto_del(BlockDriverState *bs)
+{
+ DriveInfo *dinfo = drive_get_by_blockdev(bs);
+
+ dinfo->auto_del = 1;
+}
+
+void blockdev_auto_del(BlockDriverState *bs)
+{
+ DriveInfo *dinfo = drive_get_by_blockdev(bs);
+
+ if (dinfo->auto_del) {
+ drive_uninit(dinfo);
+ }
+}
+
QemuOpts *drive_add(const char *file, const char *fmt, ...)
{
va_list ap;
diff --git a/blockdev.h b/blockdev.h
index 6ab592f..32e6979 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -13,6 +13,9 @@
#include "block.h"
#include "qemu-queue.h"
+void blockdev_mark_auto_del(BlockDriverState *bs);
+void blockdev_auto_del(BlockDriverState *bs);
+
typedef enum {
IF_NONE,
IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
@@ -28,6 +31,7 @@ typedef struct DriveInfo {
BlockInterfaceType type;
int bus;
int unit;
+ int auto_del; /* see blockdev_mark_auto_del() */
QemuOpts *opts;
char serial[BLOCK_SERIAL_STRLEN + 1];
QTAILQ_ENTRY(DriveInfo) next;
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 5b7fd77..d7dc234 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -313,6 +313,15 @@ static int parse_drive(DeviceState *dev, Property *prop,
const char *str)
return 0;
}
+static void free_drive(DeviceState *dev, Property *prop)
+{
+ DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
+
+ if (*ptr) {
+ blockdev_auto_del((*ptr)->bdrv);
+ }
+}
+
static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t
len)
{
DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
@@ -325,6 +334,7 @@ PropertyInfo qdev_prop_drive = {
.size = sizeof(DriveInfo*),
.parse = parse_drive,
.print = print_drive,
+ .free = free_drive,
};
/* --- character device --- */
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 2b38984..d76e640 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1043,7 +1043,7 @@ static void scsi_destroy(SCSIDevice *dev)
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
scsi_disk_purge_requests(s);
- drive_uninit(s->qdev.conf.dinfo);
+ blockdev_mark_auto_del(s->qdev.conf.dinfo->bdrv);
}
static int scsi_disk_initfn(SCSIDevice *dev)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e31060e..1859c94 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -453,7 +453,7 @@ static void scsi_destroy(SCSIDevice *d)
r = DO_UPCAST(SCSIGenericReq, req, QTAILQ_FIRST(&s->qdev.requests));
scsi_remove_request(r);
}
- drive_uninit(s->qdev.conf.dinfo);
+ blockdev_mark_auto_del(s->qdev.conf.dinfo->bdrv);
}
static int scsi_generic_initfn(SCSIDevice *dev)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 8e9718c..3dbfcab 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -522,24 +522,36 @@ static void usb_msd_password_cb(void *opaque, int err)
static int usb_msd_initfn(USBDevice *dev)
{
MSDState *s = DO_UPCAST(MSDState, dev, dev);
+ DriveInfo *dinfo = s->conf.dinfo;
- if (!s->conf.dinfo || !s->conf.dinfo->bdrv) {
+ if (!dinfo || !dinfo->bdrv) {
error_report("usb-msd: drive property not set");
return -1;
}
+ /*
+ * Hack alert: this pretends to be a block device, but it's really
+ * a SCSI bus that can serve only a single device, which it
+ * creates automatically. Two drive properties pointing to the
+ * same drive is not good: free_drive() dies for the second one.
+ * Zap the one we're not going to use.
+ *
+ * The hack is probably a bad idea.
+ */
+ s->conf.dinfo = NULL;
+
s->dev.speed = USB_SPEED_FULL;
scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
- s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, s->conf.dinfo, 0);
+ s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, dinfo, 0);
if (!s->scsi_dev) {
return -1;
}
s->bus.qbus.allow_hotplug = 0;
usb_msd_handle_reset(dev);
- if (bdrv_key_required(s->conf.dinfo->bdrv)) {
+ if (bdrv_key_required(dinfo->bdrv)) {
if (cur_mon) {
- monitor_read_bdrv_key_start(cur_mon, s->conf.dinfo->bdrv,
+ monitor_read_bdrv_key_start(cur_mon, dinfo->bdrv,
usb_msd_password_cb, s);
s->dev.auto_attach = 0;
} else {
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index d1303b1..31a68fe 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -571,7 +571,7 @@ static int virtio_blk_exit_pci(PCIDevice *pci_dev)
{
VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
- drive_uninit(proxy->block.dinfo);
+ blockdev_mark_auto_del(proxy->block.dinfo->bdrv);
return virtio_exit_pci(pci_dev);
}
--
1.6.6.1
- [Qemu-devel] [PATCH 10/23] blockdev: drive_get_by_id() is no longer used, remove, (continued)
- [Qemu-devel] [PATCH 10/23] blockdev: drive_get_by_id() is no longer used, remove, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 11/23] block: Catch attempt to attach multiple devices to a blockdev, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 19/23] ide: Make PIIX and ISA IDE init functions return the qdev, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 20/23] pc: Fix CMOS info for drives defined with -device, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 22/23] block: Fix early failure in multiwrite, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 23/23] block: Handle multiwrite errors only when all requests have completed, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 21/23] qemu-img: avoid calling exit(1) to release resources properly, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 15/23] blkdebug: Initialize state as 1, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 09/23] qdev: Decouple qdev_prop_drive from DriveInfo, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 07/23] blockdev: New drive_get_by_blockdev(), Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 08/23] blockdev: Clean up automatic drive deletion,
Kevin Wolf <=
- [Qemu-devel] [PATCH 17/23] block: Clean up bdrv_snapshots(), Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 18/23] block: Fix virtual media change for if=none, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 16/23] savevm: Survive hot-unplug of snapshot device, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 04/23] ide: Make it explicit that ide_create_drive() can't fail, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 12/23] qemu-option: New qemu_opts_reset(), Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 14/23] blkdebug: Free QemuOpts after having read the config, Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 05/23] blockdev: Remove drive_get_serial(), Kevin Wolf, 2010/07/02
- [Qemu-devel] [PATCH 13/23] blkdebug: Fix set_state_opts definition, Kevin Wolf, 2010/07/02