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>
This commit is contained in:
Markus Armbruster 2014-09-12 21:26:22 +02:00 committed by Kevin Wolf
parent a0f1eab157
commit 3ae59580a0
5 changed files with 24 additions and 5 deletions

View File

@ -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);
}

View File

@ -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:

View File

@ -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 */

View File

@ -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

12
stubs/blockdev.c Normal file
View File

@ -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);
}