[PATCH] 2.6.2, Partition support for SCSI CDROM...

[PATCH] 2.6.2, Partition support for SCSI CDROM...

[Steven J. Hill]

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

Greetings.

This patch enables support for CDROMs that have partitions on
them, like say SGI and SUN media. It was sent to me by Christoph
Hellwig and then I cleaned it up a bit. I am posting it more
for flamebait^Wcomments to see if people are comfortable with it.
Thanks.

-Steve

[-- Attachment #2: scsi-sr-partitions-2.6.2.patch --]
[-- Type: text/x-patch, Size: 3157 bytes --]

diff -urN linux-2.6.2/drivers/scsi/Kconfig linux-2.6.2-patched/drivers/scsi/Kconfig
--- linux-2.6.2/drivers/scsi/Kconfig	2004-02-22 20:15:07.000000000 -0500
+++ linux-2.6.2-patched/drivers/scsi/Kconfig	2004-02-08 20:29:04.000000000 -0500
@@ -112,6 +112,48 @@
 	  drives (and HP Writers). If you have such a drive and get the first
 	  session only, try saying Y here; everybody else says N.
 
+config BLK_DEV_SR_PARTITIONS
+	bool "Enable partitions (for SCSI CDROM)"
+	depends on BLK_DEV_SR
+	help
+	  This enables the utilisation of partitions on CDs, usually
+	  media from SGI or Sun. You must enable "SGI partition support"
+	  and/or "Sun partition tables support" to be able to see their
+	  respective CD partitions. Also make sure to say Y or M to
+	  "EFS file system support" and "UFS file system support" to
+	  support reading of SGI and Sun filesystems.
+	  
+	  If you want to this feature, say Y here; everybody else will
+	  most likely say N.
+
+config BLK_DEV_SR_PARTITIONS_PER_DEVICE
+	int "Number of paritions supported per SCSI CDROM device"
+	depends on BLK_DEV_SR_PARTITIONS
+	default "0"
+	help
+	  The number of partitions supported per SCSI CDROM device. If
+	  the value chosen was 7, you will have the following device
+	  node mappings:
+
+	     sr0 - first CDROM, whole disk
+	     sr1 - first CDROM, first partition
+	     
+	     [...]
+	     
+	     sr7 - first CDROM, seventh partition
+	     sr8 - second CDROM, whole disk
+	     sr9 - second CDROM, first partition
+
+	     [...]
+
+	  You made need to create additional device nodes depending on
+	  the number of partitions and SCSI CDROM devices you have in
+	  your system.
+	  
+	  If SCSI CDROM support is compiled as a module, you can specify
+	  how many partitions you want when you insert the module which
+	  will override this default value.
+	  
 config CHR_DEV_SG
 	tristate "SCSI generic support"
 	depends on SCSI
diff -urN linux-2.6.2/drivers/scsi/sr.c linux-2.6.2-patched/drivers/scsi/sr.c
--- linux-2.6.2/drivers/scsi/sr.c	2004-02-22 20:15:08.000000000 -0500
+++ linux-2.6.2-patched/drivers/scsi/sr.c	2004-02-08 20:31:52.000000000 -0500
@@ -55,6 +55,13 @@
 #include "scsi_logging.h"
 #include "sr.h"
 
+#ifdef CONFIG_BLK_DEV_SR_PARTITIONS
+static int partitions = CONFIG_BLK_DEV_SR_PARTITIONS_PER_DEVICE;
+MODULE_PARM(partitions, "i");
+MODULE_PARM_DESC(partitions, "number of SCSI CDROM partitions to support");
+#else
+static int partitions = 0;
+#endif
 
 MODULE_PARM(xa_test, "i");	/* see sr_ioctl.c */
 
@@ -518,7 +525,7 @@
 		goto fail;
 	memset(cd, 0, sizeof(*cd));
 
-	disk = alloc_disk(1);
+	disk = alloc_disk(partitions + 1);
 	if (!disk)
 		goto fail_free;
 
@@ -533,7 +540,7 @@
 	spin_unlock(&sr_index_lock);
 
 	disk->major = SCSI_CDROM_MAJOR;
-	disk->first_minor = minor;
+	disk->first_minor = minor * (partitions + 1);
 	sprintf(disk->disk_name, "sr%d", minor);
 	disk->fops = &sr_bdops;
 	disk->flags = GENHD_FL_CD;
@@ -868,6 +875,12 @@
 {
 	int rc;
 
+#ifdef MODULE
+	/* Check number of partitions specified. */
+	if (partitions < 0)
+		partitions = 0;
+#endif
+
 	rc = register_blkdev(SCSI_CDROM_MAJOR, "sr");
 	if (rc)
 		return rc;

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Andrew Morton]

"Steven J. Hill" <sjhill@realitydiluted.com> wrote:
>
> Greetings.
> 
> This patch enables support for CDROMs that have partitions on
> them, like say SGI and SUN media. It was sent to me by Christoph
> Hellwig and then I cleaned it up a bit. I am posting it more
> for flamebait^Wcomments to see if people are comfortable with it.

> +config BLK_DEV_SR_PARTITIONS
> +config BLK_DEV_SR_PARTITIONS_PER_DEVICE

Do we actually need these config options?  Why not hardwire it to some
reasonable upper bound and be done with it?

>  
> +#ifdef MODULE
> +	/* Check number of partitions specified. */
> +	if (partitions < 0)
> +		partitions = 0;
> +#endif
> +

Why is this ifdef needed?

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Steven J. Hill]

