From 3ae59580a0db469c1de72d5c58266b08fb096056 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 12 Sep 2014 21:26:22 +0200 Subject: block: Keep DriveInfo alive until BlockDriverState dies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Max Reitz Reviewed-by: Benoît Canet Signed-off-by: Kevin Wolf --- include/sysemu/blockdev.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') 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 */ -- cgit v1.1 From 9aebf3b89281a173d2dfeee379b800be5e3f363e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 25 Sep 2014 09:54:02 +0200 Subject: block: Validate node-name The device_name of a BlockDriverState is currently checked because it is always used as a QemuOpts ID and qemu_opts_create() checks whether such IDs are wellformed. node-name is supposed to share the same namespace, but it isn't checked currently. This patch adds explicit checks both for device_name and node-name so that the same rules will still apply even if QemuOpts won't be used any more at some point. qemu-img used to use names with spaces in them, which isn't allowed any more. Replace them with underscores. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- include/qemu/option.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/qemu/option.h b/include/qemu/option.h index 59bea75..945347c 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -103,6 +103,7 @@ typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaq int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, int abort_on_failure); +int qemu_opts_id_wellformed(const char *id); QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id); QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists, Error **errp); -- cgit v1.1