Patchwork Pegasos and partition numbering

login
register
mail settings
Submitter David Woodhouse
Date March 28, 2008, 12:04 a.m.
Message ID <1206662642.9540.433.camel@pmac.infradead.org>
Download mbox | patch
Permalink /patch/22966/
State Accepted
Commit 37578220a9120f0a7770b0c6c6eae046ce72fc93
Headers show

Comments

David Woodhouse - March 28, 2008, 12:04 a.m.
The whole bplan partition numbering thing is a clusterfuck, but it
shouldn't be as crappy as it is. 

We can _cope_ with the fact that firmware is broken and has an
off-by-one in its partition numbering. All we need to do in yaboot is
add 1 to the partition numbers we infer from in /chosen/boot-device, if
we detect that we're on an afflicted machine. That's all we need to do,
and all we ever _should_ have done. And is already in yaboot. Yaboot
does its own block device handling, including partitions, and doesn't
ever care about the firmware's problems (in that respect, at least).

It doesn't affect the installer at _all_, because the installer can't
set up the firmware's boot-device anyway and it has to be done by hand.
All it affects is the release notes telling the user how to make it
bootable.

Unfortunately, we didn't make it that simple when we first supported the
Pegasos. For reasons which aren't entirely clear to me, we ended up with
Amiga partition table support in yaboot with the _same_ off-by-one
error, to match the firmware. And thus we have hacks in the installer to
use amiga partitions for Pegasos, and to cope with the off-by-one crap.

It's only after avoiding all this crap purely by accident on Efika, by
using DOS partition tables, that I realise how stupid I was to blindly
copy the crap that other people were doing, and to believe that Pegasos
would only work with Amiga partition tables. 

I'd like to get rid of the off-by-one bug in yaboot's Amiga partition
handling. At the moment, our simple 'if bplan, partition++' in
yaboot_main() is wrong when we have Amiga partitions, although it's fine
for other partition types.

Actually, I'd also like to make that same increment conditional on 
!conf_given, so that if someone specifies 'conf=hd:1,/yaboot.conf' on
the command line, that partition number _isn't_ incremented.

So any time you see a proper path specified as 'dev:part,/path/name' you
know it's a real one with proper partition numbers. Remember, the bplan
firmware doesn't allow that form, and takes a space between the
'dev:part' bit and the filename:
	boot hd:0 /yaboot/yaboot conf=hd:1,/yaboot/yaboot.conf

Fixing the off-by-one bug in the Amiga partition handling means that
upgrades might break. I suppose we could have a 'noamigaoffbyone'
configuration option which all newly-written yaboot.conf files would
have, which controls this behaviour. But to be honest I just don't think
it's worth it.
Paul Nasrat - April 15, 2008, 6:48 a.m.
On 28 Mar 2008, at 00:04, David Woodhouse wrote:
> The whole bplan partition numbering thing is a clusterfuck, but it
> shouldn't be as crappy as it is.

Applied - thanks.

Paul

Patch

diff --git a/second/partition.c b/second/partition.c
index 9a7fe73..d20a0ed 100644
--- a/second/partition.c
+++ b/second/partition.c
@@ -287,7 +287,7 @@  partition_amiga_lookup( const char *dev_name, prom_handle disk,
 	for (i=0; i < possible; i++) used[i] = 0;
 
 
-	for (part = amiga_block[AMIGA_PARTITIONS], partition = 0;
+	for (part = amiga_block[AMIGA_PARTITIONS], partition = 1;
 		part != AMIGA_END;
 		part = amiga_block[AMIGA_PART_NEXT], partition++)
 	{
diff --git a/second/yaboot.c b/second/yaboot.c
index 5fc1213..2d2b7bc 100644
--- a/second/yaboot.c
+++ b/second/yaboot.c
@@ -1749,7 +1749,7 @@  yaboot_main(void)
 	  prom_printf("%s: Unable to parse\n", bootdevice);
 	  return -1;
      }
-     if (_machine == _MACH_bplan)
+     if (_machine == _MACH_bplan && !conf_given)
         boot.part++;
      DEBUG_F("After parse_device_path: dev=%s, part=%d, file=%s\n",
 	     boot.dev, boot.part, boot.file);