Andrew Morton wrote:
> 
>>+config BLK_DEV_SR_PARTITIONS
>>+config BLK_DEV_SR_PARTITIONS_PER_DEVICE
> 
> 
> Do we actually need these config options?  Why not hardwire it to some
> reasonable upper bound and be done with it?
>
I have no problem hardwiring the number of partitions, but the
BLK_DEV_SR_PARTITIONS should still be an option to allow the
user to decided if they want partitioning support for their
SCSI CDROMs. Or are you suggesting that from now on partitions
will be supported by default?

> Why is this ifdef needed?
> 
Indeed, it is pointless.

-Steve

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Andrew Morton]

"Steven J. Hill" <sjhill@realitydiluted.com> wrote:
>
> Andrew Morton wrote:
> > 
> >>+config BLK_DEV_SR_PARTITIONS
> >>+config BLK_DEV_SR_PARTITIONS_PER_DEVICE
> > 
> > 
> > Do we actually need these config options?  Why not hardwire it to some
> > reasonable upper bound and be done with it?
> >
> I have no problem hardwiring the number of partitions, but the
> BLK_DEV_SR_PARTITIONS should still be an option to allow the
> user to decided if they want partitioning support for their
> SCSI CDROMs. Or are you suggesting that from now on partitions
> will be supported by default?

Well we need to be able to handle both types at runtime anyway, and the
amount of added code is tiny.

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Jeremy Higdon]

On Sun, Feb 22, 2004 at 10:07:59PM -0500, Steven J. Hill wrote:
> Andrew Morton wrote:
> >
> >>+config BLK_DEV_SR_PARTITIONS
> >>+config BLK_DEV_SR_PARTITIONS_PER_DEVICE
> >
> >
> >Do we actually need these config options?  Why not hardwire it to some
> >reasonable upper bound and be done with it?
> >
> I have no problem hardwiring the number of partitions, but the
> BLK_DEV_SR_PARTITIONS should still be an option to allow the
> user to decided if they want partitioning support for their
> SCSI CDROMs. Or are you suggesting that from now on partitions
> will be supported by default?

A couple of comments on this.

First, I've seen CDs in which the capacity reported by the CD is
actually slightly greater than the number of burned sectors.  In
that case, you'll get errors during the partition scanning.  You
can just ignore the errors, however, so it's no big deal.

Also, the default should probably be 16 partitions, since that
is what's supported on SGI CDs.  Most SGI CDs are in ISO format,
but Irix installation CDs do have an SGI partition table.

jeremy

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Steven J. Hill]

