> I think I finally figured out where the problem comes from. > > The "- LABELSECTOR" on line 144 is indeed causing it and should be > removed. But it has been added here for a reason: to balance the "+ > LABELSECTOR" in the writedisklabel function below, when this part of the > code was factorized years ago > <http://www.openbsd.org/cgi-bin/cvsweb/src/sys/arch/macppc/macppc/disksubr.c.diff?r1=1.55;r2=1.56;f=h>
Right. > As far as I can tell, LABELSECTOR is an MBR related constant and > probably shouldn't have been put in a DPME code in the first place. Not quite. LABELSECTOR is not a MBR related constant. It is a machine-dependent block offset from the start-of-openbsd-area where the native disklabel is placed. It exists on MBR-based machines, but it also exists on machines which do not use MBR's. In this case, if we are reading a non-native label though (and then spoofing a label from that), LABELSECTOR is invalid. So your diff to readdpmelabel() looks correct. > But > in practice, at that time it wasn't a problem since the "hfspartoff" > variable was only used (indirectly) by the writedisklabel function. > Unfortunately this was no longer the case when bounds were added > <http://www.openbsd.org/cgi-bin/cvsweb/src/sys/arch/macppc/macppc/disksubr.c.diff?r1=1.61;r2=1.63;f=h> I think you have also done the right thing for the writedisklabel part: Only add the LABELSECTOR if it is a native label. I will give this a try for a bit and get back to you. ps. Please try to indent code in the same existing style. > So, here is my patch to fix the issue. > ==================================================================== > diff -u -r1.69 disksubr.c > --- disksubr.c 26 Feb 2011 13:07:48 -0000 1.69 > +++ disksubr.c 21 May 2011 15:29:35 -0000 > @@ -141,7 +141,7 @@ > *s = (*s - 'a' + 'A'); > > if (strcmp(part->pmPartType, PART_TYPE_OPENBSD) == 0) { > - hfspartoff = part->pmPyPartStart - LABELSECTOR; > + hfspartoff = part->pmPyPartStart; > hfspartend = hfspartoff + part->pmPartBlkCnt; > if (partoffp) { > *partoffp = hfspartoff; > @@ -176,7 +176,7 @@ > return (0); > > /* next, dig out disk label */ > - bp->b_blkno = hfspartoff + LABELSECTOR; > + bp->b_blkno = hfspartoff; > bp->b_bcount = lp->d_secsize; > bp->b_flags = B_BUSY | B_READ | B_RAW; > (*strat)(bp); > @@ -201,12 +201,17 @@ > bp = geteblk((int)lp->d_secsize); > bp->b_dev = dev; > > - if (readdpmelabel(bp, strat, lp, &partoff, 1) != NULL && > - readdoslabel(bp, strat, lp, &partoff, 1) != NULL) > - goto done; > + if (readdpmelabel(bp, strat, lp, &partoff, 1) != NULL) > + { > + if (readdoslabel(bp, strat, lp, &partoff, 1) != NULL) > + goto done; > + else > + bp->b_blkno = partoff + LABELSECTOR; > + } > + else > + bp->b_blkno = partoff; > > /* Read it in, slap the new label in, and write it back out */ > - bp->b_blkno = partoff + LABELSECTOR; > bp->b_bcount = lp->d_secsize; > bp->b_flags = B_BUSY | B_READ | B_RAW; > (*strat)(bp); > ==================================================================== > > Here are 2 outputs from the disklabel tool, first before the patch... > > OpenBSD area: 53706878-58605119; size: 4898241; free: 4898241 > > ... and after the patch. > > OpenBSD area: 53706879-58605120; size: 4898241; free: 4898241 > > > I saw that the socppc port has the same piece of code in its > "socppc/disksubr.c" file. I can't test on this platform but my best > guess is it has the same problem. > > > On 05/08/2011 11:56 PM, Mathieu Olivier wrote: > > Hi. > > > > I recently made some space on my old iBook G4 (1.2 GHz, late 2004 model) > > to put a few extra partitions, for a Linux, an OpenBSD 4.9, and a shared > > data partition using FAT32. > > > > During the installation process, I briefly struggled with the disklabel > > tool before realizing that my problems came from the fact that it gave > > me an incorrect automatic layout to begin with: the root partition was > > starting one sector *ahead* of the OpenBSD partition itself. I fixed the > > layout manually, but the "boundstart" displayed by disklabel is still > > incorrect on my running system, as you can see from those outputs of > > pdisk and disklabel respectively: > > > > > > # pdisk wd0 > > Edit wd0 - > > Command (? for help): p > > > > Partition map (with 512 byte blocks) on 'wd0' > > #: type name length base ( size ) > > 1: Apple_partition_map Apple 63 @ 1 > > 2: Apple_Bootstrap untitled 1954 @ 39935392 > > 3: Apple_HFS Untitled 39673184 @ 262208 ( 18.9G) > > 4: Apple_UNIX_SVR2 LinuxRoot 10742188 @ 39937346 ( 5.1G) > > 5: Apple_UNIX_SVR2 swap 585938 @ 50679534 (286.1M) > > 6: Apple_UNIX_SVR2 Shared 2441407 @ 51265472 ( 1.2G) > > 7: Apple_Free Extra 262144 @ 64 (128.0M) > > 8: OpenBSD OpenBSD 4898241 @ 53706879 ( 2.3G) > > > > Device block size=512, Number of Blocks=58605120 (27.9G) > > DeviceType=0x0, DeviceId=0x0 > > > > Command (? for help): q > > > > # disklabel wd0 > > # /dev/rwd0c: > > type: ESDI > > disk: ESDI/IDE disk > > label: TOSHIBA MK3025GA > > duid: c2997bcd646b6636 > > flags: > > bytes/sector: 512 > > sectors/track: 63 > > tracks/cylinder: 16 > > sectors/cylinder: 1008 > > cylinders: 58140 > > total sectors: 58605120 > > boundstart: 53706878 > > boundend: 58605119 > > drivedata: 0 > > > > 16 partitions: > > # size offset fstype [fsize bsize cpg] > > a: 4499968 53706880 4.2BSD 2048 16384 1 # / > > b: 398241 58206879 swap > > c: 58605120 0 unused > > i: 39673184 262208 HFS > > j: 2441407 51265472 MSDOS > > > > > > I read some documentation and source files to try to figure out where > > this problem comes from, and I stepped across a very suspicious line of > > code, in "src/sys/arch/macppc/macppc/disksubr.c" > > <http://www.openbsd.org/cgi-bin/cvsweb/src/sys/arch/macppc/macppc/disksubr.c?annotate=1.69>. > > > > Line 144: "hfspartoff = part->pmPyPartStart - LABELSECTOR;" > > > > And, if I'm not mistaken, LABELSECTOR is equal to 1 on MacPPC, and > > hfspartoff is used to initialize the boundstart 6 lines later... So > > coincidence or real bug ? :) > > > > -- > Mathieu Olivier <moliv...@users.sourceforge.net>
