On 20.12.2012, at 11:54, Michael S. Tsirkin wrote:
> On Wed, Dec 19, 2012 at 08:40:15PM +0100, Alexander Graf wrote:
>> When a file system is mounted on a virtio-blk disk, we then remove it
>> and then reattach it, the reattached disk gets the same disk name and
>> ids as the hot removed one.
>>
>> This leads to very nasty effects - mostly rendering the newly attached
>> device completely unusable.
>>
>> Trying what happens when I do the same thing with a USB device, I saw
>> that the sd node simply doesn't get free'd when a device gets forcefully
>> removed.
>>
>> Imitate the same behavior for vd devices. This way broken vd devices
>> simply are never free'd and newly attached ones keep working just fine.
>>
>> Signed-off-by: Alexander Graf <ag...@suse.de>
>> ---
>> drivers/block/virtio_blk.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index 0bdde8f..07a18e2 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
>> @@ -889,6 +889,7 @@ static void __devexit virtblk_remove(struct
>> virtio_device *vdev)
>> {
>> struct virtio_blk *vblk = vdev->priv;
>> int index = vblk->index;
>> + int refc;
>>
>> /* Prevent config work handler from accessing the device. */
>> mutex_lock(&vblk->config_lock);
>> @@ -903,11 +904,15 @@ static void __devexit virtblk_remove(struct
>> virtio_device *vdev)
>>
>> flush_work(&vblk->config_work);
>>
>> + refc = atomic_read(&disk_to_dev(vblk->disk)->kobj.kref.refcount);
>> put_disk(vblk->disk);
>> mempool_destroy(vblk->pool);
>> vdev->config->del_vqs(vdev);
>> kfree(vblk);
>> - ida_simple_remove(&vd_index_ida, index);
>> +
>> + /* Only free device id if we don't have any users */
>> + if (refc == 1)
>> + ida_simple_remove(&vd_index_ida, index);
>> }
>>
>> #ifdef CONFIG_PM
>
> Network devices take the approach of retrying every second.
> Donnu if it makes sense here.
I would rather think the 100% right approach would be a recursive unrolling of
all users bottom to top. Force unmount. Force close all fd's. I'm not sure why
that doesn't happen today, but it doesn't :).
Alex
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
