[PATCH] virtio_blk: Add help function to format mass of disks

[PATCH] virtio_blk: Add help function to format mass of disks

[Ren Mingxin]

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);
+	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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Asias He]

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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Avi Kivity]

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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Michael S. Tsirkin]

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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Tejun Heo]

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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Michael S. Tsirkin]

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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Asias He]

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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Ren Mingxin]

  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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Ren Mingxin]

  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

Re: [PATCH] virtio_blk: Add help function to format mass of disks

[Michael S. Tsirkin]

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