On Mon, Jun 14, 2010 at 05:58:55PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote: > On 06/14/2010 05:02 PM, Colin Watson wrote: > > 2010-06-14 Colin Watson <cjwat...@ubuntu.com> > > > > 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. > > That makes a regression for multiboot loading of image if it was moved > from one partition to another.
Do you have any suggestions on how to deal with that? I'm not familiar with multiboot and need guidance. > > === 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); > > + } > > Or grub_prefix[1] == ')' to allow ()/boot/grub syntax for unpartitioned > devices Makes sense. I've updated my local tree. > > + 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"); > > > > This error must go away with that new syntax. I agree, but how should kern/i386/pc/startup.S:multiboot_trampoline cope? I left this there because I didn't know a straightforward way to deal with that. > > - 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); > > + } > > > > Please use grub_partition_get_name. It will greatly simplify this part. Done, thanks. (I had a memory leak too.) Updated patch follows; same ChangeLog. === 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 16:21:53 +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] == ',' || 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 16:29:54 +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); @@ -305,6 +308,18 @@ setup (const char *dir, dos_part = root_dev->disk->partition->number; bsd_part = -1; } + + if (prefix[0] != '(') + { + char *root_part_name, *new_prefix; + + root_part_name = + grub_partition_get_name (root_dev->disk->partition); + new_prefix = xasprintf ("(,%s)%s", root_part_name, prefix); + strcpy (prefix, new_prefix); + free (new_prefix); + free (root_part_name); + } } else dos_part = bsd_part = -1; -- Colin Watson [cjwat...@ubuntu.com] _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel