Re: RFC: Modularize /proc/partitons (was Re: /proc/partitions for LVM)

From: Brian Kress (kressb@fsc-usa.com)
Date: Thu Nov 23 2000 - 14:52:39 EST


"Heinz J. Mauelshagen" wrote:
>
> On Wed, Nov 22, 2000 at 04:27:51PM -0500, Brian Kress wrote:
> > "Heinz J. Mauelshagen" wrote:
> > >
> > > On Wed, Nov 22, 2000 at 04:05:39PM -0500, Brian Kress wrote:
> > > > Question about /proc/partitions and LVM. LVM devices in
> > > > /proc/partitions currently show up as lvma, lvmb, etc, depending on
> > > > their device number. This breaks things like mount by filesystem
> > > > name. Back when LVM existed as a patch, I believe there was some
> > > > support in fs/partitions/check.c for showing the proper name for
> > > > the device (something like <vgname>/<lvname>).
> > >
> > > Yes.
> > > It actually was <vgname>/<lvname>.
> > >
> > > Linus didn't accept it though.
> > >
> > > The code is still available in case he changes his mind.
> >
> > Yes, I can see that now looking through lvm.c. (LVM_HD_NAME)
> > I'm guessing why he didn't take it is the minor hack of adding a
> > function pointer to genhd.c as a special case just for LVM.
> > A general solution to this problem would be nice. The
> > big switch statement in disk_name in fs/paritions/check.c is kind
> > of ugly as it has generic code have to know things about specific
> > drivers.
>
> Yep.
>
> > How's this for an idea to generalize this. Put a function
> > pointer in the gendisk structure for a specific function to call
> > for disk_name for disks for that gendisk.
>
> Sounds resonable to me.
>
> > That way, LVM gets its
> > <vgname>/<lvname>, IDE gets its two disks per major (special cased
> > right now), all the other special cases get what they need and future
> > drivers can call their devices whatever they like without touching
> > this file. Makes the whole thing more modular. Make a NULL for
> > this function default to the old <name>a, <name>b, etc.
> > What to people think about this? If this sounds reasonable
> > I can come up with a patch.
>
> Very good. I'ld appreciate it.
>

OK, here's the patch. It has three sets of changes:

1) Add a function pointer to struct gendisk called hd_name.

2) Make disk_name in fs/partitions/check.c use that function
    pointer if its non-null.

3) Change the following drivers to use this method. (adding the
    driver specific method and removing the old code in check.c)

drivers/md/lvm.c
drivers/md/md.c
drivers/ide/ide-probe.c
drivers/scsi/sd.c
drivers/block/cpqarray.c
drivers/block/cciss.c
drivers/block/DAC960.c

        Let me know what you think. Think there's any chance we
can get this in the main kernel tree?

Brian Kress
kressb@fsc-usa.com

diff -u --recursive linux-2.4.0-test11/drivers/block/DAC960.c linux-2.4.0-test11-ppfix/drivers/block/DAC960.c
--- linux-2.4.0-test11/drivers/block/DAC960.c Mon Nov 20 15:17:25 2000
+++ linux-2.4.0-test11-ppfix/drivers/block/DAC960.c Thu Nov 23 14:29:37 2000
@@ -1885,6 +1885,21 @@
 }
 
 
+char *DAC960_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int ctlr = hd->major - DAC960_MAJOR;
+ int disk = minor >> hd->minor_shift;
+ int part = minor & ((1 << hd->minor_shift) - 1);
+
+ if (part)
+ sprintf(buf, "%s/c%dd%dp%d", hd->major_name, ctlr, disk, part);
+ else
+ sprintf(buf, "%s/c%dd%d", hd->major_name, ctlr, disk);
+ return buf;
+}
+
+
 /*
   DAC960_RegisterBlockDevice registers the Block Device structures
   associated with Controller.
@@ -1945,6 +1960,7 @@
   Controller->GenericDiskInfo.nr_real = Controller->LogicalDriveCount;
   Controller->GenericDiskInfo.next = NULL;
   Controller->GenericDiskInfo.fops = &DAC960_BlockDeviceOperations;
+ Controller->GenericDiskInfo.hd_name = DAC960_disk_name;
   /*
     Install the Generic Disk Information structure at the end of the list.
   */