[-- Attachment #1: Type: text/plain, Size: 45 bytes --]

Here is the second try at the patch.

-Steve

[-- Attachment #2: scsi-sr-partitions-2.6.2-2.patch --]
[-- Type: text/x-patch, Size: 1402 bytes --]

diff -urN linux-2.6.2/drivers/scsi/sr.c linux-2.6.2-patched/drivers/scsi/sr.c
--- linux-2.6.2/drivers/scsi/sr.c	2004-02-22 20:15:08.000000000 -0500
+++ linux-2.6.2-patched/drivers/scsi/sr.c	2004-02-24 11:48:16.000000000 -0500
@@ -55,6 +55,23 @@
 #include "scsi_logging.h"
 #include "sr.h"
 
+/*
+ * Device node mappings are as follows:
+ *
+ *    sr0 - first CDROM, whole disk
+ *    sr1 - first CDROM, first partition
+ *
+ *    [...]
+ *
+ *    sr16 - first CDROM, sixteenth partition
+ *    sr17 - second CDROM, whole disk
+ *    sr18 - second CDROM, first partition
+ *
+ *    [...]
+ */
+static int partitions = 16;
+MODULE_PARM(partitions, "i");
+MODULE_PARM_DESC(partitions, "number of SCSI CDROM partitions to support");
 
 MODULE_PARM(xa_test, "i");	/* see sr_ioctl.c */
 
@@ -518,7 +535,7 @@
 		goto fail;
 	memset(cd, 0, sizeof(*cd));
 
-	disk = alloc_disk(1);
+	disk = alloc_disk(partitions + 1);
 	if (!disk)
 		goto fail_free;
 
@@ -533,7 +550,7 @@
 	spin_unlock(&sr_index_lock);
 
 	disk->major = SCSI_CDROM_MAJOR;
-	disk->first_minor = minor;
+	disk->first_minor = minor * (partitions + 1);
 	sprintf(disk->disk_name, "sr%d", minor);
 	disk->fops = &sr_bdops;
 	disk->flags = GENHD_FL_CD;
@@ -868,6 +885,10 @@
 {
 	int rc;
 
+	/* Check number of partitions specified. */
+	if (partitions < 0)
+		partitions = 0;
+
 	rc = register_blkdev(SCSI_CDROM_MAJOR, "sr");
 	if (rc)
 		return rc;

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Christoph Hellwig]

On Tue, Feb 24, 2004 at 11:51:20AM -0500, Steven J. Hill wrote:
> Here is the second try at the patch.
> 
> -Steve


+static int partitions = 16;

This is changes what sr1 is mapped to without specicying any option.
The default _must_ be 0 partitions or existing setups will break.

+MODULE_PARM(partitions, "i");

please make this module_param so it works at boot-time aswell.

+MODULE_PARM_DESC(partitions, "number of SCSI CDROM partitions to support");
 
+	/* Check number of partitions specified. */
+	if (partitions < 0)
+		partitions = 0;

now if you made the variable 'unsigned' you wouldn't have that problem..

While you're at it please also cook up an ide-cd variant, having partitions
only supported on scsi cdroms is more than confusing.

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Matthew Wilcox]

On Tue, Feb 24, 2004 at 11:51:20AM -0500, Steven J. Hill wrote:
> +/*
> + * Device node mappings are as follows:
> + *
> + *    sr0 - first CDROM, whole disk
> + *    sr1 - first CDROM, first partition
> + *
> + *    [...]
> + *
> + *    sr16 - first CDROM, sixteenth partition
> + *    sr17 - second CDROM, whole disk
> + *    sr18 - second CDROM, first partition

Umm... no.  I suspect you mean:

sr15 - first CDROM, fifteenth partition
sr16 - second CDROM, whole disk
sr17 - second CDROM, first partition

But what a bad idea for device names.  Why not

sr0 whole disc
sr0a ... sr0o partitions
sr1, sr1a ... sr1o

It's probably too late to be consistent with discs and call them
sra, sra1, ... sra15
srb, srb1, ... srb15

> + *    [...]
> + */
> +static int partitions = 16;

15.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Miles Bader]

