On 12/20/2012 07:47 PM, Alexander Graf wrote:
>
> 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 --
What kernel version are you using? Any debug messages available.
>
> 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
>
--
Asias
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
