aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarkus Armbruster <armbru@redhat.com>2014-09-12 21:26:22 +0200
committerKevin Wolf <kwolf@redhat.com>2014-09-25 15:24:14 +0200
commit3ae59580a0db469c1de72d5c58266b08fb096056 (patch)
tree1ffa044464f4f93492bd490697b035d9cb92f605
parenta0f1eab157560d953e8460b066a362f034bbf717 (diff)
block: Keep DriveInfo alive until BlockDriverState dies
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. The problem was introduced when we replaced DriveInfo reference counting by BDS reference counting in commit a94a3fa..fa510eb. 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 <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: BenoƮt Canet <benoit.canet@nodalink.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
-rw-r--r--block.c2
-rw-r--r--blockdev.c13
-rw-r--r--include/sysemu/blockdev.h1
-rw-r--r--stubs/Makefile.objs1
-rw-r--r--stubs/blockdev.c12
5 files changed, 24 insertions, 5 deletions
diff --git a/block.c b/block.c
index a857913fc2..0c4540c060 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 5ec4635437..450f95cbe7 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 23a5d10c68..abec381049 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 5e347d04bd..c0b1f6a320 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 0000000000..5d0a79c3a6
--- /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);
+}