Subject: Re: kern/25108: Default disklabel for cd(4)
To: None <firstname.lastname@example.org>
From: David Laight <email@example.com>
Date: 05/06/2004 21:50:31
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
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)
> 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.
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.p_size = lp->d_partitions[RAW_PART].p_size;
- lp->d_partitions.p_fstype = FS_BSDFFS;
+ lp->d_partitions.p_fstype = lp->d_partitions[RAW_PART].p_fstype;
/* get a buffer and initialize it */
a.bp = geteblk((int)lp->d_secsize);
David Laight: firstname.lastname@example.org