[PATCH] virtio_blk: decrement index when blockdevice is removed

[PATCH] virtio_blk: decrement index when blockdevice is removed

[Takuma Umeya]

When virtio block device is removed, index does not get decremented. When 
another virtio disk is attached it uses the next device letter to the 
one that is suppose to be available. 

Signed-off-by: Takuma Umeya <tumeya@redhat.com>

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6ecf89c..730e7af 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -489,6 +489,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
        mempool_destroy(vblk->pool);
        vdev->config->del_vqs(vdev);
        kfree(vblk);
+       index--;
 }
 
 static const struct virtio_device_id id_table[] = {

-- 
Umeya, Takuma
Technical Account Manager
Red Hat GSS APAC
+81.3.5798.8584 (direct)
tumeya@redhat.com

Re: [PATCH] virtio_blk: decrement index when blockdevice is removed

[Jens Axboe]

On 2011-04-05 06:49, Takuma Umeya wrote:
> When virtio block device is removed, index does not get decremented. When 
> another virtio disk is attached it uses the next device letter to the 
> one that is suppose to be available. 
> 
> Signed-off-by: Takuma Umeya <tumeya@redhat.com>
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 6ecf89c..730e7af 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -489,6 +489,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
>         mempool_destroy(vblk->pool);
>         vdev->config->del_vqs(vdev);
>         kfree(vblk);
> +       index--;
>  }
>  
>  static const struct virtio_device_id id_table[] = {

What happens when you delete a device that isn't the last one?

-- 
Jens Axboe

Re: [PATCH] virtio_blk: decrement index when blockdevice is removed

[Rusty Russell]

On Tue, 05 Apr 2011 07:08:12 +0200, Jens Axboe <jaxboe@fusionio.com> wrote:
> On 2011-04-05 06:49, Takuma Umeya wrote:
> > When virtio block device is removed, index does not get decremented. When 
> > another virtio disk is attached it uses the next device letter to the 
> > one that is suppose to be available. 
> > 
> > Signed-off-by: Takuma Umeya <tumeya@redhat.com>
> > 
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index 6ecf89c..730e7af 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -489,6 +489,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
> >         mempool_destroy(vblk->pool);
> >         vdev->config->del_vqs(vdev);
> >         kfree(vblk);
> > +       index--;
> >  }
> >  
> >  static const struct virtio_device_id id_table[] = {
> 
> What happens when you delete a device that isn't the last one?

Obviously we should do something cleverer for assigning drives.

It might be a cute if genhd gave us a function to get the next free
index for a given major number, and format it for us, like so:

      /* Return the next available minor for a given @major, at least
       * @spacing after the previous and, and append appropriate letters
       * to @name if it's not NULL.  -ve errno on fail (-ENOSPC?). */
      int disk_next_minor(int major, unsigned spacing, char *name);

A trivial optimization would be to remember the last major and max minor
(resetting that if any disks are removed).

This could clean up other code, too.

Cheers,
Rusty.

Re: [PATCH] virtio_blk: decrement index when blockdevice is removed

[Jens Axboe]

On 2011-04-06 03:32, Rusty Russell wrote:
> On Tue, 05 Apr 2011 07:08:12 +0200, Jens Axboe <jaxboe@fusionio.com> wrote:
>> On 2011-04-05 06:49, Takuma Umeya wrote:
>>> When virtio block device is removed, index does not get decremented. When 
>>> another virtio disk is attached it uses the next device letter to the 
>>> one that is suppose to be available. 
>>>
>>> Signed-off-by: Takuma Umeya <tumeya@redhat.com>
>>>
>>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>>> index 6ecf89c..730e7af 100644
>>> --- a/drivers/block/virtio_blk.c
>>> +++ b/drivers/block/virtio_blk.c
>>> @@ -489,6 +489,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
>>>         mempool_destroy(vblk->pool);
>>>         vdev->config->del_vqs(vdev);
>>>         kfree(vblk);
>>> +       index--;
>>>  }
>>>  
>>>  static const struct virtio_device_id id_table[] = {
>>
>> What happens when you delete a device that isn't the last one?
> 
> Obviously we should do something cleverer for assigning drives.
> 
> It might be a cute if genhd gave us a function to get the next free
> index for a given major number, and format it for us, like so:
> 
>       /* Return the next available minor for a given @major, at least
>        * @spacing after the previous and, and append appropriate letters
>        * to @name if it's not NULL.  -ve errno on fail (-ENOSPC?). */
>       int disk_next_minor(int major, unsigned spacing, char *name);
> 
> A trivial optimization would be to remember the last major and max minor
> (resetting that if any disks are removed).
> 
> This could clean up other code, too.

Something like idr would be a good fit for this. But yes, adding some
helpers for this might not hurt...

-- 
Jens Axboe

Re: [PATCH] virtio_blk: decrement index when blockdevice is removed

[Ryan Harper]

* Takuma Umeya <tumeya@redhat.com> [2011-04-05 00:00]:
> When virtio block device is removed, index does not get decremented. When 
> another virtio disk is attached it uses the next device letter to the 
> one that is suppose to be available. 
> 
> Signed-off-by: Takuma Umeya <tumeya@redhat.com>
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 6ecf89c..730e7af 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -489,6 +489,7 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
>         mempool_destroy(vblk->pool);
>         vdev->config->del_vqs(vdev);
>         kfree(vblk);
> +       index--;
>  }

What about referencing /dev/disk/by-id  and using drive serial numbers?
How much do we care about re-using minor numbers?  IIRC, virtio-blk can
go out to vdxxx; that's a lot of disks or add/removes.

> 
>  static const struct virtio_device_id id_table[] = {
> 
> -- 
> Umeya, Takuma
> Technical Account Manager
> Red Hat GSS APAC
> +81.3.5798.8584 (direct)
> tumeya@redhat.com
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/virtualization

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com