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 <agraf@xxxxxxx>
> >> ---
> >> 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.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
