On 20.12.2012, at 12:38, Michael S. Tsirkin wrote: > On Thu, Dec 20, 2012 at 12:27:53PM +0100, Alexander Graf wrote: >> >> 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 > > Especially force close fd's won't be easy. A simpler alternative > would be preventing unplug by taking some reference count.
That's what happens when you do <add> mount /mnt ls /mnt <remove> -- here remove failed for me -- But when you do <add> mount /mnt <remove> then remove works and we get the above problem. I have to admit that I'm really not a big fan of preventing unplug though. In some cases, preventing simply doesn't work, for example when you unplug a USB stick or when your iscsi remote just vanishes. Alex _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization