NetBSD Problem Report #25108

Received: (qmail 27457 invoked by uid 605); 8 Apr 2004 18:10:18 -0000
Message-Id: <E1BBdyi-0005xz-N9@absd.org>
Date: Thu, 08 Apr 2004 19:10:16 +0100
From: abs@absd.org
Sender: gnats-bugs-owner@NetBSD.org
Reply-To: abs@absd.org
To: gnats-bugs@gnats.NetBSD.org
Subject: autogenerated disklabel incorrect for CD
X-Send-Pr-Version: 3.95

>Number:         25108
>Category:       kern
>Synopsis:       autogenerated disklabel incorrect for CD
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Apr 08 18:11:00 +0000 2004
>Closed-Date:    
>Last-Modified:  Fri May 07 17:17:00 +0000 2004
>Originator:     David Brownlee
>Release:        NetBSD 2.0_BETA
>Organization:
>Environment:


System: NetBSD forsaken 2.0_BETA NetBSD 2.0_BETA (_FORSAKEN_) #0: Wed Apr 7 18:27:43 BST 2004 abs@tll.i.purplei.com:/var/obj/i386/files/netbsd/2.0/sys/arch/i386/compile/_FORSAKEN_ i386
Architecture: i386
Machine: i386
>Description:
        On 1.6.x a 'disklabel' of an ISO9660 cd reports partition 'a' as
        ISO9660, while on under 2.0_BETA it is reported as 4.2BSD.

        1.6.x:

# /dev/rcd1d:
type: ATAPI
disk: LITE-ON LTR-4812
[...]
4 partitions:
#        size    offset     fstype  [fsize bsize cpg/sgs]
 a:   1196844         0    ISO9660                      # (Cyl.    0 -  11968*)
 d:   1196844         0    ISO9660                      # (Cyl.    0 -  11968*)

        2.0_BETA:

# /dev/rcd0d:
type: ATAPI
disk: mydisc
label: fictitious
[...]
4 partitions:
#        size    offset     fstype [fsize bsize cpg/sgs]
 a:   1196844         0     4.2BSD      0     0     0  # (Cyl.      0 - 11968*)
 d:   1196844         0    ISO9660       0             # (Cyl.      0 - 11968*)

>How-To-Repeat:

>Fix:
>Release-Note:
>Audit-Trail:

From: David Laight <david@l8s.co.uk>
To: gnats-bugs@gnats.netbsd.org
Cc: tech-kern@netbsd.org
Subject: Re: kern/25108: Default disklabel for cd(4)
Date: Thu, 6 May 2004 21:50:31 +0100

 On Thu, May 06, 2004 at 09:45:00AM +0200, Quentin Garnier wrote:
 (but didn't add to the gnats entry...)
 > Hi all,
 > 
 > There is a bug in the way readdisklabel() interacts with device label
 > generation routines in lower level driver.  It is clearly apparent with
 > cd(4), but might as well happen with other devices for which it can be
 > completely normal to have neither a disklabel nor a MBR.
 > 
 > kern/subr_disk_mbr.c is not used by all archs, only by amd64, i386 and
 > ibmnws as it seems.  So the issue as I will describe it only happens on
 > those archs.  That doesn't mean some other archs don't have their
 > readdisklabel() bugged, too.

 FWIW I added subr_disk_mbr.c in order to start the process of commoning
 up a load of code that should never have got itself MD...

 > When cd(4) wants to generate a disklabel, it calls cdgetdisklabel().
 > That function first tries to call readdisklabel(), and if that function
 > returns an error, it outputs it on the console and use a default label.
 > 
 > On i386 (I confirm it does about the same on -1-6, too), readdisklabel()
 > tries to find a MBR first, then tries to directly read a disklabel on
 > the device.
 > 
 > The difference between -1-6 and -current from after July, 7th, 2003, is
 > that the MI readdisklabel() does not return an error if it couldn't read
 > neither a MBR not a disklabel.
 > 
 > That avoids the message "cd0: no disk label" we can see on our -1-6
 > hosts, but that makes cdgetdisklabel immediately return, not generating
 > a correct label.
 > 
 > The reason why disklabel shows 4.2BSD for 'a' is that the MI
 > readdisklabel sets that by default, in case it doesn't find a MBR or a
 > label.

 Yes - otherwise everything suffers from serious grief since nothing
 excpect to have to use cd0d.

 Before I wrote subr_disk_mbr.c all the partitions used to get set to
 cover the whole disk....

 > I think this is not a correct behaviour, and even though I agree we
 > should avoid printing an error message when the case of actually having
 > a MBR or a label on a CD-ROM would be very unexpected.
 > 
 > My proposal is the following:  making readdisklabel() return an empty
 > string in case no relevant data was found while no error happened. Then
 > the device routine that calls readdisklabel() can check the length of
 > the returned message and not print it if it is empty.

 That just adds code to all the drivers - which just isn't (and shouldn't be)
 necessary.

 > Somehow I wonder what happens for devices that first calls their default
 > labelling function, and then call readdisklabel(), which inconditionally
 > overwrites type for 'a'.  I'd like some input on what to do here, too.

 The obvious thing is for the default label (returned by readdisklabel)
 to contain the fs_type from the raw partition in entry 'a'.

 This patch should suffice.

 	David

 Index: subr_disk_mbr.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/subr_disk_mbr.c,v
 retrieving revision 1.4
 diff -u -p -r1.4 subr_disk_mbr.c
 --- subr_disk_mbr.c	8 Oct 2003 04:25:46 -0000	1.4
 +++ subr_disk_mbr.c	6 May 2004 20:32:59 -0000
 @@ -225,13 +225,15 @@ readdisklabel(dev, strat, lp, osdep)
  	if (lp->d_partitions[RAW_PART].p_size == 0)
  		lp->d_partitions[RAW_PART].p_size = 0x1fffffff;
  	lp->d_partitions[RAW_PART].p_offset = 0;
 +	if (lp->d_partitions[RAW_PART].p_fstype == FS_UNUSED)
 +		lp->d_partitions[RAW_PART].p_fstype = FS_BSDFFS;

  	/*
  	 * Set partition 'a' to be the whole disk.
  	 * Cleared if we find an mbr or a netbsd label.
  	 */
  	lp->d_partitions[0].p_size = lp->d_partitions[RAW_PART].p_size;
 -	lp->d_partitions[0].p_fstype = FS_BSDFFS;
 +	lp->d_partitions[0].p_fstype = lp->d_partitions[RAW_PART].p_fstype;

  	/* get a buffer and initialize it */
  	a.bp = geteblk((int)lp->d_secsize);

 -- 
 David Laight: david@l8s.co.uk

