* [PATCH] Fix grub-install with OS device name @ 2013-12-04 21:39 Colin Watson 2013-12-04 21:54 ` Jonathan McCune 0 siblings, 1 reply; 14+ messages in thread From: Colin Watson @ 2013-12-04 21:39 UTC (permalink / raw) To: grub-devel * util/setup.c (SETUP): Accept new dev_is_drive argument. If passed, don't map dev to a GRUB drive again. * include/grub/util/install.h (grub_util_bios_setup): Update prototype. (grub_util_sparc_setup): Likewise. * util/grub-install.c (main): Tell grub_util_bios_setup that install_drive has already been mapped to a GRUB drive. Likewise for grub_util_sparc_setup, and pass install_drive rather than install_device. * util/grub-setup.c (main): Adjust call to GRUB_SETUP_FUNC. --- ChangeLog | 13 +++++++++++++ include/grub/util/install.h | 4 ++-- util/grub-install.c | 8 +++++--- util/grub-setup.c | 2 +- util/setup.c | 39 ++++++++++++++++++++++----------------- 5 files changed, 43 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index b7e716f..d4a7e02 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,18 @@ 2013-12-04 Colin Watson <cjwatson@ubuntu.com> + * util/setup.c (SETUP): Accept new dev_is_drive argument. If + passed, don't map dev to a GRUB drive again. + * include/grub/util/install.h (grub_util_bios_setup): Update + prototype. + (grub_util_sparc_setup): Likewise. + * util/grub-install.c (main): Tell grub_util_bios_setup that + install_drive has already been mapped to a GRUB drive. Likewise for + grub_util_sparc_setup, and pass install_drive rather than + install_device. + * util/grub-setup.c (main): Adjust call to GRUB_SETUP_FUNC. + +2013-12-04 Colin Watson <cjwatson@ubuntu.com> + Copying the themes directory in grub-shell isn't parallel-test-friendly and breaks on the second test when the source directory is read-only (as in "make distcheck"). Instead, add a diff --git a/include/grub/util/install.h b/include/grub/util/install.h index 4ba00f5..d1b4567 100644 --- a/include/grub/util/install.h +++ b/include/grub/util/install.h @@ -180,12 +180,12 @@ grub_install_get_image_target (const char *arg); void grub_util_bios_setup (const char *dir, const char *boot_file, const char *core_file, - const char *dest, int force, + const char *dev, int dev_is_drive, int force, int fs_probe, int allow_floppy); void grub_util_sparc_setup (const char *dir, const char *boot_file, const char *core_file, - const char *dest, int force, + const char *dev, int dev_is_drive, int force, int fs_probe, int allow_floppy); char * diff --git a/util/grub-install.c b/util/grub-install.c index 2d6ef75..3bb82fc 100644 --- a/util/grub-install.c +++ b/util/grub-install.c @@ -1436,11 +1436,12 @@ main (int argc, char *argv[]) platdir, device_map, install_device); + grub_util_info ("('%s' mapped to '%s')", install_device, install_drive); /* Now perform the installation. */ if (install_bootsector) grub_util_bios_setup (platdir, "boot.img", "core.img", - install_drive, force, + install_drive, 1, force, fs_probe, allow_floppy); break; } @@ -1461,12 +1462,13 @@ main (int argc, char *argv[]) !fs_probe ? "--skip-fs-probe" : "", platdir, device_map, - install_drive); + install_device); + grub_util_info ("('%s' mapped to '%s')", install_device, install_drive); /* Now perform the installation. */ if (install_bootsector) grub_util_sparc_setup (platdir, "boot.img", "core.img", - install_device, force, + install_drive, 1, force, fs_probe, allow_floppy); break; } diff --git a/util/grub-setup.c b/util/grub-setup.c index cc3af5d..6e8951e 100644 --- a/util/grub-setup.c +++ b/util/grub-setup.c @@ -254,7 +254,7 @@ main (int argc, char *argv[]) GRUB_SETUP_FUNC (arguments.dir ? : DEFAULT_DIRECTORY, arguments.boot_file ? : DEFAULT_BOOT_FILE, arguments.core_file ? : DEFAULT_CORE_FILE, - arguments.device, arguments.force, + arguments.device, 0, arguments.force, arguments.fs_probe, arguments.allow_floppy); /* Free resources. */ diff --git a/util/setup.c b/util/setup.c index c1de3d2..3f8866e 100644 --- a/util/setup.c +++ b/util/setup.c @@ -247,7 +247,7 @@ identify_partmap (grub_disk_t disk __attribute__ ((unused)), void SETUP (const char *dir, const char *boot_file, const char *core_file, - const char *dev, int force, + const char *dev, int dev_is_drive, int force, int fs_probe, int allow_floppy) { char *core_path; @@ -270,25 +270,30 @@ SETUP (const char *dir, #endif bl.last_length = 0; - { - size_t len = strlen (dev); + if (dev_is_drive) + dest = xstrdup (dev); + else + { + /* Perhaps the user specified a parenthesised GRUB drive name. */ + size_t len = strlen (dev); - if (len > 2 && dev[0] == '(' && dev[len - 1] == ')') - { - dest = xmalloc (len - 1); - strncpy (dest, dev + 1, len - 2); - dest[len - 2] = '\0'; - } - } + if (len > 2 && dev[0] == '(' && dev[len - 1] == ')') + { + dest = xmalloc (len - 1); + strncpy (dest, dev + 1, len - 2); + dest[len - 2] = '\0'; + } - if (! dest) - { - /* Possibly, the user specified an OS device file. */ - dest = grub_util_get_grub_dev (dev); if (! dest) - grub_util_error (_("Invalid device `%s'.\n"), dev); - grub_util_info ("transformed OS device `%s' into GRUB device `%s'", - dev, dest); + { + /* Possibly, the user specified an OS device file. */ + grub_util_pull_device (dev); + dest = grub_util_get_grub_dev (dev); + if (! dest) + grub_util_error (_("Invalid device `%s'.\n"), dev); + grub_util_info ("transformed OS device `%s' into GRUB device `%s'", + dev, dest); + } } -- 1.8.4.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix grub-install with OS device name 2013-12-04 21:39 [PATCH] Fix grub-install with OS device name Colin Watson @ 2013-12-04 21:54 ` Jonathan McCune 2013-12-04 22:15 ` Colin Watson 0 siblings, 1 reply; 14+ messages in thread From: Jonathan McCune @ 2013-12-04 21:54 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 627 bytes --] On Wed, Dec 4, 2013 at 1:39 PM, Colin Watson <cjwatson@ubuntu.com> wrote: > * util/setup.c (SETUP): Accept new dev_is_drive argument. If > passed, don't map dev to a GRUB drive again. > * include/grub/util/install.h (grub_util_bios_setup): Update > prototype. > (grub_util_sparc_setup): Likewise. > * util/grub-install.c (main): Tell grub_util_bios_setup that > install_drive has already been mapped to a GRUB drive. Likewise for > grub_util_sparc_setup, and pass install_drive rather than > install_device. > * util/grub-setup.c (main): Adjust call to GRUB_SETUP_FUNC. > Tested successfully on i386-pc in a QEMU VM. -Jon [-- Attachment #2: Type: text/html, Size: 1012 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix grub-install with OS device name 2013-12-04 21:54 ` Jonathan McCune @ 2013-12-04 22:15 ` Colin Watson 2013-12-05 5:56 ` arvidjaar 0 siblings, 1 reply; 14+ messages in thread From: Colin Watson @ 2013-12-04 22:15 UTC (permalink / raw) To: grub-devel On Wed, Dec 04, 2013 at 01:54:42PM -0800, Jonathan McCune wrote: > Tested successfully on i386-pc in a QEMU VM. Thanks. I made a slight mistake, though, and broke the case of "grub-install '(hd0)'", which is what Andrey had been trying to fix in the first place. Here's a better version which I've tested to handle both OS and GRUB device forms successfully. diff --git a/ChangeLog b/ChangeLog index 8fba56c..c8072b5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2013-12-04 Colin Watson <cjwatson@ubuntu.com> + + * util/setup.c (SETUP): Accept new dev_is_drive argument. If + passed, don't map dev to a GRUB drive again. + * include/grub/util/install.h (grub_util_bios_setup): Update + prototype. + (grub_util_sparc_setup): Likewise. + * util/grub-install.c (main): Tell grub_util_bios_setup that + install_drive has already been mapped to a GRUB drive. Likewise for + grub_util_sparc_setup, and pass install_drive rather than + install_device. + * util/grub-setup.c (main): Adjust call to GRUB_SETUP_FUNC. + 2013-12-04 Vladimir Serbinenko <phcoder@gmail.com> * grub-core/boot/sparc64/ieee1275/boot.S [CDBOOT]: Move scratchpad diff --git a/include/grub/util/install.h b/include/grub/util/install.h index 4ba00f5..d1b4567 100644 --- a/include/grub/util/install.h +++ b/include/grub/util/install.h @@ -180,12 +180,12 @@ grub_install_get_image_target (const char *arg); void grub_util_bios_setup (const char *dir, const char *boot_file, const char *core_file, - const char *dest, int force, + const char *dev, int dev_is_drive, int force, int fs_probe, int allow_floppy); void grub_util_sparc_setup (const char *dir, const char *boot_file, const char *core_file, - const char *dest, int force, + const char *dev, int dev_is_drive, int force, int fs_probe, int allow_floppy); char * diff --git a/util/grub-install.c b/util/grub-install.c index 2d6ef75..3bb82fc 100644 --- a/util/grub-install.c +++ b/util/grub-install.c @@ -1436,11 +1436,12 @@ main (int argc, char *argv[]) platdir, device_map, install_device); + grub_util_info ("('%s' mapped to '%s')", install_device, install_drive); /* Now perform the installation. */ if (install_bootsector) grub_util_bios_setup (platdir, "boot.img", "core.img", - install_drive, force, + install_drive, 1, force, fs_probe, allow_floppy); break; } @@ -1461,12 +1462,13 @@ main (int argc, char *argv[]) !fs_probe ? "--skip-fs-probe" : "", platdir, device_map, - install_drive); + install_device); + grub_util_info ("('%s' mapped to '%s')", install_device, install_drive); /* Now perform the installation. */ if (install_bootsector) grub_util_sparc_setup (platdir, "boot.img", "core.img", - install_device, force, + install_drive, 1, force, fs_probe, allow_floppy); break; } diff --git a/util/grub-setup.c b/util/grub-setup.c index cc3af5d..6e8951e 100644 --- a/util/grub-setup.c +++ b/util/grub-setup.c @@ -254,7 +254,7 @@ main (int argc, char *argv[]) GRUB_SETUP_FUNC (arguments.dir ? : DEFAULT_DIRECTORY, arguments.boot_file ? : DEFAULT_BOOT_FILE, arguments.core_file ? : DEFAULT_CORE_FILE, - arguments.device, arguments.force, + arguments.device, 0, arguments.force, arguments.fs_probe, arguments.allow_floppy); /* Free resources. */ diff --git a/util/setup.c b/util/setup.c index c1de3d2..7bf125d 100644 --- a/util/setup.c +++ b/util/setup.c @@ -247,7 +247,7 @@ identify_partmap (grub_disk_t disk __attribute__ ((unused)), void SETUP (const char *dir, const char *boot_file, const char *core_file, - const char *dev, int force, + const char *dev, int dev_is_drive, int force, int fs_probe, int allow_floppy) { char *core_path; @@ -271,6 +271,7 @@ SETUP (const char *dir, bl.last_length = 0; { + /* Perhaps the user specified a parenthesised GRUB drive name. */ size_t len = strlen (dev); if (len > 2 && dev[0] == '(' && dev[len - 1] == ')') @@ -283,12 +284,18 @@ SETUP (const char *dir, if (! dest) { - /* Possibly, the user specified an OS device file. */ - dest = grub_util_get_grub_dev (dev); - if (! dest) - grub_util_error (_("Invalid device `%s'.\n"), dev); - grub_util_info ("transformed OS device `%s' into GRUB device `%s'", - dev, dest); + if (dev_is_drive) + dest = xstrdup (dev); + else + { + /* Possibly, the user specified an OS device file. */ + grub_util_pull_device (dev); + dest = grub_util_get_grub_dev (dev); + if (! dest) + grub_util_error (_("Invalid device `%s'.\n"), dev); + grub_util_info ("transformed OS device `%s' into GRUB device `%s'", + dev, dest); + } } -- 1.8.4.4 -- Colin Watson [cjwatson@ubuntu.com] ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix grub-install with OS device name 2013-12-04 22:15 ` Colin Watson @ 2013-12-05 5:56 ` arvidjaar 2013-12-05 7:08 ` Andrey Borzenkov 0 siblings, 1 reply; 14+ messages in thread From: arvidjaar @ 2013-12-05 5:56 UTC (permalink / raw) To: The development of GNU GRUB Отправлено с iPhone > 04 дек. 2013 г., в 23:15, Colin Watson <cjwatson@ubuntu.com> написал(а): > >> On Wed, Dec 04, 2013 at 01:54:42PM -0800, Jonathan McCune wrote: >> Tested successfully on i386-pc in a QEMU VM. > > Thanks. I made a slight mistake, though, and broke the case of > "grub-install '(hd0)'", which is what Andrey had been trying to fix in > the first place. Here's a better version which I've tested to handle > both OS and GRUB device forms successfully. > Ouch. It becomes messier and messier ... :) Why not simply drop resolution of device in grub-install? Let's do it in one place. > diff --git a/ChangeLog b/ChangeLog > index 8fba56c..c8072b5 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,16 @@ > +2013-12-04 Colin Watson <cjwatson@ubuntu.com> > + > + * util/setup.c (SETUP): Accept new dev_is_drive argument. If > + passed, don't map dev to a GRUB drive again. > + * include/grub/util/install.h (grub_util_bios_setup): Update > + prototype. > + (grub_util_sparc_setup): Likewise. > + * util/grub-install.c (main): Tell grub_util_bios_setup that > + install_drive has already been mapped to a GRUB drive. Likewise for > + grub_util_sparc_setup, and pass install_drive rather than > + install_device. > + * util/grub-setup.c (main): Adjust call to GRUB_SETUP_FUNC. > + > 2013-12-04 Vladimir Serbinenko <phcoder@gmail.com> > > * grub-core/boot/sparc64/ieee1275/boot.S [CDBOOT]: Move scratchpad > diff --git a/include/grub/util/install.h b/include/grub/util/install.h > index 4ba00f5..d1b4567 100644 > --- a/include/grub/util/install.h > +++ b/include/grub/util/install.h > @@ -180,12 +180,12 @@ grub_install_get_image_target (const char *arg); > void > grub_util_bios_setup (const char *dir, > const char *boot_file, const char *core_file, > - const char *dest, int force, > + const char *dev, int dev_is_drive, int force, > int fs_probe, int allow_floppy); > void > grub_util_sparc_setup (const char *dir, > const char *boot_file, const char *core_file, > - const char *dest, int force, > + const char *dev, int dev_is_drive, int force, > int fs_probe, int allow_floppy); > > char * > diff --git a/util/grub-install.c b/util/grub-install.c > index 2d6ef75..3bb82fc 100644 > --- a/util/grub-install.c > +++ b/util/grub-install.c > @@ -1436,11 +1436,12 @@ main (int argc, char *argv[]) > platdir, > device_map, > install_device); > + grub_util_info ("('%s' mapped to '%s')", install_device, install_drive); > > /* Now perform the installation. */ > if (install_bootsector) > grub_util_bios_setup (platdir, "boot.img", "core.img", > - install_drive, force, > + install_drive, 1, force, > fs_probe, allow_floppy); > break; > } > @@ -1461,12 +1462,13 @@ main (int argc, char *argv[]) > !fs_probe ? "--skip-fs-probe" : "", > platdir, > device_map, > - install_drive); > + install_device); > + grub_util_info ("('%s' mapped to '%s')", install_device, install_drive); > > /* Now perform the installation. */ > if (install_bootsector) > grub_util_sparc_setup (platdir, "boot.img", "core.img", > - install_device, force, > + install_drive, 1, force, > fs_probe, allow_floppy); > break; > } > diff --git a/util/grub-setup.c b/util/grub-setup.c > index cc3af5d..6e8951e 100644 > --- a/util/grub-setup.c > +++ b/util/grub-setup.c > @@ -254,7 +254,7 @@ main (int argc, char *argv[]) > GRUB_SETUP_FUNC (arguments.dir ? : DEFAULT_DIRECTORY, > arguments.boot_file ? : DEFAULT_BOOT_FILE, > arguments.core_file ? : DEFAULT_CORE_FILE, > - arguments.device, arguments.force, > + arguments.device, 0, arguments.force, > arguments.fs_probe, arguments.allow_floppy); > > /* Free resources. */ > diff --git a/util/setup.c b/util/setup.c > index c1de3d2..7bf125d 100644 > --- a/util/setup.c > +++ b/util/setup.c > @@ -247,7 +247,7 @@ identify_partmap (grub_disk_t disk __attribute__ ((unused)), > void > SETUP (const char *dir, > const char *boot_file, const char *core_file, > - const char *dev, int force, > + const char *dev, int dev_is_drive, int force, > int fs_probe, int allow_floppy) > { > char *core_path; > @@ -271,6 +271,7 @@ SETUP (const char *dir, > bl.last_length = 0; > > { > + /* Perhaps the user specified a parenthesised GRUB drive name. */ > size_t len = strlen (dev); > > if (len > 2 && dev[0] == '(' && dev[len - 1] == ')') > @@ -283,12 +284,18 @@ SETUP (const char *dir, > > if (! dest) > { > - /* Possibly, the user specified an OS device file. */ > - dest = grub_util_get_grub_dev (dev); > - if (! dest) > - grub_util_error (_("Invalid device `%s'.\n"), dev); > - grub_util_info ("transformed OS device `%s' into GRUB device `%s'", > - dev, dest); > + if (dev_is_drive) > + dest = xstrdup (dev); > + else > + { > + /* Possibly, the user specified an OS device file. */ > + grub_util_pull_device (dev); > + dest = grub_util_get_grub_dev (dev); > + if (! dest) > + grub_util_error (_("Invalid device `%s'.\n"), dev); > + grub_util_info ("transformed OS device `%s' into GRUB device `%s'", > + dev, dest); > + } > } > > > -- > 1.8.4.4 > > > -- > Colin Watson [cjwatson@ubuntu.com] > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix grub-install with OS device name 2013-12-05 5:56 ` arvidjaar @ 2013-12-05 7:08 ` Andrey Borzenkov 2013-12-07 8:44 ` [PATCH 1/2] revert 69ca97c820, it broke using OS device name as install device Andrey Borzenkov 0 siblings, 1 reply; 14+ messages in thread From: Andrey Borzenkov @ 2013-12-05 7:08 UTC (permalink / raw) To: The development of GNU GRUB On Thu, Dec 5, 2013 at 9:56 AM, <arvidjaar@gmail.com> wrote: > > > Отправлено с iPhone > >> 04 дек. 2013 г., в 23:15, Colin Watson <cjwatson@ubuntu.com> написал(а): >> >>> On Wed, Dec 04, 2013 at 01:54:42PM -0800, Jonathan McCune wrote: >>> Tested successfully on i386-pc in a QEMU VM. >> >> Thanks. I made a slight mistake, though, and broke the case of >> "grub-install '(hd0)'", which is what Andrey had been trying to fix in >> the first place. Here's a better version which I've tested to handle >> both OS and GRUB device forms successfully. >> > > Ouch. It becomes messier and messier ... :) Why not simply drop resolution of device in grub-install? Let's do it in one place. > One should not answer that early in the morning ... Colin, I see where the problem is, but let's do not complicate it even further. The real fix is to revert my patch and simply strip parenthesis in grub-install (with suitable modification of is_same_disk). Feel free to do it, otherwise I'll fix it on weekend. > >> diff --git a/ChangeLog b/ChangeLog >> index 8fba56c..c8072b5 100644 >> --- a/ChangeLog >> +++ b/ChangeLog >> @@ -1,3 +1,16 @@ >> +2013-12-04 Colin Watson <cjwatson@ubuntu.com> >> + >> + * util/setup.c (SETUP): Accept new dev_is_drive argument. If >> + passed, don't map dev to a GRUB drive again. >> + * include/grub/util/install.h (grub_util_bios_setup): Update >> + prototype. >> + (grub_util_sparc_setup): Likewise. >> + * util/grub-install.c (main): Tell grub_util_bios_setup that >> + install_drive has already been mapped to a GRUB drive. Likewise for >> + grub_util_sparc_setup, and pass install_drive rather than >> + install_device. >> + * util/grub-setup.c (main): Adjust call to GRUB_SETUP_FUNC. >> + >> 2013-12-04 Vladimir Serbinenko <phcoder@gmail.com> >> >> * grub-core/boot/sparc64/ieee1275/boot.S [CDBOOT]: Move scratchpad >> diff --git a/include/grub/util/install.h b/include/grub/util/install.h >> index 4ba00f5..d1b4567 100644 >> --- a/include/grub/util/install.h >> +++ b/include/grub/util/install.h >> @@ -180,12 +180,12 @@ grub_install_get_image_target (const char *arg); >> void >> grub_util_bios_setup (const char *dir, >> const char *boot_file, const char *core_file, >> - const char *dest, int force, >> + const char *dev, int dev_is_drive, int force, >> int fs_probe, int allow_floppy); >> void >> grub_util_sparc_setup (const char *dir, >> const char *boot_file, const char *core_file, >> - const char *dest, int force, >> + const char *dev, int dev_is_drive, int force, >> int fs_probe, int allow_floppy); >> >> char * >> diff --git a/util/grub-install.c b/util/grub-install.c >> index 2d6ef75..3bb82fc 100644 >> --- a/util/grub-install.c >> +++ b/util/grub-install.c >> @@ -1436,11 +1436,12 @@ main (int argc, char *argv[]) >> platdir, >> device_map, >> install_device); >> + grub_util_info ("('%s' mapped to '%s')", install_device, install_drive); >> >> /* Now perform the installation. */ >> if (install_bootsector) >> grub_util_bios_setup (platdir, "boot.img", "core.img", >> - install_drive, force, >> + install_drive, 1, force, >> fs_probe, allow_floppy); >> break; >> } >> @@ -1461,12 +1462,13 @@ main (int argc, char *argv[]) >> !fs_probe ? "--skip-fs-probe" : "", >> platdir, >> device_map, >> - install_drive); >> + install_device); >> + grub_util_info ("('%s' mapped to '%s')", install_device, install_drive); >> >> /* Now perform the installation. */ >> if (install_bootsector) >> grub_util_sparc_setup (platdir, "boot.img", "core.img", >> - install_device, force, >> + install_drive, 1, force, >> fs_probe, allow_floppy); >> break; >> } >> diff --git a/util/grub-setup.c b/util/grub-setup.c >> index cc3af5d..6e8951e 100644 >> --- a/util/grub-setup.c >> +++ b/util/grub-setup.c >> @@ -254,7 +254,7 @@ main (int argc, char *argv[]) >> GRUB_SETUP_FUNC (arguments.dir ? : DEFAULT_DIRECTORY, >> arguments.boot_file ? : DEFAULT_BOOT_FILE, >> arguments.core_file ? : DEFAULT_CORE_FILE, >> - arguments.device, arguments.force, >> + arguments.device, 0, arguments.force, >> arguments.fs_probe, arguments.allow_floppy); >> >> /* Free resources. */ >> diff --git a/util/setup.c b/util/setup.c >> index c1de3d2..7bf125d 100644 >> --- a/util/setup.c >> +++ b/util/setup.c >> @@ -247,7 +247,7 @@ identify_partmap (grub_disk_t disk __attribute__ ((unused)), >> void >> SETUP (const char *dir, >> const char *boot_file, const char *core_file, >> - const char *dev, int force, >> + const char *dev, int dev_is_drive, int force, >> int fs_probe, int allow_floppy) >> { >> char *core_path; >> @@ -271,6 +271,7 @@ SETUP (const char *dir, >> bl.last_length = 0; >> >> { >> + /* Perhaps the user specified a parenthesised GRUB drive name. */ >> size_t len = strlen (dev); >> >> if (len > 2 && dev[0] == '(' && dev[len - 1] == ')') >> @@ -283,12 +284,18 @@ SETUP (const char *dir, >> >> if (! dest) >> { >> - /* Possibly, the user specified an OS device file. */ >> - dest = grub_util_get_grub_dev (dev); >> - if (! dest) >> - grub_util_error (_("Invalid device `%s'.\n"), dev); >> - grub_util_info ("transformed OS device `%s' into GRUB device `%s'", >> - dev, dest); >> + if (dev_is_drive) >> + dest = xstrdup (dev); >> + else >> + { >> + /* Possibly, the user specified an OS device file. */ >> + grub_util_pull_device (dev); >> + dest = grub_util_get_grub_dev (dev); >> + if (! dest) >> + grub_util_error (_("Invalid device `%s'.\n"), dev); >> + grub_util_info ("transformed OS device `%s' into GRUB device `%s'", >> + dev, dest); >> + } >> } >> >> >> -- >> 1.8.4.4 >> >> >> -- >> Colin Watson [cjwatson@ubuntu.com] >> >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> https://lists.gnu.org/mailman/listinfo/grub-devel ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] revert 69ca97c820, it broke using OS device name as install device 2013-12-05 7:08 ` Andrey Borzenkov @ 2013-12-07 8:44 ` Andrey Borzenkov 2013-12-07 8:44 ` [PATCH 2/2] second attempt to fix using grub " Andrey Borzenkov 0 siblings, 1 reply; 14+ messages in thread From: Andrey Borzenkov @ 2013-12-07 8:44 UTC (permalink / raw) To: grub-devel grub-install already performs install device to grub device mapping (it needs it to compare install and /boot devices), so after this patch SETUP got called with grub device and tried to perform mapping again. --- ChangeLog | 5 +++++ util/grub-setup.c | 41 ++++++++++++++++++++++++++++++++++++++++- util/setup.c | 26 +------------------------- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 203038e..b93db5e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2013-11-29 Andrey Borzenkov <arvidjaar@gmail.com> + + Revert commit 69ca97c820, it cause failures in using OS device name + in grub-install. + 2013-12-06 Vladimir Serbinenko <phcoder@gmail.com> Don't add -mlong-calls when compiling with clang. diff --git a/util/grub-setup.c b/util/grub-setup.c index cc3af5d..90b9de0 100644 --- a/util/grub-setup.c +++ b/util/grub-setup.c @@ -209,9 +209,23 @@ DEVICE must be an OS device (e.g. /dev/sda)."), NULL, help_filter, NULL }; +static char * +get_device_name (char *dev) +{ + size_t len = strlen (dev); + + if (dev[0] != '(' || dev[len - 1] != ')') + return 0; + + dev[len - 1] = '\0'; + return dev + 1; +} + int main (int argc, char *argv[]) { + char *root_dev = NULL; + char *dest_dev = NULL; struct arguments arguments; grub_util_host_init (&argc, &argv); @@ -250,11 +264,34 @@ main (int argc, char *argv[]) grub_mdraid1x_init (); grub_lvm_init (); + dest_dev = get_device_name (arguments.device); + if (! dest_dev) + { + /* Possibly, the user specified an OS device file. */ + dest_dev = grub_util_get_grub_dev (arguments.device); + if (! dest_dev) + { + char *program = xstrdup(program_name); + fprintf (stderr, _("Invalid device `%s'.\n"), arguments.device); + argp_help (&argp, stderr, ARGP_HELP_STD_USAGE, program); + free(program); + exit(1); + } + grub_util_info ("transformed OS device `%s' into GRUB device `%s'", + arguments.device, dest_dev); + } + else + { + /* For simplicity. */ + dest_dev = xstrdup (dest_dev); + grub_util_info ("Using `%s' as GRUB device", dest_dev); + } + /* Do the real work. */ GRUB_SETUP_FUNC (arguments.dir ? : DEFAULT_DIRECTORY, arguments.boot_file ? : DEFAULT_BOOT_FILE, arguments.core_file ? : DEFAULT_CORE_FILE, - arguments.device, arguments.force, + dest_dev, arguments.force, arguments.fs_probe, arguments.allow_floppy); /* Free resources. */ @@ -266,6 +303,8 @@ main (int argc, char *argv[]) free (arguments.dir); free (arguments.dev_map); free (arguments.device); + free (root_dev); + free (dest_dev); return 0; } diff --git a/util/setup.c b/util/setup.c index c1de3d2..337c304 100644 --- a/util/setup.c +++ b/util/setup.c @@ -247,13 +247,12 @@ identify_partmap (grub_disk_t disk __attribute__ ((unused)), void SETUP (const char *dir, const char *boot_file, const char *core_file, - const char *dev, int force, + const char *dest, int force, int fs_probe, int allow_floppy) { char *core_path; char *boot_img, *core_img, *boot_path; char *root = 0; - char *dest = 0; size_t boot_size, core_size; #ifdef GRUB_SETUP_BIOS grub_uint16_t core_sectors; @@ -270,28 +269,6 @@ SETUP (const char *dir, #endif bl.last_length = 0; - { - size_t len = strlen (dev); - - if (len > 2 && dev[0] == '(' && dev[len - 1] == ')') - { - dest = xmalloc (len - 1); - strncpy (dest, dev + 1, len - 2); - dest[len - 2] = '\0'; - } - } - - if (! dest) - { - /* Possibly, the user specified an OS device file. */ - dest = grub_util_get_grub_dev (dev); - if (! dest) - grub_util_error (_("Invalid device `%s'.\n"), dev); - grub_util_info ("transformed OS device `%s' into GRUB device `%s'", - dev, dest); - } - - /* Read the boot image by the OS service. */ boot_path = grub_util_get_path (dir, boot_file); boot_size = grub_util_get_image_size (boot_path); @@ -326,7 +303,6 @@ SETUP (const char *dir, dest_dev = grub_device_open (dest); if (! dest_dev) grub_util_error ("%s", grub_errmsg); - free (dest); core_dev = dest_dev; -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] second attempt to fix using grub device name as install device 2013-12-07 8:44 ` [PATCH 1/2] revert 69ca97c820, it broke using OS device name as install device Andrey Borzenkov @ 2013-12-07 8:44 ` Andrey Borzenkov 2013-12-07 8:47 ` Vladimir 'φ-coder/phcoder' Serbinenko 0 siblings, 1 reply; 14+ messages in thread From: Andrey Borzenkov @ 2013-12-07 8:44 UTC (permalink / raw) To: grub-devel grub-install already resolved passed install device to grub device. So do the same as grub-setup and strip parenthesis if we get legacy (hdX). --- ChangeLog | 4 ++-- util/grub-install.c | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index b93db5e..161c568 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,7 @@ 2013-11-29 Andrey Borzenkov <arvidjaar@gmail.com> - Revert commit 69ca97c820, it cause failures in using OS device name - in grub-install. + Revert commit 69ca97c820, it caused failures when using OS device name + in grub-install. Instead just strip off parenthesis in grub-install if (hdX) was passed. 2013-12-06 Vladimir Serbinenko <phcoder@gmail.com> diff --git a/util/grub-install.c b/util/grub-install.c index 7a1db42..831c550 100644 --- a/util/grub-install.c +++ b/util/grub-install.c @@ -1098,7 +1098,11 @@ main (int argc, char *argv[]) { if (install_device[0] == '(' && install_device[grub_strlen (install_device) - 1] == ')') - install_drive = xstrdup (install_device); + { + install_device[grub_strlen (install_device) - 1] = '\0'; + install_drive = xstrdup (install_device + 1); + install_device[grub_strlen (install_device) - 1] = ')'; + } else { grub_util_pull_device (install_device); -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] second attempt to fix using grub device name as install device 2013-12-07 8:44 ` [PATCH 2/2] second attempt to fix using grub " Andrey Borzenkov @ 2013-12-07 8:47 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-12-07 9:40 ` Andrey Borzenkov 0 siblings, 1 reply; 14+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-12-07 8:47 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 349 bytes --] On 07.12.2013 09:44, Andrey Borzenkov wrote: > + install_device[grub_strlen (install_device) - 1] = '\0'; > + install_drive = xstrdup (install_device + 1); > + install_device[grub_strlen (install_device) - 1] = ')'; > + } Bad code. You forgot that grub_strlen changes with your operations. You need to keep explicit pointer. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 291 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] second attempt to fix using grub device name as install device 2013-12-07 8:47 ` Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-12-07 9:40 ` Andrey Borzenkov 2013-12-07 9:42 ` Vladimir 'φ-coder/phcoder' Serbinenko 0 siblings, 1 reply; 14+ messages in thread From: Andrey Borzenkov @ 2013-12-07 9:40 UTC (permalink / raw) To: grub-devel [-- Attachment #1: Type: text/plain, Size: 609 bytes --] В Sat, 07 Dec 2013 09:47:44 +0100 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет: > On 07.12.2013 09:44, Andrey Borzenkov wrote: > > + install_device[grub_strlen (install_device) - 1] = '\0'; > > + install_drive = xstrdup (install_device + 1); > > + install_device[grub_strlen (install_device) - 1] = ')'; > > + } > Bad code. You forgot that grub_strlen changes with your operations. You > need to keep explicit pointer. > > Oops. Any reason "our" gnulib is missing xstrndup? It is not the only place where it will make things more simple and readable. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] second attempt to fix using grub device name as install device 2013-12-07 9:40 ` Andrey Borzenkov @ 2013-12-07 9:42 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-12-07 10:19 ` [PATCH 2/2 v2] " Andrey Borzenkov 0 siblings, 1 reply; 14+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-12-07 9:42 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 939 bytes --] On 07.12.2013 10:40, Andrey Borzenkov wrote: > В Sat, 07 Dec 2013 09:47:44 +0100 > Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет: > >> On 07.12.2013 09:44, Andrey Borzenkov wrote: >>> + install_device[grub_strlen (install_device) - 1] = '\0'; >>> + install_drive = xstrdup (install_device + 1); >>> + install_device[grub_strlen (install_device) - 1] = ')'; >>> + } >> Bad code. You forgot that grub_strlen changes with your operations. You >> need to keep explicit pointer. >> >> > > Oops. Any reason "our" gnulib is missing xstrndup? It is not the only > place where it will make things more simple and readable. > xmalloc + memcpy + '\0' terminator in this case does the same, a little bit more efficiently. > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 291 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2 v2] second attempt to fix using grub device name as install device 2013-12-07 9:42 ` Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-12-07 10:19 ` Andrey Borzenkov 2013-12-07 13:50 ` Vladimir 'φ-coder/phcoder' Serbinenko 0 siblings, 1 reply; 14+ messages in thread From: Andrey Borzenkov @ 2013-12-07 10:19 UTC (permalink / raw) To: grub-devel grub-install already resolved passed install device to grub device. So do the same as grub-setup and strip parenthesis if we get legacy (hdX). --- ChangeLog | 4 ++-- util/grub-install.c | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index b93db5e..161c568 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,7 @@ 2013-11-29 Andrey Borzenkov <arvidjaar@gmail.com> - Revert commit 69ca97c820, it cause failures in using OS device name - in grub-install. + Revert commit 69ca97c820, it caused failures when using OS device name + in grub-install. Instead just strip off parenthesis in grub-install if (hdX) was passed. 2013-12-06 Vladimir Serbinenko <phcoder@gmail.com> diff --git a/util/grub-install.c b/util/grub-install.c index 7a1db42..0a9790a 100644 --- a/util/grub-install.c +++ b/util/grub-install.c @@ -1098,7 +1098,13 @@ main (int argc, char *argv[]) { if (install_device[0] == '(' && install_device[grub_strlen (install_device) - 1] == ')') - install_drive = xstrdup (install_device); + { + + size_t len = grub_strlen (install_device) - 2; + install_drive = xmalloc (len + 1); + memcpy (install_drive, install_device + 1, len); + install_drive[len] = '\0'; + } else { grub_util_pull_device (install_device); -- tg: (074285e..) u/fix-grub-install-on-hdX (depends on: u/revert-69ca97c820) ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2 v2] second attempt to fix using grub device name as install device 2013-12-07 10:19 ` [PATCH 2/2 v2] " Andrey Borzenkov @ 2013-12-07 13:50 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-12-07 13:59 ` Andrey Borzenkov 0 siblings, 1 reply; 14+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-12-07 13:50 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1558 bytes --] On 07.12.2013 11:19, Andrey Borzenkov wrote: > grub-install already resolved passed install device to grub device. So do the > same as grub-setup and strip parenthesis if we get legacy (hdX). > Did you test it with both syntaxes? > --- > ChangeLog | 4 ++-- > util/grub-install.c | 8 +++++++- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index b93db5e..161c568 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,7 +1,7 @@ > 2013-11-29 Andrey Borzenkov <arvidjaar@gmail.com> > > - Revert commit 69ca97c820, it cause failures in using OS device name > - in grub-install. > + Revert commit 69ca97c820, it caused failures when using OS device name > + in grub-install. Instead just strip off parenthesis in grub-install if (hdX) was passed. > > 2013-12-06 Vladimir Serbinenko <phcoder@gmail.com> > > diff --git a/util/grub-install.c b/util/grub-install.c > index 7a1db42..0a9790a 100644 > --- a/util/grub-install.c > +++ b/util/grub-install.c > @@ -1098,7 +1098,13 @@ main (int argc, char *argv[]) > { > if (install_device[0] == '(' > && install_device[grub_strlen (install_device) - 1] == ')') > - install_drive = xstrdup (install_device); > + { > + > + size_t len = grub_strlen (install_device) - 2; > + install_drive = xmalloc (len + 1); > + memcpy (install_drive, install_device + 1, len); > + install_drive[len] = '\0'; > + } > else > { > grub_util_pull_device (install_device); > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 291 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2 v2] second attempt to fix using grub device name as install device 2013-12-07 13:50 ` Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-12-07 13:59 ` Andrey Borzenkov 2013-12-07 14:14 ` Vladimir 'φ-coder/phcoder' Serbinenko 0 siblings, 1 reply; 14+ messages in thread From: Andrey Borzenkov @ 2013-12-07 13:59 UTC (permalink / raw) To: grub-devel [-- Attachment #1: Type: text/plain, Size: 1597 bytes --] В Sat, 07 Dec 2013 14:50:09 +0100 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет: > On 07.12.2013 11:19, Andrey Borzenkov wrote: > > grub-install already resolved passed install device to grub device. So do the > > same as grub-setup and strip parenthesis if we get legacy (hdX). > > > Did you test it with both syntaxes? Yes. linux-cor4:~ # /usr/local/grub2/sbin/grub-install '(hd1)' FATAL: Error inserting efivars (/lib/modules/3.11.6-4-pae/kernel/drivers/firmware/efi/efivars.ko): No such device /usr/local/grub2/sbin/grub-install: warning: cannot open directory `/usr/local/grub2/share/locale': No such file or directory. Installation finished. No error reported. linux-cor4:~ # /usr/local/grub2/sbin/grub-install /dev/sdb FATAL: Error inserting efivars (/lib/modules/3.11.6-4-pae/kernel/drivers/firmware/efi/efivars.ko): No such device /usr/local/grub2/sbin/grub-install: warning: cannot open directory `/usr/local/grub2/share/locale': No such file or directory. Installation finished. No error reported. linux-cor4:~ # /usr/local/grub2/sbin/grub-install /dev/disk/by-id/ata-QEMU_HARDDISK_QM00002 FATAL: Error inserting efivars (/lib/modules/3.11.6-4-pae/kernel/drivers/firmware/efi/efivars.ko): No such device /usr/local/grub2/sbin/grub-install: warning: cannot open directory `/usr/local/grub2/share/locale': No such file or directory. Installation finished. No error reported. linux-cor4:~ # cat /boot/grub/device.map (hd0) /dev/disk/by-id/ata-QEMU_HARDDISK_QM00001 (hd1) /dev/disk/by-id/ata-QEMU_HARDDISK_QM00002 linux-cor4:~ # exit [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2 v2] second attempt to fix using grub device name as install device 2013-12-07 13:59 ` Andrey Borzenkov @ 2013-12-07 14:14 ` Vladimir 'φ-coder/phcoder' Serbinenko 0 siblings, 0 replies; 14+ messages in thread From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-12-07 14:14 UTC (permalink / raw) To: The development of GNU GRUB [-- Attachment #1: Type: text/plain, Size: 1944 bytes --] On 07.12.2013 14:59, Andrey Borzenkov wrote: > В Sat, 07 Dec 2013 14:50:09 +0100 > Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет: > >> On 07.12.2013 11:19, Andrey Borzenkov wrote: >>> grub-install already resolved passed install device to grub device. So do the >>> same as grub-setup and strip parenthesis if we get legacy (hdX). >>> >> Did you test it with both syntaxes? > > Yes. > Go ahead for both parts. Colin told on IRC that he's ok with this patch as well. > linux-cor4:~ # /usr/local/grub2/sbin/grub-install '(hd1)' > FATAL: Error inserting efivars (/lib/modules/3.11.6-4-pae/kernel/drivers/firmware/efi/efivars.ko): No such device > /usr/local/grub2/sbin/grub-install: warning: cannot open directory `/usr/local/grub2/share/locale': No such file or directory. > Installation finished. No error reported. > linux-cor4:~ # /usr/local/grub2/sbin/grub-install /dev/sdb > FATAL: Error inserting efivars (/lib/modules/3.11.6-4-pae/kernel/drivers/firmware/efi/efivars.ko): No such device > /usr/local/grub2/sbin/grub-install: warning: cannot open directory `/usr/local/grub2/share/locale': No such file or directory. > Installation finished. No error reported. > linux-cor4:~ # /usr/local/grub2/sbin/grub-install /dev/disk/by-id/ata-QEMU_HARDDISK_QM00002 > FATAL: Error inserting efivars (/lib/modules/3.11.6-4-pae/kernel/drivers/firmware/efi/efivars.ko): No such device > /usr/local/grub2/sbin/grub-install: warning: cannot open directory `/usr/local/grub2/share/locale': No such file or directory. > Installation finished. No error reported. > linux-cor4:~ # cat /boot/grub/device.map > (hd0) /dev/disk/by-id/ata-QEMU_HARDDISK_QM00001 > (hd1) /dev/disk/by-id/ata-QEMU_HARDDISK_QM00002 > linux-cor4:~ # exit > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 291 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-12-07 14:14 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-04 21:39 [PATCH] Fix grub-install with OS device name Colin Watson 2013-12-04 21:54 ` Jonathan McCune 2013-12-04 22:15 ` Colin Watson 2013-12-05 5:56 ` arvidjaar 2013-12-05 7:08 ` Andrey Borzenkov 2013-12-07 8:44 ` [PATCH 1/2] revert 69ca97c820, it broke using OS device name as install device Andrey Borzenkov 2013-12-07 8:44 ` [PATCH 2/2] second attempt to fix using grub " Andrey Borzenkov 2013-12-07 8:47 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-12-07 9:40 ` Andrey Borzenkov 2013-12-07 9:42 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-12-07 10:19 ` [PATCH 2/2 v2] " Andrey Borzenkov 2013-12-07 13:50 ` Vladimir 'φ-coder/phcoder' Serbinenko 2013-12-07 13:59 ` Andrey Borzenkov 2013-12-07 14:14 ` Vladimir 'φ-coder/phcoder' Serbinenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).