Matthew Wilcox <willy@debian.org> writes:
> sr0 whole disc
> sr0a ... sr0o partitions
> sr1, sr1a ... sr1o
> 
> It's probably too late to be consistent with discs and call them
> sra, sra1, ... sra15
> srb, srb1, ... srb15

The (BSDish) xx0a convention is arguably better anyway (because the
various parts of the name split naturally along the letter-digit
boundaries); I never quite figured out why linux used the convention it
does for disks.

[Of course consistency generally wins out over niceness, but where
consistency isn't an option...]

-Miles
-- 
`The suburb is an obsolete and contradictory form of human settlement'

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Andries Brouwer]

On Tue, Feb 24, 2004 at 05:09:06PM +0000, Matthew Wilcox wrote:
> On Tue, Feb 24, 2004 at 11:51:20AM -0500, Steven J. Hill wrote:

> > + *    sr0 - first CDROM, whole disk
> > + *    sr1 - first CDROM, first partition
> > + *
> > + *    [...]
> > + *
> > + *    sr16 - first CDROM, sixteenth partition
> > + *    sr17 - second CDROM, whole disk
> > + *    sr18 - second CDROM, first partition
> 
> Umm... no.  I suspect you mean:
> 
> sr15 - first CDROM, fifteenth partition
> sr16 - second CDROM, whole disk
> sr17 - second CDROM, first partition
> 
> But what a bad idea for device names.  Why not
> 
> sr0 whole disc
> sr0a ... sr0o partitions
> sr1, sr1a ... sr1o
> 
> It's probably too late to be consistent with discs and call them
> sra, sra1, ... sra15
> srb, srb1, ... srb15

It is standard convention to use numerical suffixes to refer
to partitions, with a 'p' separator in case the full device
has a name ending in a digit.

So: sr0p1, ..., sr0p15, sr1p1, ...

Andries

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Rusty Russell]

In message <20040224170626.A25066@infradead.org> you write:
> +MODULE_PARM(partitions, "i");
> 
> please make this module_param so it works at boot-time aswell.

But beware: there's another MODULE_PARM in the same module.  You can't
mix them, you'll need to find and fix that too.

Thanks,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] 2.6.2, Partition support for SCSI CDROM...

[Bill Davidsen]

Andries Brouwer wrote:
> On Tue, Feb 24, 2004 at 05:09:06PM +0000, Matthew Wilcox wrote:
> 
>>On Tue, Feb 24, 2004 at 11:51:20AM -0500, Steven J. Hill wrote:
> 
> 
>>>+ *    sr0 - first CDROM, whole disk
>>>+ *    sr1 - first CDROM, first partition
>>>+ *
>>>+ *    [...]
>>>+ *
>>>+ *    sr16 - first CDROM, sixteenth partition
>>>+ *    sr17 - second CDROM, whole disk
>>>+ *    sr18 - second CDROM, first partition
>>
>>Umm... no.  I suspect you mean:
>>
>>sr15 - first CDROM, fifteenth partition
>>sr16 - second CDROM, whole disk
>>sr17 - second CDROM, first partition
>>
>>But what a bad idea for device names.  Why not
>>
>>sr0 whole disc
>>sr0a ... sr0o partitions
>>sr1, sr1a ... sr1o
>>
>>It's probably too late to be consistent with discs and call them
>>sra, sra1, ... sra15
>>srb, srb1, ... srb15
> 
> 
> It is standard convention to use numerical suffixes to refer
> to partitions, with a 'p' separator in case the full device
> has a name ending in a digit.
> 
> So: sr0p1, ..., sr0p15, sr1p1, ...

That sounds a LOT better. Of course mknod is your friend, and some of us 
have sr0, sr1 etc, but that's our problem. I think one of the distros 
does it that way, but not one I have here.

-- 
bill davidsen <davidsen@tmr.com>
   CTO TMR Associates, Inc
   Doing interesting things with small computers since 1979