From: Manuel Bouyer <bouyer@antioche.lip6.fr>
To: gnats-bugs@gnats.netbsd.org, tech-kern@netbsd.org
Cc:  
Subject: Re: kern/25108: Default disklabel for cd(4)
Date: Fri, 7 May 2004 11:10:36 +0200

 On Thu, May 06, 2004 at 09:50:31PM +0100, David Laight wrote:
 > 
 > That just adds code to all the drivers - which just isn't (and shouldn't be)
 > necessary.
 > 
 > > Somehow I wonder what happens for devices that first calls their default
 > > labelling function, and then call readdisklabel(), which inconditionally
 > > overwrites type for 'a'.  I'd like some input on what to do here, too.
 > 
 > The obvious thing is for the default label (returned by readdisklabel)
 > to contain the fs_type from the raw partition in entry 'a'.
 > 
 > This patch should suffice.
 > 
 > 	David
 > 
 > Index: subr_disk_mbr.c
 > ===================================================================
 > RCS file: /cvsroot/src/sys/kern/subr_disk_mbr.c,v
 > retrieving revision 1.4
 > diff -u -p -r1.4 subr_disk_mbr.c
 > --- subr_disk_mbr.c	8 Oct 2003 04:25:46 -0000	1.4
 > +++ subr_disk_mbr.c	6 May 2004 20:32:59 -0000
 > @@ -225,13 +225,15 @@ readdisklabel(dev, strat, lp, osdep)
 >  	if (lp->d_partitions[RAW_PART].p_size == 0)
 >  		lp->d_partitions[RAW_PART].p_size = 0x1fffffff;
 >  	lp->d_partitions[RAW_PART].p_offset = 0;
 > +	if (lp->d_partitions[RAW_PART].p_fstype == FS_UNUSED)
 > +		lp->d_partitions[RAW_PART].p_fstype = FS_BSDFFS;

 Does this mean that the raw partition will always be of type 4.2BSD ?
 I think it should remain as unused.

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: David Laight <david@l8s.co.uk>
To: Manuel Bouyer <bouyer@antioche.lip6.fr>
Cc: gnats-bugs@gnats.netbsd.org, tech-kern@netbsd.org
Subject: Re: kern/25108: Default disklabel for cd(4)
Date: Fri, 7 May 2004 18:24:18 +0100

 > > +	if (lp->d_partitions[RAW_PART].p_fstype == FS_UNUSED)
 > > +		lp->d_partitions[RAW_PART].p_fstype = FS_BSDFFS;
 > 
 > Does this mean that the raw partition will always be of type 4.2BSD ?
 > I think it should remain as unused.

 You are right!  I was mislead by noticing the the cd driver sets
 the raw partition to FS_ISO9660 (or whatever the ident is).

 That check would need to go into the entry for ptn 'a'.
 (or maybe get the default from the original entry for 'a').

 Of course the ptn type doesn't matter (much) and won't stop a mount.

 	David

 -- 
 David Laight: david@l8s.co.uk
>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.