Subject: multisession CD support (again) - please review
To: None <tech-kern@netbsd.org>
From: Matthias Drochner <M.Drochner@fz-juelich.de>
List: tech-kern
Date: 05/25/2002 16:13:45
Now that the branch is cut...

As I wrote in a mail some weeks ago, I'd like to access data
stored in old sessions on a CD.
This is handy if CDs are used as backup media - so I can recover
a file which was overwritten in the meantime -, and as a fallback
after botching a session (this is what happened to me just before
that mail).

Our current multisession CD support consists of a somewhat specific
call from the cd9660 filesystem code to the cd device driver
(CDIOREADMSADDR), which makes that always the most recent session
is used.

FreeBSD adds a mount argument to the cd9660 filesystem code to
pass the sector offset of the desired session.

As far as I can tell, finding the offsets of sessions other
then the last (which we already do) and the very first (0 - trivial)
is hard to do portably. There are corner cases like unclosed sessions
and probably a number of quirky drives. This is a good reason to
keep offset calculation out of the kernel. (Otherwise, one would pass
a session index, so the caller wouldn't have to deal with the
details of the CD layout.)

I'd propose a solution which is more aesthetic imho:
using the (in-core) disklabel to pass the offset to the filesystem
code. This avoids the "specific call" into the CD driver. For the
reasons just told, the CD driver would only set up partition
entries for the first and the last session. User programs however
can manipulate the disklabel, just as already done by "mbrlabel",
and add entries for more sessions.

The only field in the disklabel's partition structure which
is wide enough and not already used by cd9660 is "p_fsize".

The appended patches implement the proposed change. Can you please
tell me whether there are problems with this approach?

best regards
Matthias


