* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-10 7:28 [PATCH] virtio_blk: Add help function to format mass of disks Ren Mingxin
@ 2012-04-10 13:08 ` Asias He
2012-04-10 13:16 ` Avi Kivity
2012-04-11 8:43 ` Michael S. Tsirkin
2 siblings, 0 replies; 10+ messages in thread
From: Asias He @ 2012-04-10 13:08 UTC (permalink / raw)
To: Ren Mingxin
Cc: axboe, kvm, linux-scsi, mst, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, tj
On 04/10/2012 03:28 PM, Ren Mingxin wrote:
> The current virtio block's naming algorithm just supports 18278
> (26^3 + 26^2 + 26) disks. If there are mass of virtio blocks,
> there will be disks with the same name.
>
> Based on commit 3e1a7ff8a0a7b948f2684930166954f9e8e776fe, I add
> function "virtblk_name_format()" for virtio block to support mass
> of disks naming.
>
> Signed-off-by: Ren Mingxin<renmx@cn.fujitsu.com>
Make sense to me.
Acked-by: Asias He <asias@redhat.com>
> ---
> drivers/block/virtio_blk.c | 38 ++++++++++++++++++++++++++------------
> 1 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index c4a60ba..86516c8 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -374,6 +374,31 @@ static int init_vq(struct virtio_blk *vblk)
> return err;
> }
>
> +static int virtblk_name_format(char *prefix, int index, char *buf, int buflen)
> +{
> + const int base = 'z' - 'a' + 1;
> + char *begin = buf + strlen(prefix);
> + char *begin = buf + strlen(prefix);
> + char *end = buf + buflen;
> + char *p;
> + int unit;
> +
> + p = end - 1;
> + *p = '\0';
> + unit = base;
> + do {
> + if (p == begin)
> + return -EINVAL;
> + *--p = 'a' + (index % unit);
> + index = (index / unit) - 1;
> + } while (index>= 0);
> +
> + memmove(begin, p, end - p);
> + memcpy(buf, prefix, strlen(prefix));
> +
> + return 0;
> +}
> +
> static int __devinit virtblk_probe(struct virtio_device *vdev)
> {
> struct virtio_blk *vblk;
> @@ -442,18 +467,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
>
> q->queuedata = vblk;
>
> - if (index< 26) {
> - sprintf(vblk->disk->disk_name, "vd%c", 'a' + index % 26);
> - } else if (index< (26 + 1) * 26) {
> - sprintf(vblk->disk->disk_name, "vd%c%c",
> - 'a' + index / 26 - 1, 'a' + index % 26);
> - } else {
> - const unsigned int m1 = (index / 26 - 1) / 26 - 1;
> - const unsigned int m2 = (index / 26 - 1) % 26;
> - const unsigned int m3 = index % 26;
> - sprintf(vblk->disk->disk_name, "vd%c%c%c",
> - 'a' + m1, 'a' + m2, 'a' + m3);
> - }
> + virtblk_name_format("vd", index, vblk->disk->disk_name, DISK_NAME_LEN);
>
> vblk->disk->major = major;
> vblk->disk->first_minor = index_to_minor(index);
--
Asias
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-10 7:28 [PATCH] virtio_blk: Add help function to format mass of disks Ren Mingxin
2012-04-10 13:08 ` Asias He
@ 2012-04-10 13:16 ` Avi Kivity
2012-04-10 13:34 ` Michael S. Tsirkin
2012-04-11 1:28 ` Ren Mingxin
2012-04-11 8:43 ` Michael S. Tsirkin
2 siblings, 2 replies; 10+ messages in thread
From: Avi Kivity @ 2012-04-10 13:16 UTC (permalink / raw)
To: Ren Mingxin
Cc: axboe, kvm, linux-scsi, mst, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, tj
On 04/10/2012 10:28 AM, Ren Mingxin wrote:
> The current virtio block's naming algorithm just supports 18278
> (26^3 + 26^2 + 26) disks. If there are mass of virtio blocks,
> there will be disks with the same name.
>
> Based on commit 3e1a7ff8a0a7b948f2684930166954f9e8e776fe, I add
> function "virtblk_name_format()" for virtio block to support mass
> of disks naming.
>
> Signed-off-by: Ren Mingxin <renmx@cn.fujitsu.com>
> ---
> drivers/block/virtio_blk.c | 38 ++++++++++++++++++++++++++------------
> 1 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index c4a60ba..86516c8 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -374,6 +374,31 @@ static int init_vq(struct virtio_blk *vblk)
> return err;
> }
>
> +static int virtblk_name_format(char *prefix, int index, char *buf, int buflen)
> +{
> + const int base = 'z' - 'a' + 1;
> + char *begin = buf + strlen(prefix);
> + char *begin = buf + strlen(prefix);
Duplicate line.
> + char *end = buf + buflen;
> + char *p;
> + int unit;
> +
> + p = end - 1;
> + *p = '\0';
> + unit = base;
Why not use 'base' below? neither unit nor base change.
> + do {
> + if (p == begin)
> + return -EINVAL;
> + *--p = 'a' + (index % unit);
> + index = (index / unit) - 1;
> + } while (index >= 0);
> +
> + memmove(begin, p, end - p);
> + memcpy(buf, prefix, strlen(prefix));
> +
> + return 0;
> +}
> +
>
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-10 13:16 ` Avi Kivity
@ 2012-04-10 13:34 ` Michael S. Tsirkin
2012-04-10 15:49 ` Tejun Heo
2012-04-11 1:28 ` Ren Mingxin
1 sibling, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-04-10 13:34 UTC (permalink / raw)
To: Avi Kivity
Cc: axboe, kvm, linux-scsi, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, Ren Mingxin, tj
On Tue, Apr 10, 2012 at 04:16:10PM +0300, Avi Kivity wrote:
> On 04/10/2012 10:28 AM, Ren Mingxin wrote:
> > The current virtio block's naming algorithm just supports 18278
> > (26^3 + 26^2 + 26) disks. If there are mass of virtio blocks,
> > there will be disks with the same name.
> >
> > Based on commit 3e1a7ff8a0a7b948f2684930166954f9e8e776fe, I add
> > function "virtblk_name_format()" for virtio block to support mass
> > of disks naming.
> >
> > Signed-off-by: Ren Mingxin <renmx@cn.fujitsu.com>
> > ---
> > drivers/block/virtio_blk.c | 38 ++++++++++++++++++++++++++------------
> > 1 files changed, 26 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index c4a60ba..86516c8 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -374,6 +374,31 @@ static int init_vq(struct virtio_blk *vblk)
> > return err;
> > }
> >
> > +static int virtblk_name_format(char *prefix, int index, char *buf, int buflen)
> > +{
> > + const int base = 'z' - 'a' + 1;
> > + char *begin = buf + strlen(prefix);
> > + char *begin = buf + strlen(prefix);
>
> Duplicate line.
>
> > + char *end = buf + buflen;
> > + char *p;
> > + int unit;
> > +
> > + p = end - 1;
> > + *p = '\0';
> > + unit = base;
>
> Why not use 'base' below? neither unit nor base change.
Yes it's a bit strange, it was the same in Tejun's patch.
Tejun, any idea?
> > + do {
> > + if (p == begin)
> > + return -EINVAL;
> > + *--p = 'a' + (index % unit);
> > + index = (index / unit) - 1;
> > + } while (index >= 0);
> > +
> > + memmove(begin, p, end - p);
> > + memcpy(buf, prefix, strlen(prefix));
> > +
> > + return 0;
> > +}
> > +
> >
>
> --
> error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-10 13:34 ` Michael S. Tsirkin
@ 2012-04-10 15:49 ` Tejun Heo
2012-04-10 15:53 ` Michael S. Tsirkin
0 siblings, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2012-04-10 15:49 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: axboe, kvm, linux-scsi, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, Ren Mingxin, Avi Kivity
Hello, guys.
On Tue, Apr 10, 2012 at 04:34:06PM +0300, Michael S. Tsirkin wrote:
> > Why not use 'base' below? neither unit nor base change.
>
> Yes it's a bit strange, it was the same in Tejun's patch.
> Tejun, any idea?
It was years ago, so I don't recall much. I think I wanted to use a
variable name which signifies its role - I worked out the rather
convoluted base number logic on paper first and I probably wanted to
keep the distinctions. I don't think it really matters at this point
tho. Just make sure those functions are marked deprecated so that no
one else copies them.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-10 15:49 ` Tejun Heo
@ 2012-04-10 15:53 ` Michael S. Tsirkin
2012-04-11 1:21 ` Asias He
0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-04-10 15:53 UTC (permalink / raw)
To: Tejun Heo
Cc: axboe, kvm, linux-scsi, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, Ren Mingxin, Avi Kivity
On Tue, Apr 10, 2012 at 08:49:43AM -0700, Tejun Heo wrote:
> Hello, guys.
>
> On Tue, Apr 10, 2012 at 04:34:06PM +0300, Michael S. Tsirkin wrote:
> > > Why not use 'base' below? neither unit nor base change.
> >
> > Yes it's a bit strange, it was the same in Tejun's patch.
> > Tejun, any idea?
>
> It was years ago, so I don't recall much. I think I wanted to use a
> variable name which signifies its role - I worked out the rather
> convoluted base number logic on paper first and I probably wanted to
> keep the distinctions. I don't think it really matters at this point
> tho. Just make sure those functions are marked deprecated so that no
> one else copies them.
>
> Thanks.
I guess I'll keep it same so it's easier to deduplicate
if someon wants to.
> --
> tejun
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-10 15:53 ` Michael S. Tsirkin
@ 2012-04-11 1:21 ` Asias He
2012-04-11 1:31 ` Ren Mingxin
0 siblings, 1 reply; 10+ messages in thread
From: Asias He @ 2012-04-11 1:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: axboe, kvm, linux-scsi, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, Ren Mingxin, Avi Kivity,
Tejun Heo
On 04/10/2012 11:53 PM, Michael S. Tsirkin wrote:
> On Tue, Apr 10, 2012 at 08:49:43AM -0700, Tejun Heo wrote:
>> Hello, guys.
>>
>> On Tue, Apr 10, 2012 at 04:34:06PM +0300, Michael S. Tsirkin wrote:
>>>> Why not use 'base' below? neither unit nor base change.
>>>
>>> Yes it's a bit strange, it was the same in Tejun's patch.
>>> Tejun, any idea?
>>
>> It was years ago, so I don't recall much. I think I wanted to use a
>> variable name which signifies its role - I worked out the rather
>> convoluted base number logic on paper first and I probably wanted to
>> keep the distinctions. I don't think it really matters at this point
>> tho. Just make sure those functions are marked deprecated so that no
>> one else copies them.
>>
>> Thanks.
>
> I guess I'll keep it same so it's easier to deduplicate
> if someon wants to.
Why not fix it both in sd_format_disk_name() and virtblk_name_format().
Ren, mind to send v2 to drop the duplicate line?
>
>> --
>> tejun
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
--
Asias
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-11 1:21 ` Asias He
@ 2012-04-11 1:31 ` Ren Mingxin
0 siblings, 0 replies; 10+ messages in thread
From: Ren Mingxin @ 2012-04-11 1:31 UTC (permalink / raw)
To: Asias He, Michael S. Tsirkin
Cc: axboe, kvm, linux-scsi, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, Avi Kivity, Tejun Heo
On 04/11/2012 09:21 AM, Asias He wrote:
> On 04/10/2012 11:53 PM, Michael S. Tsirkin wrote:
>> On Tue, Apr 10, 2012 at 08:49:43AM -0700, Tejun Heo wrote:
>>> Hello, guys.
>>>
>>> On Tue, Apr 10, 2012 at 04:34:06PM +0300, Michael S. Tsirkin wrote:
>>>>> Why not use 'base' below? neither unit nor base change.
>>>>
>>>> Yes it's a bit strange, it was the same in Tejun's patch.
>>>> Tejun, any idea?
>>>
>>> It was years ago, so I don't recall much. I think I wanted to use a
>>> variable name which signifies its role - I worked out the rather
>>> convoluted base number logic on paper first and I probably wanted to
>>> keep the distinctions. I don't think it really matters at this point
>>> tho. Just make sure those functions are marked deprecated so that no
>>> one else copies them.
>>>
>>> Thanks.
>>
>> I guess I'll keep it same so it's easier to deduplicate
>> if someon wants to.
So, I'd keep this in the next version.
>
> Why not fix it both in sd_format_disk_name() and virtblk_name_format().
> Ren, mind to send v2 to drop the duplicate line?
>
I'll send v2 soon.
--
Thanks,
Ren
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-10 13:16 ` Avi Kivity
2012-04-10 13:34 ` Michael S. Tsirkin
@ 2012-04-11 1:28 ` Ren Mingxin
1 sibling, 0 replies; 10+ messages in thread
From: Ren Mingxin @ 2012-04-11 1:28 UTC (permalink / raw)
To: Avi Kivity
Cc: axboe, kvm, linux-scsi, mst, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, tj
On 04/10/2012 09:16 PM, Avi Kivity wrote:
> On 04/10/2012 10:28 AM, Ren Mingxin wrote:
>> The current virtio block's naming algorithm just supports 18278
>> (26^3 + 26^2 + 26) disks. If there are mass of virtio blocks,
>> there will be disks with the same name.
>>
>> Based on commit 3e1a7ff8a0a7b948f2684930166954f9e8e776fe, I add
>> function "virtblk_name_format()" for virtio block to support mass
>> of disks naming.
>>
>> Signed-off-by: Ren Mingxin<renmx@cn.fujitsu.com>
>> ---
>> drivers/block/virtio_blk.c | 38 ++++++++++++++++++++++++++------------
>> 1 files changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index c4a60ba..86516c8 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
>> @@ -374,6 +374,31 @@ static int init_vq(struct virtio_blk *vblk)
>> return err;
>> }
>>
>> +static int virtblk_name_format(char *prefix, int index, char *buf, int buflen)
>> +{
>> + const int base = 'z' - 'a' + 1;
>> + char *begin = buf + strlen(prefix);
>> + char *begin = buf + strlen(prefix);
> Duplicate line.
>
Oh, obvious missed :-(
--
Thanks,
Ren
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] virtio_blk: Add help function to format mass of disks
2012-04-10 7:28 [PATCH] virtio_blk: Add help function to format mass of disks Ren Mingxin
2012-04-10 13:08 ` Asias He
2012-04-10 13:16 ` Avi Kivity
@ 2012-04-11 8:43 ` Michael S. Tsirkin
2 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-04-11 8:43 UTC (permalink / raw)
To: Ren Mingxin
Cc: axboe, kvm, linux-scsi, asamymuthupa, linux-kernel,
virtualization, James.Bottomley, tj
On Tue, Apr 10, 2012 at 03:28:05PM +0800, Ren Mingxin wrote:
> The current virtio block's naming algorithm just supports 18278
> (26^3 + 26^2 + 26) disks. If there are mass of virtio blocks,
> there will be disks with the same name.
>
> Based on commit 3e1a7ff8a0a7b948f2684930166954f9e8e776fe, I add
> function "virtblk_name_format()" for virtio block to support mass
> of disks naming.
>
> Signed-off-by: Ren Mingxin <renmx@cn.fujitsu.com>
Applied, thanks everyone.
> ---
> drivers/block/virtio_blk.c | 38 ++++++++++++++++++++++++++------------
> 1 files changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index c4a60ba..86516c8 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -374,6 +374,31 @@ static int init_vq(struct virtio_blk *vblk)
> return err;
> }
>
> +static int virtblk_name_format(char *prefix, int index, char *buf, int buflen)
> +{
> + const int base = 'z' - 'a' + 1;
> + char *begin = buf + strlen(prefix);
> + char *begin = buf + strlen(prefix);
> + char *end = buf + buflen;
> + char *p;
> + int unit;
> +
> + p = end - 1;
> + *p = '\0';
> + unit = base;
> + do {
> + if (p == begin)
> + return -EINVAL;
> + *--p = 'a' + (index % unit);
> + index = (index / unit) - 1;
> + } while (index >= 0);
> +
> + memmove(begin, p, end - p);
> + memcpy(buf, prefix, strlen(prefix));
> +
> + return 0;
> +}
> +
> static int __devinit virtblk_probe(struct virtio_device *vdev)
> {
> struct virtio_blk *vblk;
> @@ -442,18 +467,7 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
>
> q->queuedata = vblk;
>
> - if (index < 26) {
> - sprintf(vblk->disk->disk_name, "vd%c", 'a' + index % 26);
> - } else if (index < (26 + 1) * 26) {
> - sprintf(vblk->disk->disk_name, "vd%c%c",
> - 'a' + index / 26 - 1, 'a' + index % 26);
> - } else {
> - const unsigned int m1 = (index / 26 - 1) / 26 - 1;
> - const unsigned int m2 = (index / 26 - 1) % 26;
> - const unsigned int m3 = index % 26;
> - sprintf(vblk->disk->disk_name, "vd%c%c%c",
> - 'a' + m1, 'a' + m2, 'a' + m3);
> - }
> + virtblk_name_format("vd", index, vblk->disk->disk_name, DISK_NAME_LEN);
>
> vblk->disk->major = major;
> vblk->disk->first_minor = index_to_minor(index);
> --
> 1.7.1
^ permalink raw reply [flat|nested] 10+ messages in thread