If the BDS's refcnt > 0, drive_del() destroys the DriveInfo, but not
the BDS. This can happen in three places:
* Device model destruction during unplug: blockdev_auto_del()
* Xen IDE unplug: pci_piix3_xen_ide_unplug()
* drive_del command when no device model is attached: do_drive_del()
The other callers of drive_del are on error paths where refcnt == 1.
If the user somehow manages to plug in a device model using a BDS that
has gone through drive_del(), the legacy configuration passed in
DriveInfo doesn't reach the device model, and automatic deletion on
unplug doesn't work. Worse, some device models such as scsi-disk
crash when DriveInfo doesn't exist.
This is theoretical; I didn't research an actual reproducer.
Fix by keeping DriveInfo alive until its BDS dies.
This affects qemu_drive_opts: now you can't reuse the same ID for new
drive options until the BDS dies. Before, you could, but since the
code always attempts to create a BDS with the same ID next, the
enclosing operation "create a new drive" failed anyway. Different
error path, same result.
Unfortunately, the fix involves use of blockdev.c stuff from block.c,
which is a layering violation. Fortunately, my forthcoming
BlockBackend work will get rid of it again.
Signed-off-by: Markus Armbruster <arm...@redhat.com>
---
block.c | 2 ++
blockdev.c | 13 ++++++++-----
include/sysemu/blockdev.h | 1 +
stubs/Makefile.objs | 1 +
stubs/blockdev.c | 12 ++++++++++++
5 files changed, 24 insertions(+), 5 deletions(-)
create mode 100644 stubs/blockdev.c
diff --git a/block.c b/block.c
index d06dd51..6faf36f 100644
--- a/block.c
+++ b/block.c
@@ -29,6 +29,7 @@
#include "qemu/module.h"
#include "qapi/qmp/qjson.h"
#include "sysemu/sysemu.h"
+#include "sysemu/blockdev.h" /* FIXME layering violation */
#include "qemu/notify.h"
#include "block/coroutine.h"
#include "block/qapi.h"
@@ -2110,6 +2111,7 @@ static void bdrv_delete(BlockDriverState *bs)
/* remove from list, if necessary */
bdrv_make_anon(bs);
+ drive_info_del(drive_get_by_blockdev(bs));
g_free(bs);
}
diff --git a/blockdev.c b/blockdev.c
index 5ec4635..450f95c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -216,11 +216,17 @@ static void bdrv_format_print(void *opaque, const char
*name)
void drive_del(DriveInfo *dinfo)
{
+ bdrv_unref(dinfo->bdrv);
+}
+
+void drive_info_del(DriveInfo *dinfo)
+{
+ if (!dinfo) {
+ return;
+ }
if (dinfo->opts) {
qemu_opts_del(dinfo->opts);
}
-
- bdrv_unref(dinfo->bdrv);
g_free(dinfo->id);
QTAILQ_REMOVE(&drives, dinfo, next);
g_free(dinfo->serial);
@@ -525,9 +531,6 @@ static DriveInfo *blockdev_init(const char *file, QDict
*bs_opts,
err:
bdrv_unref(bs);
- QTAILQ_REMOVE(&drives, dinfo, next);
- g_free(dinfo->id);
- g_free(dinfo);
early_err:
qemu_opts_del(opts);
err_no_opts:
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 23a5d10..abec381 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -56,6 +56,7 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const
char *file,
const char *optstr);
DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
void drive_del(DriveInfo *dinfo);
+void drive_info_del(DriveInfo *dinfo);
/* device-hotplug */
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 5e347d0..c0b1f6a 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,5 +1,6 @@
stub-obj-y += arch-query-cpu-def.o
stub-obj-y += bdrv-commit-all.o
+stub-obj-y += blockdev.o
stub-obj-y += chr-baum-init.o
stub-obj-y += chr-msmouse.o
stub-obj-y += chr-testdev.o
diff --git a/stubs/blockdev.c b/stubs/blockdev.c
new file mode 100644
index 0000000..5d0a79c
--- /dev/null
+++ b/stubs/blockdev.c
@@ -0,0 +1,12 @@
+#include <assert.h>
+#include "sysemu/blockdev.h"
+
+DriveInfo *drive_get_by_blockdev(BlockDriverState *bs)
+{
+ return NULL;
+}
+
+void drive_info_del(DriveInfo *dinfo)
+{
+ assert(!dinfo);
+}
--
1.9.3
