From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1OOBBl-0003mJ-1g for mharc-grub-devel@gnu.org; Mon, 14 Jun 2010 11:03:01 -0400 Received: from [140.186.70.92] (port=58488 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OOBBh-0003lD-Fc for grub-devel@gnu.org; Mon, 14 Jun 2010 11:02:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OOBBf-0003B1-VB for grub-devel@gnu.org; Mon, 14 Jun 2010 11:02:57 -0400 Received: from smarthost02.mail.zen.net.uk ([212.23.3.141]:41335) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OOBBf-0003Ap-Ju for grub-devel@gnu.org; Mon, 14 Jun 2010 11:02:55 -0400 Received: from [82.69.40.219] (helo=riva.pelham.vpn.ucam.org) by smarthost02.mail.zen.net.uk with esmtp (Exim 4.63) (envelope-from ) id 1OOBBc-0007tr-VK; Mon, 14 Jun 2010 15:02:53 +0000 Received: from cjwatson by riva.pelham.vpn.ucam.org with local (Exim 3.36 #1 (Debian)) id 1OOBBc-00056J-00; Mon, 14 Jun 2010 16:02:52 +0100 Date: Mon, 14 Jun 2010 16:02:52 +0100 From: Colin Watson To: grub-devel@gnu.org Message-ID: <20100614150251.GA19053@riva.ucam.org> References: <4C0BE2C7.4020407@gmail.com> <4C0D5AAE.6070504@gmail.com> <4C13B6A8.7060102@gmail.com> <4C13C33B.4090302@gmail.com> <4C15044D.9050608@gmail.com> <20100614113702.GN21862@riva.ucam.org> <20100614132536.GO21862@riva.ucam.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20100614132536.GO21862@riva.ucam.org> User-Agent: Mutt/1.5.18 (2008-05-17) X-Originating-Smarthost02-IP: [82.69.40.219] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) Cc: Axel Beckert , 585068@bugs.debian.org Subject: Re: Which partitioning schemes should be supported by GRUB? X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Jun 2010 15:02:59 -0000 On Mon, Jun 14, 2010 at 02:25:39PM +0100, Colin Watson wrote: > On Mon, Jun 14, 2010 at 08:07:35AM -0500, richardvoigt@gmail.com wrote: > > Colin Watson wrote: > > > I can think of an alternative.  We do still need grub_install_dos_part > > > and grub_install_bsd_part for the multiboot trampoline, which is in > > > assembly, so it's difficult to abandon them altogether.  However, > > > there's no reason we need to use them in make_install_device.  How about > > > we invent a way to encode most of the information in string form in > > > grub_prefix, while leaving a placeholder for make_install_device to fill > > > in the disk?  There are 64 bytes available for grub_prefix, which should > > > be plenty.  For example, how about the following (with \0 standing for > > > ASCII NUL): > > > > > >  (\0,msdos1,bsd1)/boot/grub > > > > > > It's then just a matter of spotting the "(\0," sequence and replacing > > > the \0 with the drive name.  I think this can probably be done using > > > less code than the first option above, and all told it feels a bit less > > > hacky to me. > > > > Instead of doing string replacement, why not just define a > > disk-relative partition format? > > Simple string replacement would be much easier to implement, and > probably smaller. Plus, we don't need disk-relative device naming > elsewhere, and I think it would require putting platform-specific code > (otherwise how do you know which disk to be relative to?) in places that > are otherwise pretty platform-independent. > > > Also, the '\0' seems unnecessary. Is having "(," meaningful in some > > way already? > > Good point. This would be sufficient. How about the following patch, implementing this proposal? I've tested this with Debian GNU/kFreeBSD, and it's enough to make the boot loader work again out of the box after grub-install. The 'root' variable is still wrong, but that isn't particularly urgent as UUIDs usually take care of that. The kernel.img size increase is 84 bytes, yielding a core.img size increase of 50 bytes in this configuration (22932 -> 22982 bytes). Do I need to work on making that smaller somehow? It seems tolerable to me. 2010-06-14 Colin Watson Fix i386-pc prefix handling with nested partitions (Debian bug #585068). * kern/i386/pc/init.c (make_install_device): If the prefix starts with "(,", fill the boot drive in between those two characters, but expect that a full partition specification including partition map names will follow. * util/i386/pc/grub-setup.c (setup): Unless an explicit prefix was specified, write a prefix without the drive name but including a full partition specification. === modified file 'kern/i386/pc/init.c' --- kern/i386/pc/init.c 2010-05-21 18:08:48 +0000 +++ kern/i386/pc/init.c 2010-06-14 14:44:13 +0000 @@ -83,6 +83,14 @@ make_install_device (void) grub_snprintf (ptr, sizeof (dev) - (ptr - dev), ")%s", grub_prefix); grub_strcpy (grub_prefix, dev); } + else if (grub_prefix[1] == ',') + { + /* We have a prefix, but still need to fill in the boot drive. */ + grub_snprintf (dev, sizeof (dev), + "(%cd%u%s", (grub_boot_drive & 0x80) ? 'h' : 'f', + grub_boot_drive & 0x7f, grub_prefix + 1); + grub_strcpy (grub_prefix, dev); + } return grub_prefix; } === modified file 'util/i386/pc/grub-setup.c' --- util/i386/pc/grub-setup.c 2010-06-11 20:31:16 +0000 +++ util/i386/pc/grub-setup.c 2010-06-14 14:45:24 +0000 @@ -99,6 +99,7 @@ setup (const char *dir, struct grub_boot_blocklist *first_block, *block; grub_int32_t *install_dos_part, *install_bsd_part; grub_int32_t dos_part, bsd_part; + char *prefix; char *tmp_img; int i; grub_disk_addr_t first_sector; @@ -230,6 +231,8 @@ setup (const char *dir, + GRUB_KERNEL_MACHINE_INSTALL_DOS_PART); install_bsd_part = (grub_int32_t *) (core_img + GRUB_DISK_SECTOR_SIZE + GRUB_KERNEL_MACHINE_INSTALL_BSD_PART); + prefix = (char *) (core_img + GRUB_DISK_SECTOR_SIZE + + GRUB_KERNEL_MACHINE_PREFIX); /* Open the root device and the destination device. */ root_dev = grub_device_open (root); @@ -289,21 +292,45 @@ setup (const char *dir, override the current setting. */ if (*install_dos_part != -2) { + grub_partition_t root_part = root_dev->disk->partition; + /* Embed information about the installed location. */ - if (root_dev->disk->partition) + if (root_part) { - if (root_dev->disk->partition->parent) + char *new_prefix; + + if (root_part->parent) { - if (root_dev->disk->partition->parent->parent) + if (root_part->parent->parent) grub_util_error ("Installing on doubly nested partitions is " "not supported"); - dos_part = root_dev->disk->partition->parent->number; - bsd_part = root_dev->disk->partition->number; + dos_part = root_part->parent->number; + bsd_part = root_part->number; + + if (prefix[0] != '(') + { + new_prefix = xasprintf ("(,%s%d,%s%d)%s", + root_part->parent->partmap->name, + root_part->parent->number + 1, + root_part->partmap->name, + root_part->number + 1, + prefix); + strcpy (prefix, new_prefix); + } } else { - dos_part = root_dev->disk->partition->number; + dos_part = root_part->number; bsd_part = -1; + + if (prefix[0] != '(') + { + new_prefix = xasprintf ("(,%s%d)%s", + root_part->partmap->name, + root_part->number + 1, + prefix); + strcpy (prefix, new_prefix); + } } } else Thanks, -- Colin Watson [cjwatson@ubuntu.com]