Index: sys/disklabel.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/disklabel.h,v
retrieving revision 1.70
diff -u -r1.70 disklabel.h
--- sys/disklabel.h	2002/03/27 19:07:32	1.70
+++ sys/disklabel.h	2002/05/25 13:19:11
@@ -180,7 +180,13 @@
 	struct	partition {		/* the partition table */
 		u_int32_t p_size;	/* number of sectors in partition */
 		u_int32_t p_offset;	/* starting sector */
-		u_int32_t p_fsize;	/* filesystem basic fragment size */
+		union {
+			u_int32_t fsize; /* FFS, ADOS:
+					    filesystem basic fragment size */
+			u_int32_t cdsession; /* ISO9660: session offset */
+		} __partition_u2;
+#define	p_fsize		__partition_u2.fsize
+#define	p_cdsession	__partition_u2.cdsession
 		u_int8_t p_fstype;	/* filesystem type, see below */
 		u_int8_t p_frag;	/* filesystem fragments per block */
 		union {
@@ -235,7 +241,10 @@
 	struct	opartition {
 		u_int32_t p_size;
 		u_int32_t p_offset;
-		u_int32_t p_fsize;
+		union {
+			u_int32_t fsize;
+			u_int32_t cdsession;
+		} __partition_u2;
 		u_int8_t p_fstype;
 		u_int8_t p_frag;
 		union {
Index: isofs/cd9660/cd9660_vfsops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/isofs/cd9660/cd9660_vfsops.c,v
retrieving revision 1.60
diff -u -r1.60 cd9660_vfsops.c
--- isofs/cd9660/cd9660_vfsops.c	2002/03/26 19:04:23	1.60
+++ isofs/cd9660/cd9660_vfsops.c	2002/05/25 13:19:11
@@ -309,7 +309,8 @@
 	struct iso_supplementary_descriptor *sup;
 	int sess = 0;
 	int ext_attr_length;
-	
+	struct disklabel label;
+
 	if (!ronly)
 		return EROFS;
 	
@@ -337,14 +338,22 @@
 	 */
 	iso_bsize = ISO_DEFAULT_BLOCK_SIZE;
 
-	error = VOP_IOCTL(devvp, CDIOREADMSADDR, (caddr_t)&sess, 0, FSCRED, p);
-	if (error)
-		sess = 0;	/* never mind */
-#if 0
-	else
-		printf("cdfs: detected multi-session at offset %d\n", sess);
+	error = VOP_IOCTL(devvp, DIOCGDINFO, (caddr_t)&label, FREAD, FSCRED, p);
+	if (!error &&
+	    label.d_partitions[DISKPART(dev)].p_fstype == FS_ISO9660) {
+		/* XXX more sanity checks? */
+		sess = label.d_partitions[DISKPART(dev)].p_cdsession;
+	} else {
+		/* fallback to old method */
+		error = VOP_IOCTL(devvp, CDIOREADMSADDR, (caddr_t)&sess, 0,
+				  FSCRED, p);
+		if (error)
+			sess = 0;	/* never mind */
+	}
+#ifdef DEBUG
+	printf("isofs: session offset (part %d) %d\n", DISKPART(dev), sess);
 #endif
-	
+
 	for (iso_blknum = 16; iso_blknum < 100; iso_blknum++) {
 		if ((error = bread(devvp, (iso_blknum+sess) * btodb(iso_bsize),
 				   iso_bsize, NOCRED, &bp)) != 0)
Index: dev/scsipi/cd.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/scsipi/cd.c,v
retrieving revision 1.162
diff -u -r1.162 cd.c
--- dev/scsipi/cd.c	2002/05/05 15:16:31	1.162
+++ dev/scsipi/cd.c	2002/05/25 13:19:12
@@ -129,6 +129,7 @@
 int	cd_read_toc __P((struct cd_softc *, int, int, void *, int, int, int));
 int	cd_get_parms __P((struct cd_softc *, int));
 int	cd_load_toc __P((struct cd_softc *, struct cd_toc *, int));
+int	cdreadmsaddr __P((struct cd_softc *, int *));
 int	dvd_auth __P((struct cd_softc *, dvd_authinfo *));
 int	dvd_read_physical __P((struct cd_softc *, dvd_struct *));
 int	dvd_read_copyright __P((struct cd_softc *, dvd_struct *));
@@ -1043,6 +1044,37 @@
 	return ((((m * CD_SECS) + s) * CD_FRAMES + f) - CD_BLOCK_OFFSET);
 }
 
+int
+cdreadmsaddr(cd, addr)
+	struct cd_softc *cd;
+	int *addr;
+{
+	struct scsipi_periph *periph = cd->sc_periph;
+	int error;
+	struct cd_toc toc;
+	struct cd_toc_entry *cte;
+
+	error = cd_read_toc(cd, 0, 0, &toc,
+	    sizeof(struct ioc_toc_header) + sizeof(struct cd_toc_entry),
+	    XS_CTL_DATA_ONSTACK,
+	    0x40 /* control word for "get MS info" */);
+
+	if (error)
+		return (error);
+
+	cte = &toc.entries[0];
+	if (periph->periph_quirks & PQUIRK_LITTLETOC) {
+		cte->addr.lba = le32toh(cte->addr.lba);
+		toc.header.len = le16toh(toc.header.len);
+	} else {
+		cte->addr.lba = be32toh(cte->addr.lba);
+		toc.header.len = be16toh(toc.header.len);
+	}
+
+	*addr = (toc.header.len >= 10 && cte->track > 1) ?
+		cte->addr.lba : 0;
+	return 0;
+}
 
 /*
  * Perform special action on behalf of the user.
@@ -1274,33 +1306,12 @@
 		return (copyout(toc.entries, te->data, len));
 	}
 	case CDIOREADMSADDR: {
-		struct cd_toc toc;
 		int sessno = *(int*)addr;
-		struct cd_toc_entry *cte;
 
 		if (sessno != 0)
 			return (EINVAL);
-
-		error = cd_read_toc(cd, 0, 0, &toc,
-		  sizeof(struct ioc_toc_header) + sizeof(struct cd_toc_entry),
-		  XS_CTL_DATA_ONSTACK,
-		  0x40 /* control word for "get MS info" */);
-
-		if (error)
-			return (error);
 
-		cte = &toc.entries[0];
-		if (periph->periph_quirks & PQUIRK_LITTLETOC) {
-			cte->addr.lba = le32toh(cte->addr.lba);
-			toc.header.len = le16toh(toc.header.len);
-		} else {
-			cte->addr.lba = be32toh(cte->addr.lba);
-			toc.header.len = be16toh(toc.header.len);
-		}
-
-		*(int*)addr = (toc.header.len >= 10 && cte->track > 1) ?
-			cte->addr.lba : 0;
-		return 0;
+		return (cdreadmsaddr(cd, (int*)addr));
 	}
 	case CDIOCSETPATCH: {
 		struct ioc_patch *arg = (struct ioc_patch *)addr;
@@ -1414,6 +1425,7 @@
 	struct cd_softc *cd;
 	struct disklabel *lp;
 {
+	int lastsession;
 
 	memset(lp, 0, sizeof(struct disklabel));
 
@@ -1442,13 +1454,25 @@
 	lp->d_interleave = 1;
 	lp->d_flags = D_REMOVABLE;
 
+	if (cdreadmsaddr(cd, &lastsession) != 0)
+		lastsession = 0;
+
 	lp->d_partitions[0].p_offset = 0;
+#ifdef notyet /* have to fix bounds_check_with_label() first */
+	lp->d_partitions[0].p_size = lp->d_secperunit;
+#else
 	lp->d_partitions[0].p_size =
 	    lp->d_secperunit * (lp->d_secsize / DEV_BSIZE);
+#endif
+	lp->d_partitions[0].p_cdsession = lastsession;
 	lp->d_partitions[0].p_fstype = FS_ISO9660;
 	lp->d_partitions[RAW_PART].p_offset = 0;
+#ifdef notyet
+	lp->d_partitions[RAW_PART].p_size = lp->d_secperunit;
+#else
 	lp->d_partitions[RAW_PART].p_size =
 	    lp->d_secperunit * (lp->d_secsize / DEV_BSIZE);
+#endif
 	lp->d_partitions[RAW_PART].p_fstype = FS_ISO9660;
 	lp->d_npartitions = RAW_PART + 1;