* [Qemu-devel] [PATCH 1/2] block: detach devices from DriveInfo at unrealize time
2016-01-08 17:37 [Qemu-devel] [RFC PATCH 0/2] Early release of -drive QemuOpts Paolo Bonzini
@ 2016-01-08 17:37 ` Paolo Bonzini
2016-01-08 17:37 ` [Qemu-devel] [PATCH 2/2] block: remove legacy_dinfo at blk_detach_dev time Paolo Bonzini
2016-01-22 14:01 ` [Qemu-devel] [RFC PATCH 0/2] Early release of -drive QemuOpts Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2016-01-08 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-block, Max Reitz
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). We cannot make
blk_detach_dev do both tasks because of the USB mass storage hack.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
blockdev.c | 21 +++++----------------
hw/block/virtio-blk.c | 4 +++-
hw/block/xen_disk.c | 1 +
hw/core/qdev-properties-system.c | 2 +-
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, 19 insertions(+), 23 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 917ae06..a8309fb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -110,20 +110,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);
@@ -135,14 +131,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 848f3fe..6e51246 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -960,7 +960,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 02eda6e..a6a0b7c 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -1054,6 +1054,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 921e799..cf147f4 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -102,7 +102,7 @@ static void release_drive(Object *obj, const char *name, void *opaque)
if (*ptr) {
blk_detach_dev(*ptr, dev);
- blockdev_auto_del(*ptr);
+ blockdev_del_drive(*ptr);
}
}
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5a26c86..2b2d043 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -181,6 +181,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 fd1171e..a805fad 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -213,7 +213,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 597d8fd..f191011 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -642,7 +642,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH 2/2] block: remove legacy_dinfo at blk_detach_dev time
2016-01-08 17:37 [Qemu-devel] [RFC PATCH 0/2] Early release of -drive QemuOpts Paolo Bonzini
2016-01-08 17:37 ` [Qemu-devel] [PATCH 1/2] block: detach devices from DriveInfo at unrealize time Paolo Bonzini
@ 2016-01-08 17:37 ` Paolo Bonzini
2016-01-22 14:01 ` [Qemu-devel] [RFC PATCH 0/2] Early release of -drive QemuOpts Paolo Bonzini
2 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2016-01-08 17:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, qemu-block, Max Reitz
Currently, blockdev_del_drive does a blk_unref (and before it
blockdev_auto_del did the same) that will cause blk_delete to be called
and the DriveInfo to be freed. But really, we want to free the drive info
as soon as the device is detached, even if there are other references
for whatever reason, so that the QemuOpts are freed as well and the id
can be reused.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/block-backend.c | 14 ++++++++++----
blockdev.c | 7 ++++---
include/sysemu/blockdev.h | 1 +
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 9889e81..a1db52b 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -62,8 +62,6 @@ static const AIOCBInfo block_backend_aiocb_info = {
.aiocb_size = sizeof(BlockBackendAIOCB),
};
-static void drive_info_del(DriveInfo *dinfo);
-
/* All the BlockBackends (except for hidden ones) */
static QTAILQ_HEAD(, BlockBackend) blk_backends =
QTAILQ_HEAD_INITIALIZER(blk_backends);
@@ -160,6 +158,7 @@ static void blk_delete(BlockBackend *blk)
{
assert(!blk->refcnt);
assert(!blk->dev);
+ assert(!blk->legacy_dinfo);
if (blk->bs) {
assert(blk->bs->blk == blk);
blk->bs->blk = NULL;
@@ -175,19 +174,26 @@ static void blk_delete(BlockBackend *blk)
QTAILQ_REMOVE(&blk_backends, blk, link);
}
g_free(blk->name);
- drive_info_del(blk->legacy_dinfo);
block_acct_cleanup(&blk->stats);
g_free(blk);
}
-static void drive_info_del(DriveInfo *dinfo)
+void blk_release_legacy_dinfo(BlockBackend *blk)
{
+ DriveInfo *dinfo = blk->legacy_dinfo;
+
if (!dinfo) {
return;
}
qemu_opts_del(dinfo->opts);
g_free(dinfo->serial);
g_free(dinfo);
+ blk->legacy_dinfo = NULL;
+
+ /* We are not interested anymore in retrieving the BlockBackend
+ * via blk_by_legacy_dinfo, so let it die.
+ */
+ blk_unref(blk);
}
int blk_get_refcnt(BlockBackend *blk)
diff --git a/blockdev.c b/blockdev.c
index a8309fb..d75be63 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -116,10 +116,10 @@ void override_max_devs(BlockInterfaceType type, int max_devs)
*/
void blockdev_del_drive(BlockBackend *blk)
{
- DriveInfo *dinfo = blk_legacy_dinfo(blk);
BlockDriverState *bs = blk_bs(blk);
AioContext *aio_context;
+ blk_ref(blk);
if (bs) {
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -131,9 +131,10 @@ void blockdev_del_drive(BlockBackend *blk)
aio_context_release(aio_context);
}
- if (dinfo) {
- blk_unref(blk);
+ if (blk_legacy_dinfo(blk)) {
+ blk_release_legacy_dinfo(blk);
}
+ blk_unref(blk);
}
/**
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index ae7ad67..5722b9f 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -44,6 +44,7 @@ struct DriveInfo {
DriveInfo *blk_legacy_dinfo(BlockBackend *blk);
DriveInfo *blk_set_legacy_dinfo(BlockBackend *blk, DriveInfo *dinfo);
BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo);
+void blk_release_legacy_dinfo(BlockBackend *blk);
void override_max_devs(BlockInterfaceType type, int max_devs);
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread