Xen devel - Apr 2012 - [PATCH] xen-blkfront: properly name all devices

- devices beyond xvdzz didn''t get proper names assigned at all
- extended devices with minors not representable within the kernel''s
  major/minor bit split spilled into foreign majors

Signed-off-by: Jan Beulich <jbeulich@suse.com>

---
 drivers/block/xen-blkfront.c |   40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

--- 3.4-rc1/drivers/block/xen-blkfront.c
+++ 3.4-rc1-xen-blkfront-all-devices/drivers/block/xen-blkfront.c
@@ -528,6 +528,14 @@ static int xen_translate_vdev(int vdevic
 	return 0;
 }
 
+static char *encode_disk_name(char *ptr, unsigned int n)
+{
+	if (n >= 26)
+		ptr = encode_disk_name(ptr, n / 26 - 1);
+	*ptr = ''a'' + n % 26;
+	return ptr + 1;
+}
+
 static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
 			       struct blkfront_info *info,
 			       u16 vdisk_info, u16 sector_size)
@@ -538,6 +546,7 @@ static int xlvbd_alloc_gendisk(blkif_sec
 	unsigned int offset;
 	int minor;
 	int nr_parts;
+	char *ptr;
 
 	BUG_ON(info->gd != NULL);
 	BUG_ON(info->rq != NULL);
@@ -562,7 +571,11 @@ static int xlvbd_alloc_gendisk(blkif_sec
 					"emulated IDE disks,\n\t choose an xvd device name"
 					"from xvde on\n", info->vdevice);
 	}
-	err = -ENODEV;
+	if (minor >> MINORBITS) {
+		pr_warn("blkfront: %#x''s minor (%#x) out of range;
ignoring\n",
+			info->vdevice, minor);
+		return -ENODEV;
+	}
 
 	if ((minor % nr_parts) == 0)
 		nr_minors = nr_parts;
@@ -576,23 +589,14 @@ static int xlvbd_alloc_gendisk(blkif_sec
 	if (gd == NULL)
 		goto release;
 
-	if (nr_minors > 1) {
-		if (offset < 26)
-			sprintf(gd->disk_name, "%s%c", DEV_NAME, ''a''
+ offset);
-		else
-			sprintf(gd->disk_name, "%s%c%c", DEV_NAME,
-				''a'' + ((offset / 26)-1), ''a'' + (offset
% 26));
-	} else {
-		if (offset < 26)
-			sprintf(gd->disk_name, "%s%c%d", DEV_NAME,
-				''a'' + offset,
-				minor & (nr_parts - 1));
-		else
-			sprintf(gd->disk_name, "%s%c%c%d", DEV_NAME,
-				''a'' + ((offset / 26) - 1),
-				''a'' + (offset % 26),
-				minor & (nr_parts - 1));
-	}
+	strcpy(gd->disk_name, DEV_NAME);
+	ptr = encode_disk_name(gd->disk_name + sizeof(DEV_NAME) - 1, offset);
+	BUG_ON(ptr >= gd->disk_name + DISK_NAME_LEN);
+	if (nr_minors > 1)
+		*ptr = 0;
+	else
+		snprintf(ptr, gd->disk_name + DISK_NAME_LEN - ptr,
+			 "%d", minor & (nr_parts - 1));
 
 	gd->major = XENVBD_MAJOR;
 	gd->first_minor = minor;

On Thu, 5 Apr 2012, Jan Beulich wrote:
> - devices beyond xvdzz didn''t get proper names assigned at all
> - extended devices with minors not representable within the
kernel''s
>   major/minor bit split spilled into foreign majors
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
The patch is OK.

Would you be up for writing a couple of lines under Documentation to
clearly state what device names are going to be given by blkfront in
correspondence to vdev numbers?

>>> On 10.04.12 at 17:50, Stefano Stabellini
<stefano.stabellini@eu.citrix.com>
> Would you be up for writing a couple of lines under Documentation to
> clearly state what device names are going to be given by blkfront in
> correspondence to vdev numbers?
I''m not really eager to, and in order to be useful, this should imo
include
the compat mapping, and specifically for this I''d prefer someone (you?)
who was involved in putting this together to do eventual write-ups for it.

Jan

On Tue, 10 Apr 2012, Jan Beulich wrote:
> >>> On 10.04.12 at 17:50, Stefano Stabellini
<stefano.stabellini@eu.citrix.com>
> > Would you be up for writing a couple of lines under Documentation to
> > clearly state what device names are going to be given by blkfront in
> > correspondence to vdev numbers?
> 
> I''m not really eager to, and in order to be useful, this should
imo include
> the compat mapping, and specifically for this I''d prefer someone
(you?)
> who was involved in putting this together to do eventual write-ups for it.
 
What compat mappings?
Do you mean the non-extended vdev names?

If you refer to the behavior of older kernels, I don''t think it matters
here: the document is supposed to specify what the corresponding kernel
codebase does about xen block names, not what past kernels used to do.
Also, the Xen side vdev policy doesn''t matter that much, because we
already clarified that it is up to the guest to choose whatever name it
wants for a given vdev.

In any case if you don''t feel like writing it up, I''ll do it.