From 14bafc540774baf316e9ce2474e97d5df6cb8e6c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 25 Jun 2010 08:09:10 +0200 Subject: blockdev: Clean up automatic drive deletion 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 Signed-off-by: Kevin Wolf --- 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 +- 5 files changed, 29 insertions(+), 7 deletions(-) (limited to 'hw') 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); } -- cgit v1.1