[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/4] block: Keep DriveInfo alive until BlockDriv
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 2/4] block: Keep DriveInfo alive until BlockDriverState dies |
Date: |
Mon, 15 Sep 2014 08:23:30 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Markus Armbruster <address@hidden> writes:
> 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.
Broken when we replaced DriveInfo reference counting by BDS reference
counting in commit a94a3fa..fa510eb.
Kevin, would you mind inserting that into the commit message?
> 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 <address@hidden>
[...]
- [Qemu-devel] [PATCH 4/4] block: Improve message for device name clashing with node name, (continued)