[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 1/3] block: detach devices from DriveInfo at unreali
From: |
Paolo Bonzini |
Subject: |
[Qemu-devel] [PATCH 1/3] block: detach devices from DriveInfo at unrealize time |
Date: |
Mon, 22 Feb 2016 15:39:03 +0100 |
Instead of delaying blk_detach_dev and blockdev_auto_del until
the object is finalized and properties are released, do that
as soon as possible.
This patch replaces blockdev_mark_auto_del calls with blk_detach_dev
and blockdev_del_drive (the latter is a combination of the former
blockdev_mark_auto_del and blockdev_auto_del).
release_drive's call to blockdev_auto_del can then be removed completely.
This is of course okay in the case where the device has been unrealized
before and unrealize took care of calling blockdev_del_drive. However,
it is also okay if the device has failed to be realized. In that case,
blockdev_mark_auto_del was never called (because it is called during
unrealize) and thus release_drive's blockdev_auto_del call did nothing.
The drive-del-test qtest covers this case.
Signed-off-by: Paolo Bonzini <address@hidden>
---
blockdev.c | 21 +++++----------------
hw/block/virtio-blk.c | 4 +++-
hw/block/xen_disk.c | 1 +
hw/core/qdev-properties-system.c | 8 ++++++--
hw/ide/piix.c | 3 +++
hw/scsi/scsi-bus.c | 4 +++-
hw/usb/dev-storage.c | 3 ++-
include/sysemu/blockdev.h | 4 +---
8 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 1f73478..2dfb2d8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -114,20 +114,16 @@ void override_max_devs(BlockInterfaceType type, int
max_devs)
/*
* 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.
+ * Device models call blockdev_del_drive() to schedule the
+ * automatic deletion, and generic block layer code uses the
+ * refcount to do the deletion when it is actually safe.
*/
-void blockdev_mark_auto_del(BlockBackend *blk)
+void blockdev_del_drive(BlockBackend *blk)
{
DriveInfo *dinfo = blk_legacy_dinfo(blk);
BlockDriverState *bs = blk_bs(blk);
AioContext *aio_context;
- if (!dinfo) {
- return;
- }
-
if (bs) {
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -139,14 +135,7 @@ void blockdev_mark_auto_del(BlockBackend *blk)
aio_context_release(aio_context);
}
- dinfo->auto_del = 1;
-}
-
-void blockdev_auto_del(BlockBackend *blk)
-{
- DriveInfo *dinfo = blk_legacy_dinfo(blk);
-
- if (dinfo && dinfo->auto_del) {
+ if (dinfo) {
blk_unref(blk);
}
}
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c427698..0582787 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -945,7 +945,9 @@ static void virtio_blk_device_unrealize(DeviceState *dev,
Error **errp)
s->dataplane = NULL;
qemu_del_vm_change_state_handler(s->change);
unregister_savevm(dev, "virtio-blk", s);
- blockdev_mark_auto_del(s->blk);
+ blk_detach_dev(s->blk, dev);
+ blockdev_del_drive(s->blk);
+ s->blk = NULL;
virtio_cleanup(vdev);
}
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 7bd5bde..39a72e4 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -1041,6 +1041,7 @@ static void blk_disconnect(struct XenDevice *xendev)
if (blkdev->blk) {
blk_detach_dev(blkdev->blk, blkdev);
+ blockdev_del_drive(blkdev->blk);
blk_unref(blkdev->blk);
blkdev->blk = NULL;
}
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index e10cede..469ba8a 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -101,9 +101,13 @@ static void release_drive(Object *obj, const char *name,
void *opaque)
Property *prop = opaque;
BlockBackend **ptr = qdev_get_prop_ptr(dev, prop);
- if (*ptr) {
+ if (*ptr && blk_get_attached_dev(*ptr) != NULL) {
+ /* Unrealize has already called blk_detach_dev and blockdev_del_drive
+ * if the device has been realized; in that case, blk_get_attached_dev
+ * returns NULL. Thus, we get here if the device failed to realize,
+ * and the -drive must not be released.
+ */
blk_detach_dev(*ptr, dev);
- blockdev_auto_del(*ptr);
}
}
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index df46147..cf8fa58 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -182,6 +182,9 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev)
if (ds) {
blk_detach_dev(blk, ds);
}
+ if (pci_ide->bus[di->bus].ifs[di->unit].blk) {
+ blockdev_del_drive(blk);
+ }
pci_ide->bus[di->bus].ifs[di->unit].blk = NULL;
if (!(i % 2)) {
idedev = pci_ide->bus[di->bus].master;
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 6dcdbc0..3b2b766 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -214,7 +214,9 @@ static void scsi_qdev_unrealize(DeviceState *qdev, Error
**errp)
}
scsi_device_purge_requests(dev, SENSE_CODE(NO_SENSE));
- blockdev_mark_auto_del(dev->conf.blk);
+ blk_detach_dev(dev->conf.blk, qdev);
+ blockdev_del_drive(dev->conf.blk);
+ dev->conf.blk = NULL;
}
/* handle legacy '-drive if=scsi,...' cmd line args */
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 5ae0424..1c00211 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -643,7 +643,8 @@ static void usb_msd_realize_storage(USBDevice *dev, Error
**errp)
* blockdev, or else scsi_bus_legacy_add_drive() dies when it
* attaches again.
*
- * The hack is probably a bad idea.
+ * The hack is probably a bad idea. Anyway, this is why this does not
+ * call blockdev_del_drive.
*/
blk_detach_dev(blk, &s->dev.qdev);
s->conf.blk = NULL;
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index b06a060..ae7ad67 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -14,8 +14,7 @@
#include "qapi/error.h"
#include "qemu/queue.h"
-void blockdev_mark_auto_del(BlockBackend *blk);
-void blockdev_auto_del(BlockBackend *blk);
+void blockdev_del_drive(BlockBackend *blk);
typedef enum {
IF_DEFAULT = -1, /* for use with drive_add() only */
@@ -34,7 +33,6 @@ struct DriveInfo {
BlockInterfaceType type;
int bus;
int unit;
- int auto_del; /* see blockdev_mark_auto_del() */
bool is_default; /* Added by default_drive() ? */
int media_cd;
int cyls, heads, secs, trans;
--
2.5.0