diff -u --recursive linux-2.4.0-test11/drivers/block/cciss.c linux-2.4.0-test11-ppfix/drivers/block/cciss.c
--- linux-2.4.0-test11/drivers/block/cciss.c Mon Nov 20 15:17:25 2000
+++ linux-2.4.0-test11-ppfix/drivers/block/cciss.c Thu Nov 23 14:22:55 2000
@@ -1749,6 +1749,20 @@
         kfree(size_buff);
 }
 
+char *cciss_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int ctlr = hd->major - COMPAQ_CISS_MAJOR;
+ int disk = minor >> hd->minor_shift;
+ int part = minor & ((1 << hd->minor_shift) - 1);
+
+ if (part)
+ sprintf(buf, "%s/c%dd%dp%d", hd->major_name, ctlr, disk, part);
+ else
+ sprintf(buf, "%s/c%dd%d", hd->major_name, ctlr, disk);
+ return buf;
+}
+
 /*
  * This is it. Find all the controllers and register them. I really hate
  * stealing all these major device numbers.
@@ -1851,6 +1865,7 @@
                 hba[i]->gendisk.part = hba[i]->hd;
                 hba[i]->gendisk.sizes = hba[i]->sizes;
                 hba[i]->gendisk.nr_real = hba[i]->num_luns;
+ hba[i]->gendisk.hd_name = cciss_disk_name;
 
                 /* Get on the disk list */
                 hba[i]->gendisk.next = gendisk_head;
diff -u --recursive linux-2.4.0-test11/drivers/block/cpqarray.c linux-2.4.0-test11-ppfix/drivers/block/cpqarray.c
--- linux-2.4.0-test11/drivers/block/cpqarray.c Mon Nov 20 15:20:29 2000
+++ linux-2.4.0-test11-ppfix/drivers/block/cpqarray.c Thu Nov 23 14:14:03 2000
@@ -362,6 +362,20 @@
 }
 #endif /* MODULE */
 
+char *ida_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int ctlr = hd->major - COMPAQ_SMART2_MAJOR;
+ int disk = minor >> hd->minor_shift;
+ int part = minor & ((1 << hd->minor_shift) - 1);
+
+ if (part)
+ sprintf(buf, "%s/c%dd%dp%d", hd->major_name, ctlr, disk, part);
+ else
+ sprintf(buf, "%s/c%dd%d", hd->major_name, ctlr, disk);
+ return buf;
+}
+
 /*
  * This is it. Find all the controllers and register them. I really hate
  * stealing all these major device numbers.
@@ -512,6 +526,7 @@
                 ida_gendisk[i].part = ida + (i*256);
                 ida_gendisk[i].sizes = ida_sizes + (i*256);
                 ida_gendisk[i].nr_real = 0;
+ ida_gendisk[i].hd_name = ida_disk_name;
         
                 /* Get on the disk list */
                 ida_gendisk[i].next = gendisk_head;
diff -u --recursive linux-2.4.0-test11/drivers/ide/ide-probe.c linux-2.4.0-test11-ppfix/drivers/ide/ide-probe.c
--- linux-2.4.0-test11/drivers/ide/ide-probe.c Thu Aug 3 19:29:49 2000
+++ linux-2.4.0-test11-ppfix/drivers/ide/ide-probe.c Thu Nov 23 12:32:16 2000
@@ -726,6 +726,40 @@
         return 0;
 }
 
+char* ide_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int unit = (minor >> hd->minor_shift) + 'a';
+ unsigned int part = minor & ((1 << hd->minor_shift) - 1);
+
+ switch (hd->major) {
+ case IDE9_MAJOR:
+ unit += 2;
+ case IDE8_MAJOR:
+ unit += 2;
+ case IDE7_MAJOR:
+ unit += 2;
+ case IDE6_MAJOR:
+ unit += 2;
+ case IDE5_MAJOR:
+ unit += 2;
+ case IDE4_MAJOR:
+ unit += 2;
+ case IDE3_MAJOR:
+ unit += 2;
+ case IDE2_MAJOR:
+ unit += 2;
+ case IDE1_MAJOR:
+ unit += 2;
+ case IDE0_MAJOR:
+ }
+ if (part)
+ sprintf(buf, "hd%c%d", unit, part);
+ else
+ sprintf(buf, "hd%c", unit);
+ return buf;
+}
+
 /*
  * init_gendisk() (as opposed to ide_geninit) is called for each major device,
  * after probing for drives, to allocate partition tables and other data
@@ -781,6 +815,7 @@
         gd->fops = ide_fops; /* file operations */
         gd->de_arr = kmalloc (sizeof *gd->de_arr * units, GFP_KERNEL);
         gd->flags = kmalloc (sizeof *gd->flags * units, GFP_KERNEL);
+ gd->hd_name = ide_disk_name;
         if (gd->de_arr)
                 memset (gd->de_arr, 0, sizeof *gd->de_arr * units);
         if (gd->flags)
diff -u --recursive linux-2.4.0-test11/drivers/md/lvm.c linux-2.4.0-test11-ppfix/drivers/md/lvm.c
--- linux-2.4.0-test11/drivers/md/lvm.c Mon Nov 20 15:20:30 2000
+++ linux-2.4.0-test11-ppfix/drivers/md/lvm.c Thu Nov 23 07:47:20 2000
@@ -213,9 +213,7 @@
 &lvm_proc_get_info;
 #endif
 
-#ifdef LVM_HD_NAME
-void lvm_hd_name(char *, int);
-#endif
+char *lvm_hd_name(struct gendisk *, int, char *);
 /* End external function prototypes */
 
 
@@ -230,9 +228,6 @@
 int lvm_snapshot_alloc(lv_t *);
 void lvm_snapshot_release(lv_t *);
 
-#ifdef LVM_HD_NAME
-extern void (*lvm_hd_name_ptr) (char *, int);
-#endif
 static int lvm_map(struct buffer_head *, int);
 static int lvm_do_lock_lvm(void);
 static int lvm_do_le_remap(vg_t *, void *);
@@ -397,11 +392,6 @@
                 lvm_gendisk.next = NULL;
         }
 
-#ifdef LVM_HD_NAME
- /* reference from drivers/block/genhd.c */
- lvm_hd_name_ptr = lvm_hd_name;
-#endif
-
         blk_init_queue(BLK_DEFAULT_QUEUE(MAJOR_NR), DEVICE_REQUEST);
         blk_queue_make_request(BLK_DEFAULT_QUEUE(MAJOR_NR), lvm_make_request_fn);
         blk_queue_pluggable(BLK_DEFAULT_QUEUE(MAJOR_NR), lvm_plug_device_noop);
@@ -460,11 +450,6 @@
         remove_proc_entry(LVM_NAME, &proc_root);
 #endif
 
-#ifdef LVM_HD_NAME
- /* reference from linux/drivers/block/genhd.c */
- lvm_hd_name_ptr = NULL;
-#endif
-
         printk(KERN_INFO "%s -- Module successfully deactivated\n", lvm_name);
 
         return;
@@ -1448,24 +1433,22 @@
  * internal support functions
  */
 
-#ifdef LVM_HD_NAME
 /*
  * generate "hard disk" name
  */
-void lvm_hd_name(char *buf, int minor)
+char *lvm_hd_name(struct gendisk *hd, int minor, char *buf)
 {
         int len = 0;
         lv_t *lv_ptr;
 
         if (vg[VG_BLK(minor)] == NULL ||
             (lv_ptr = vg[VG_BLK(minor)]->lv[LV_BLK(minor)]) == NULL)
- return;
+ return buf;
         len = strlen(lv_ptr->lv_name) - 5;
         memcpy(buf, &lv_ptr->lv_name[5], len);
         buf[len] = 0;
- return;
+ return buf;
 }
-#endif
 
 
 /*
@@ -2515,6 +2498,8 @@
 
         blksize_size[MAJOR_NR] = lvm_blocksizes;
         blk_size[MAJOR_NR] = lvm_size;
+
+ lvm_gendisk.hd_name=lvm_hd_name;
 
         return;
 } /* lvm_gen_init() */
diff -u --recursive linux-2.4.0-test11/drivers/md/md.c linux-2.4.0-test11-ppfix/drivers/md/md.c
--- linux-2.4.0-test11/drivers/md/md.c Mon Nov 20 15:20:30 2000
+++ linux-2.4.0-test11-ppfix/drivers/md/md.c Thu Nov 23 07:56:06 2000
@@ -113,6 +113,8 @@
 extern struct block_device_operations md_fops;
 static devfs_handle_t devfs_handle;
 
+char *md_disk_name(struct gendisk*, int, char *);
+
 static struct gendisk md_gendisk=
 {
         major: MD_MAJOR,
@@ -125,6 +127,7 @@
         real_devices: NULL,
         next: NULL,
         fops: &md_fops,
+ hd_name: md_disk_name,
 };
 
 /*
@@ -215,6 +218,14 @@
         MOD_INC_USE_COUNT;
 
         return mddev;
+}
+
+char * md_disk_name(struct gendisk *hd, int minor, char* buf)
+
+{
+ int unit = (minor >> hd->minor_shift) + '0';
+ sprintf(buf, "%s%c", hd->major_name, unit);
+ return buf;
 }
 
 struct gendisk * find_gendisk (kdev_t dev)
diff -u --recursive linux-2.4.0-test11/drivers/scsi/sd.c linux-2.4.0-test11-ppfix/drivers/scsi/sd.c
--- linux-2.4.0-test11/drivers/scsi/sd.c Mon Nov 20 15:17:18 2000
+++ linux-2.4.0-test11-ppfix/drivers/scsi/sd.c Thu Nov 23 09:53:01 2000
@@ -1013,6 +1013,30 @@
         return i;
 }
 
+char *scsi_disk_name(struct gendisk *hd, int minor, char *buf)
+
+{
+ int unit = (minor >> hd->minor_shift) + 'a';
+ unsigned int part = minor & ((1 << hd->minor_shift) - 1);
+
+ if (hd->major >= SCSI_DISK1_MAJOR && hd->major <= SCSI_DISK7_MAJOR) {
+ unit = unit + (hd->major - SCSI_DISK1_MAJOR + 1) * 16;
+ if (unit > 'z') {
+ unit -= 'z' + 1;
+ sprintf(buf, "sd%c%c", 'a' + unit / 26, 'a' + unit % 26);
+ if (part)
+ sprintf(buf + 4, "%d", part);
+ return buf;
+ }
+ }
+
+ if (part)
+ sprintf(buf, "sd%c%d", unit, part);
+ else
+ sprintf(buf, "sd%c", unit);
+ return buf;
+}
+
 /*
  * The sd_init() function looks at all SCSI drives present, determines
  * their size, and reads partition table entries for them.
@@ -1109,6 +1133,7 @@
                 sd_gendisks[i].next = sd_gendisks + i + 1;
                 sd_gendisks[i].real_devices =
                     (void *) (rscsi_disks + i * SCSI_DISKS_PER_MAJOR);
+ sd_gendisks[i].hd_name=scsi_disk_name;
         }
 
         LAST_SD_GENDISK.next = NULL;
diff -u --recursive linux-2.4.0-test11/fs/partitions/check.c linux-2.4.0-test11-ppfix/fs/partitions/check.c
--- linux-2.4.0-test11/fs/partitions/check.c Mon Nov 20 15:17:27 2000
+++ linux-2.4.0-test11-ppfix/fs/partitions/check.c Thu Nov 23 14:30:45 2000
@@ -83,11 +83,10 @@
  */
 char *disk_name (struct gendisk *hd, int minor, char *buf)
 {
- unsigned int part;
         const char *maj = hd->major_name;
         int unit = (minor >> hd->minor_shift) + 'a';
+ unsigned int part = minor & ((1 << hd->minor_shift) - 1);
 
- part = minor & ((1 << hd->minor_shift) - 1);
         if (hd->part[minor].de) {
                 int pos;
 
@@ -95,77 +94,8 @@
                 if (pos >= 0)
                         return buf + pos;
         }
- /*
- * IDE devices use multiple major numbers, but the drives
- * are named as: {hda,hdb}, {hdc,hdd}, {hde,hdf}, {hdg,hdh}..
- * This requires special handling here.
- */
- switch (hd->major) {
- case IDE9_MAJOR:
- unit += 2;
- case IDE8_MAJOR:
- unit += 2;
- case IDE7_MAJOR:
- unit += 2;
- case IDE6_MAJOR:
- unit += 2;
- case IDE5_MAJOR:
- unit += 2;
- case IDE4_MAJOR:
- unit += 2;
- case IDE3_MAJOR:
- unit += 2;
- case IDE2_MAJOR:
- unit += 2;
- case IDE1_MAJOR:
- unit += 2;
- case IDE0_MAJOR:
- maj = "hd";
- break;
- case MD_MAJOR:
- unit -= 'a'-'0';
- break;
- }
- if (hd->major >= SCSI_DISK1_MAJOR && hd->major <= SCSI_DISK7_MAJOR) {
- unit = unit + (hd->major - SCSI_DISK1_MAJOR + 1) * 16;
- if (unit > 'z') {
- unit -= 'z' + 1;
- sprintf(buf, "sd%c%c", 'a' + unit / 26, 'a' + unit % 26);
- if (part)
- sprintf(buf + 4, "%d", part);
- return buf;
- }
- }
- if (hd->major >= COMPAQ_SMART2_MAJOR && hd->major <= COMPAQ_SMART2_MAJOR+7) {
- int ctlr = hd->major - COMPAQ_SMART2_MAJOR;
- int disk = minor >> hd->minor_shift;
- int part = minor & (( 1 << hd->minor_shift) - 1);
- if (part == 0)
- sprintf(buf, "%s/c%dd%d", maj, ctlr, disk);
- else
- sprintf(buf, "%s/c%dd%dp%d", maj, ctlr, disk, part);
- return buf;
- }
- if (hd->major >= COMPAQ_CISS_MAJOR && hd->major <= COMPAQ_CISS_MAJOR+7) {
- int ctlr = hd->major - COMPAQ_CISS_MAJOR;
- int disk = minor >> hd->minor_shift;
- int part = minor & (( 1 << hd->minor_shift) - 1);
- if (part == 0)
- sprintf(buf, "%s/c%dd%d", maj, ctlr, disk);
- else
- sprintf(buf, "%s/c%dd%dp%d", maj, ctlr, disk, part);
- return buf;
- }
- if (hd->major >= DAC960_MAJOR && hd->major <= DAC960_MAJOR+7) {
- int ctlr = hd->major - DAC960_MAJOR;
- int disk = minor >> hd->minor_shift;
- int part = minor & (( 1 << hd->minor_shift) - 1);
- if (part == 0)
- sprintf(buf, "%s/c%dd%d", maj, ctlr, disk);
- else
- sprintf(buf, "%s/c%dd%dp%d", maj, ctlr, disk, part);
- return buf;
- }
+ if (hd->hd_name) return hd->hd_name(hd, minor, buf);
+
         if (part)
                 sprintf(buf, "%s%c%d", maj, unit, part);
         else
diff -u --recursive linux-2.4.0-test11/include/linux/genhd.h linux-2.4.0-test11-ppfix/include/linux/genhd.h
--- linux-2.4.0-test11/include/linux/genhd.h Mon Nov 20 15:31:01 2000
+++ linux-2.4.0-test11-ppfix/include/linux/genhd.h Thu Nov 23 07:44:35 2000
@@ -72,6 +72,8 @@
 
         devfs_handle_t *de_arr; /* one per physical disc */
         char *flags; /* one per physical disc */
+
+ char *(*hd_name) (struct gendisk *, int, char *);
 };
 #endif /* __KERNEL__ */
 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Nov 23 2000 - 21:00:26 EST