boot directory prefix in grub-install (even with --root-directory)

[Grégoire Sutre]

Hi,

grub-install copies GRUB images into ${rootdir}/boot/grub (where 
${rootdir} is empty by default and can be changed with --root-directory).


To install GRUB files on a partition that contains a /boot file, one 
must specify a non-empty --root-directory, and grub files are then 
copied into /DIR/boot/grub.  Or one could mount the partition on a mount 
point of the form DIR/boot and specify --root-directory=DIR to force 
installation into the /grub directory of the partition.  Both solutions 
are not completely satisfactory.  Is there a better one?


The problem arises e.g. with NetBSD which uses /boot for its own 
bootloader.  The script util/grub-install.in contains lines to take care 
of that situation (lines 160-172), but these lines rely on a variable 
${host_os} that is not set.  This might be deprecated code from GRUB 
Legacy's grub-install, where ${host_os} is set by a configure substitution.


An alternative solution would be to have grub-install install in 
${rootdir}/grub, with ${rootdir} equal to /boot by default.  The default 
behavior (with no --root-directory) would be the same.


The following patch implements this alternative.

Thanks for your time,

Grégoire

--- util/grub-install.in.orig   2009-12-22 11:26:01.538833717 +0100
+++ util/grub-install.in2009-12-22 12:48:55.979476412 +0100
@@ -39,8 +39,7 @@
 fi
 grub_mkdevicemap=${sbindir}/`echo grub-mkdevicemap | sed ${transform}`
 grub_probe=${sbindir}/`echo grub-probe | sed ${transform}`
-rootdir=
-grub_prefix=`echo /boot/grub | sed ${transform}`
+rootdir=/boot
 modules=

 install_device=
@@ -66,7 +65,7 @@
   -v, --version   print the version information and exit
   --modules=MODULES   pre-load specified modules MODULES
   --root-directory=DIRinstall GRUB images under the directory DIR
-  instead of the root directory
+  instead of ${rootdir}
   --grub-setup=FILE   use FILE as grub-setup
   --grub-mkimage=FILE use FILE as grub-mkimage
   --grub-mkdevicemap=FILE use FILE as grub-mkdevicemap
@@ -84,7 +83,7 @@

 INSTALL_DEVICE can be a GRUB device name or a system device filename.

-grub-install copies GRUB images into the DIR/boot directory specified by
+grub-install copies GRUB images into the DIR/`echo grub | sed 
${transform}` directory specified by

 --root-directory, and uses grub-setup to install grub into the boot
 sector.

@@ -157,21 +156,7 @@
 setup_verbose="--verbose"
 fi

-# Initialize these directories here, since ROOTDIR was initialized.
-case "$host_os" in
-netbsd* | openbsd*)
-# Because /boot is used for the boot block in NetBSD and OpenBSD, 
use /grub

-# instead of /boot/grub.
-grub_prefix=`echo /grub | sed ${transform}`
-bootdir=${rootdir}
-;;
-*)
-# Use /boot/grub by default.
-bootdir=${rootdir}/boot
-;;
-esac
-
-grubdir=${bootdir}/`echo grub | sed ${transform}`
+grubdir=${rootdir}/`echo grub | sed ${transform}`
 device_map=${grubdir}/device.map

 grub_probe="${grub_probe} --device-map=${device_map}"
@@ -204,8 +189,7 @@
 fi

 # Create the GRUB directory if it is not present.
-test -d "$bootdir" || mkdir "$bootdir" || exit 1
-test -d "$grubdir" || mkdir "$grubdir" || exit 1
+mkdir -p "$grubdir" || exit 1

 # If --recheck is specified, remove the device map, if present.
 if test $recheck = yes; then


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: boot directory prefix in grub-install (even with --root-directory)

[Grégoire Sutre]

Vladimir 'φ-coder/phcoder' Serbinenko wrote:

The main problem is with scripts already using --root-directory and
relying on existing behaviour.


Yes, you're right.



I propose add a new option
--grub-directory=DIR
which defaults to ROOTDIR/boot/grub
What do you think about this?


Nice, I'm looking forward to it.  Thanks :-)

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Fix for grub_assert_fail undefined on NetBSD and other platforms

[Grégoire Sutre]

Hi,

> Unfortunately, I couldn't test this on NetBSD, where old technique was
> reported failing as GCC optimizations didn't happen.  Can Grégoire
> Sutr or anybody else try this out on NetBSD and confirm?

Thanks for looking into this.

I tested the second version of the patch.  Linking now works without 
errors but I get new warnings:


./include/grub/list.h:44: warning: 'error' attribute directive ignored

I attach the log of (./autogen.sh && ./configure && gmake grub-fstest 
LDFLAGS=-lintl).  This is on NetBSD 5.0 with gcc 4.1.3 (the default). 
If I use gcc 4.4 instead, then I do not get any warning.


Grégoire
acinclude.m4:17: warning: underquoted definition of grub_PROG_TARGET_CC
acinclude.m4:17:   run info '(automake)Extending aclocal'
acinclude.m4:17:   or see 
http://sources.redhat.com/automake/automake.html#Extending-aclocal
configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET
../../lib/autoconf/general.m4:1819: AC_CANONICAL_TARGET is expanded from...
configure.ac:42: the top level
configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET
../../lib/autoconf/general.m4:1819: AC_CANONICAL_TARGET is expanded from...
configure.ac:42: the top level
configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET
../../lib/autoconf/general.m4:1819: AC_CANONICAL_TARGET is expanded from...
configure.ac:42: the top level
configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET
../../lib/autoconf/general.m4:1819: AC_CANONICAL_TARGET is expanded from...
configure.ac:42: the top level
configure.ac:41: installing `./config.guess'
configure.ac:176: required file `./config.rpath' not found
configure.ac:41: installing `./config.sub'
configure.ac:35: installing `./install-sh'
configure.ac:35: installing `./missing'
automake: no `Makefile.am' found for any configure output
WARNING: C file isn't a module: ac.c
WARNING: C file isn't a module: cipher.c
WARNING: C file isn't a module: dsa.c
WARNING: C file isn't a module: ecc.c
WARNING: C file isn't a module: elgamal.c
WARNING: C file isn't a module: hash-common.c
WARNING: C file isn't a module: hmac-tests.c
WARNING: C file isn't a module: md.c
WARNING: C file isn't a module: primegen.c
WARNING: C file isn't a module: pubkey.c
WARNING: C file isn't a module: rsa.c
checking for a BSD-compatible install... /usr/pkg/bin/ginstall -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /usr/pkg/bin/gmkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking build system type... i386-unknown-netbsdelf5.0.
checking host system type... i386-unknown-netbsdelf5.0.
checking target system type... i386-unknown-netbsdelf5.0.
checking for cmp... cmp
checking for bison... bison
checking for gawk... (cached) gawk
checking whether make sets $(MAKE)... (cached) yes
checking for ruby... /usr/pkg/bin/ruby
checking for makeinfo... /usr/bin/makeinfo
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking for style of include used by make... GNU
checking dependency style of gcc... none
checking how to run the C preprocessor... gcc -E
checking for grep that handles long lines and -e... /usr/pkg/bin/ggrep
checking for egrep... /usr/pkg/bin/ggrep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking minix/config.h usability... no
checking minix/config.h presence... no
checking for minix/config.h... no
checking whether it is safe to define __EXTENSIONS__... yes
checking whether NLS is requested... yes
checking for msgfmt... /usr/bin/msgfmt
checking for gmsgfmt... /usr/bin/msgfmt
checking for xgettext... /usr/bin/xgettext
checking for msgmerge... /usr/bin/msgmerge
checking whether we are using the GNU C Library 2 or newer... no
checking for ranlib... ranlib
checking for strerror in -lcposix... no
checking for an ANSI C-conforming const... yes
checking for signed... yes
checking for inline... inline
checking for off_t... yes
checking for size_t... yes
checking for long long... yes
checking for long double... yes
checking for wchar_t... yes
checking for wint_t... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for intmax_t... yes
checking whether printf() supports POSIX/XSI format strings... yes
checking for working alloca.h... no
checking for alloca... yes
checking for stdlib.h... (cached) yes
checking for unistd.h.

Non-static variables and nested function pointers [bug #28392]

[Grégoire Sutre]

Hi,

I am trying to add NetBSD specific code to util/hostdisk.c in order to 
make grub-probe work.  This part is almost finished.  However, I had a 
hard time dealing with segfaults in callbacks (hook function pointers) 
in a number of places of the vanilla code.  Actually, I get segfaults in 
grub-probe with the vanilla trunk code (see bug report #28392).  This is 
on NetBSD 5.0 i386.


In the end, these segfaults were fixed by making sure that all variables 
accessed by pointers to nested functions are declared static.  I attach 
a patch that fixes these segfaults on my NetBSD box (this patch is also 
included in the bug report).


However, I am not a C expert, and I must be missing something as the 
code (obviously) works well on other systems.


Thanks for your help,

Grégoire
diff -Naur grub2_2009-12-22/fs/ext2.c grub2/fs/ext2.c
--- grub2_2009-12-22/fs/ext2.c	2009-12-22 17:53:27.0 +0100
+++ grub2/fs/ext2.c	2009-12-23 19:05:36.0 +0100
@@ -789,9 +789,12 @@
 	   int (*hook) (const char *filename,
 			const struct grub_dirhook_info *info))
 {
-  struct grub_ext2_data *data = 0;
+  static struct grub_ext2_data *data = 0;
   struct grub_fshelp_node *fdiro = 0;
 
+  static int (*myhook) (const char *filename,
+const struct grub_dirhook_info *info);
+
   auto int NESTED_FUNC_ATTR iterate (const char *filename,
  enum grub_fshelp_filetype filetype,
  grub_fshelp_node_t node);
@@ -817,9 +820,11 @@
 
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   grub_free (node);
-  return hook (filename, &info);
+  return myhook (filename, &info);
 }
 
+  myhook = hook;
+
   grub_dl_ref (my_mod);
 
   data = grub_ext2_mount (device->disk);
diff -Naur grub2_2009-12-22/fs/fat.c grub2/fs/fat.c
--- grub2_2009-12-22/fs/fat.c	2009-12-22 17:53:27.0 +0100
+++ grub2/fs/fat.c	2009-12-23 17:35:00.0 +0100
@@ -606,9 +606,13 @@
 		   int (*hook) (const char *filename,
 const struct grub_dirhook_info *info))
 {
-  char *dirname, *dirp;
-  int call_hook;
-  int found = 0;
+  static char *dirname;
+  char *dirp;
+  static int call_hook;
+  static int found = 0;
+  static struct grub_fat_data *mydata;
+  static int (*myhook) (const char *filename,
+const struct grub_dirhook_info *info);
 
   auto int iter_hook (const char *filename, struct grub_fat_dir_entry *dir);
   int iter_hook (const char *filename, struct grub_fat_dir_entry *dir)
@@ -622,25 +626,28 @@
 if (dir->attr & GRUB_FAT_ATTR_VOLUME_ID)
   return 0;
 if (*dirname == '\0' && call_hook)
-  return hook (filename, &info);
+  return myhook (filename, &info);
 
 if (grub_strcasecmp (dirname, filename) == 0)
   {
 	found = 1;
-	data->attr = dir->attr;
-	data->file_size = grub_le_to_cpu32 (dir->file_size);
-	data->file_cluster = ((grub_le_to_cpu16 (dir->first_cluster_high) << 16)
+	mydata->attr = dir->attr;
+	mydata->file_size = grub_le_to_cpu32 (dir->file_size);
+	mydata->file_cluster = ((grub_le_to_cpu16 (dir->first_cluster_high) << 16)
 			  | grub_le_to_cpu16 (dir->first_cluster_low));
-	data->cur_cluster_num = ~0U;
+	mydata->cur_cluster_num = ~0U;
 
 	if (call_hook)
-	  hook (filename, &info);
+	  myhook (filename, &info);
 
 	return 1;
   }
 return 0;
   }
 
+  mydata = data;
+  myhook = hook;
+
   if (! (data->attr & GRUB_FAT_ATTR_DIRECTORY))
 {
   grub_error (GRUB_ERR_BAD_FILE_TYPE, "not a directory");
diff -Naur grub2_2009-12-22/kern/device.c grub2/kern/device.c
--- grub2_2009-12-22/kern/device.c	2009-12-22 17:53:30.0 +0100
+++ grub2/kern/device.c	2009-12-23 18:43:21.0 +0100
@@ -83,17 +83,45 @@
   auto int iterate_partition (grub_disk_t disk,
 			  const grub_partition_t partition);
 
-  struct part_ent
+  static struct part_ent
   {
 struct part_ent *next;
 char name[0];
   } *ents;
 
+  static int (*myhook) (const char *name);
+
+  int iterate_partition (grub_disk_t disk, const grub_partition_t partition)
+{
+  char *partition_name;
+  struct part_ent *p;
+
+  partition_name = grub_partition_get_name (partition);
+  if (! partition_name)
+	return 1;
+
+  p = grub_malloc (sizeof (p->next) + grub_strlen (disk->name) + 1 +
+		   grub_strlen (partition_name) + 1);
+  if (!p)
+	{
+	  grub_free (partition_name);
+	  return 1;
+	}
+
+  grub_sprintf (p->name, "%s,%s", disk->name, partition_name);
+  grub_free (partition_name);
+
+  p->next = ents;
+  ents = p;
+
+  return 0;
+}
+
   int iterate_disk (const char *disk_name)
 {
   grub_device_t dev;
 
-  if (hook (disk_name))
+  if (myhook (disk_name))
 	return 1;
 
   dev = grub_device_open (disk_name);
@@ -117,7 +145,7 @@
 	  struct part_ent *next = p->next;
 
 	  if (!ret)
-		ret = hook (p->name);
+		ret = myhook (p->name);
 	  grub_free (p);
 	  p = next;
 	}
@@ -129,31 +157,7 @@
   return 0;
 }
 
-  int i

Re: Non-static variables and nested function pointers [bug #28392]

[Grégoire Sutre]

Seth Goldberg wrote:

  Your problem is probably lack of executable stack support, or at least 
you haven't linked your application with a linker mapfile that specifies 
an executable stack -- the callbacks require the use of trampolines to 
access local variables, which require an executable stack.


Thanks a lot for your answer ! That explains it :-)

I tried linking with -Wl,-z,execstack, but with no success, even though 
readelf shows a new header with this option:


Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
GNU_STACK  0x00 0x 0x 0x0 0x0 RWE 0x4

But the problem may come from NetBSD's default policy of non-executable 
stack on i386 (http://www.netbsd.org/docs/kernel/non-exec.html).


I'll have to dig further...  Thanks again,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Non-static variables and nested function pointers [bug #28392]

[Grégoire Sutre]

Seth Goldberg wrote:

 Exactly -- the presence of the execstack attribute in the segment is 
merely a request -- the kernel is free to discard it, and many OSes do, 
as you've found :).


The problem is more complex: I tried a simple example with a pointer to 
a nested function, and it runs without any segfault on NetBSD/i386.
This would suggest that, by default, the stack is executable -- at least 
if trampolines are used?


The same example segfaults on Debian/amd64 if compiled with 
-Wl,-z,noexecstack (and does not segfault otherwise).


After some digging, I found threads in the archives of the mailing list 
on the problem of executable stack on NetBSD. This led to a patch that 
is now part of trunk if I'm not mistaken.


http://lists.gnu.org/archive/html/grub-devel/2008-02/msg00095.html

I will try to test on NetBSD/amd64 and report here (in a few days), as 
NetBSD's support for non-executable stack and heap seems better on amd64.


Grégoire


#include 

int apply(void (*hook) (int *))
{
   int a = 0;
   hook(&a);
   hook(&a);
   return a;
}

int main (int argc, char *argv[])
{
   int j = 5;
   int res;

   void hook(int *n)
   {
  *n = *n + j;
  j--;
   }

   res = apply(hook);
   printf("result: %d, j=%d\n", res, j);
   return 0;
}


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Non-static variables and nested function pointers [bug #28392]

[Grégoire Sutre]

Robert Millan wrote:


The following snippet (kern/misc.c) comes to mind:

#ifdef NEED_ENABLE_EXECUTE_STACK
/* Some gcc versions generate a call to this function
   in trampolines for nested functions.  */
void __enable_execute_stack (void *addr __attribute__ ((unused)))
{
}
#endif


I was away from my email, sorry for the delay.  I hunted this
segfault down in the meantime, and I came to the same
conclusion.  If I remove `grub_CHECK_ENABLE_EXECUTE_STACK' from
configure.ac then the segfault in grub-probe disappear.



We added this for NetBSD in fact.  In that platform, GCC generates references
to this function, which are usually satisfied by libc, but we don't link with
libc, so we made it happy with an empty stub.


Yes, I realized that this was NetBSD specific from the archives of the 
mailing list -- kind of ironic ;-)


On my NetBSD/i386 5.0 system, if I compile the attached example, the 
symbol __enable_execute_stack does indeed appear in the executable 
(according to nm).  This holds for gcc 4.1.3 (the system's default 
compiler) and for gcc 4.4.1.  And the symbol __enable_execute_stack is 
found in libgcc_s.so.1.




But this is only supposed to happen when building real GRUB.  For util/
stuff, we should use the libc facility instead.  Maybe that's not the case?


I guess that it's not the case: util programs that are linked with 
kern/misc.c do not use the libc's __enable_execute_stack (if 
NEED_ENABLE_EXECUTE_STACK is defined in config.h).



In case that helps, and to continue the discussion regarding
executable stack on NetBSD, I made a few experiments on i386 and
amd64 on a simplified example (see below).  From what I could
read, NetBSD implements memory protection with PaX.

* NetBSD/i386:

 | with dummy __enable_execute_stack |
 |   no|   yes   |
-+-+-+
PaX MPROTECT off |   ok|segfault |
-+-+-+
PaX MPROTECT on  |segfault |segfault |
-+-+-+

However, in all four cases, there is no segfault if the variable
j in main is declared static.  And in that case, the generated 
executable does not contain the symbol __enable_execute_stack (according 
to nm).


* NetBSD/amd64: in all four cases, no segfault.

Also, on NetBSD/amd64, the segfault problem I experienced in grub-probe 
completely disappears.


Grégoire
#include 

#ifdef BAD
/* Some gcc versions generate a call to this function
   in trampolines for nested functions.  */
void __enable_execute_stack (void *addr __attribute__ ((unused)))
{
}
#endif

int apply(void (*hook) (int *))
{
   int a = 0;
   hook(&a);
   hook(&a);
   return a;
}

int main (int argc, char *argv[])
{
   /* static */ int j = 5;
   int res;

   void hook(int *n)
   {
  *n = *n + j;
  j--;
   }

   res = apply(hook);
   printf("result: %d, j=%d\n", res, j);
   return 0;
}
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] grub-probe support for NetBSD

[Grégoire Sutre]

Hi,

I finished adding NetBSD specific code for grub-probe.  A patch and a
changelog entry are attached.  A few notes:

- devices are required to be character (raw) devices.  In NetBSD, those
  are of the form "/dev/r[wsc]d[0-9]+[a-z]" for hard disk drives and
  CD-ROM drives, as far as I know.

- in grub_util_biosdisk_open, the disk size is obtained from the disk
  label.

- in grub_util_biosdisk_get_grub_dev, the code is derived from the Linux
  code: we get the start sector of the partition from the disk label,
  and we compare it with each partition GRUB recognizes.  I don't think
  we can do better, as device names correspond to disklabel entries, and
  those can be quite arbitrary.

- in make_device_name, I fixed what I believe to be a typo, but I may be
  wrong.

Please let me know if I missed something,

Grégoire
2009-12-27  Gregoire Sutre  

Add grub-probe support for NetBSD.

* util/getroot.c (find_root_device): Convert block device to
character device on NetBSD.
* util/probe.c (probe): Require character device on NetBSD.
* util/hostdisk.c (grub_util_biosdisk_open): NetBSD specific code.
(convert_system_partition_to_system_disk): Likewise.
(grub_util_biosdisk_get_grub_dev): Likewise.
(make_device_name): Fixed a typo in bsd_part_str.
diff -Naurp grub2_2009-12-27/util/getroot.c grub2/util/getroot.c
--- grub2_2009-12-27/util/getroot.c	2009-12-27 16:04:32.0 +0100
+++ grub2/util/getroot.c	2009-12-27 17:51:46.0 +0100
@@ -266,8 +266,14 @@ find_root_device (const char *dir, dev_t
 	  char *cwd;
 
 	  cwd = xgetcwd ();
+#if defined(__NetBSD__)
+	  /* Convert this block device to its character (raw) device */
+	  res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 3);
+	  sprintf (res, "%s/r%s", cwd, ent->d_name);
+#else
 	  res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 2);
 	  sprintf (res, "%s/%s", cwd, ent->d_name);
+#endif
 	  strip_extra_slashes (res);
 	  free (cwd);
 
diff -Naurp grub2_2009-12-27/util/grub-probe.c grub2/util/grub-probe.c
--- grub2_2009-12-27/util/grub-probe.c	2009-12-27 16:04:32.0 +0100
+++ grub2/util/grub-probe.c	2009-12-27 17:49:02.0 +0100
@@ -111,7 +111,7 @@ probe (const char *path, char *device_na
 
   if (path == NULL)
 {
-#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__NetBSD__)
   if (! grub_util_check_char_device (device_name))
 grub_util_error ("%s is not a character device.\n", device_name);
 #else
diff -Naurp grub2_2009-12-27/util/hostdisk.c grub2/util/hostdisk.c
--- grub2_2009-12-27/util/hostdisk.c	2009-12-27 16:04:32.0 +0100
+++ grub2/util/hostdisk.c	2009-12-27 18:21:15.0 +0100
@@ -97,6 +97,12 @@ struct hd_geometry
 # include 
 #endif
 
+#if defined(__NetBSD__)
+# include 
+# include /* struct disklabel */
+# include /* getrawpartition */
+#endif
+
 struct
 {
   char *drive;
@@ -191,16 +197,20 @@ grub_util_biosdisk_open (const char *nam
 return GRUB_ERR_NONE;
   }
 #elif defined(__linux__) || defined(__CYGWIN__) || defined(__FreeBSD__) || \
-  defined(__FreeBSD_kernel__) || defined(__APPLE__)
+  defined(__FreeBSD_kernel__) || defined(__APPLE__) || defined(__NetBSD__)
   {
+# if defined(__NetBSD__)
+struct disklabel label;
+# else
 unsigned long long nr;
+# endif
 int fd;
 
 fd = open (map[drive].device, O_RDONLY);
 if (fd == -1)
   return grub_error (GRUB_ERR_BAD_DEVICE, "cannot open `%s' while attempting to get disk size", map[drive].device);
 
-# if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__APPLE__)
+# if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__APPLE__) || defined(__NetBSD__)
 if (fstat (fd, &st) < 0 || ! S_ISCHR (st.st_mode))
 # else
 if (fstat (fd, &st) < 0 || ! S_ISBLK (st.st_mode))
@@ -214,6 +224,8 @@ grub_util_biosdisk_open (const char *nam
 if (ioctl (fd, DIOCGMEDIASIZE, &nr))
 # elif defined(__APPLE__)
 if (ioctl (fd, DKIOCGETBLOCKCOUNT, &nr))
+# elif defined(__NetBSD__)
+if (ioctl (fd, DIOCGDINFO, &label))
 # else
 if (ioctl (fd, BLKGETSIZE64, &nr))
 # endif
@@ -224,14 +236,16 @@ grub_util_biosdisk_open (const char *nam
 
 close (fd);
 
-#if defined (__APPLE__)
+# if defined (__APPLE__)
 disk->total_sectors = nr;
-#else
+# elif defined(__NetBSD__)
+disk->total_sectors = label.d_secperunit;
+# else
 disk->total_sectors = nr / 512;
 
 if (nr % 512)
   grub_util_error ("unaligned device size");
-#endif
+# endif
 
 grub_util_info ("the size of %s is %llu", name, disk->total_sectors);
 
@@ -683,7 +697,7 @@ make_device_name (int drive, int dos_par
 dos_part_str = xasprintf (",%d", dos_part + 1);
 
   if (bsd_part >= 0)
-bsd_part_str = xasprintf (",%c", dos_part + 'a');
+bsd_part_str = xasprintf (",%c", bsd_part + 'a');
 
   ret = xasprintf ("%s%s%s", map[drive].drive,
dos_part_str ?

Re: [PATCH] grub-probe support for NetBSD

[Grégoire Sutre]

Hi,

Here is a new version of the patch (the change log remains the same as 
in the original email).  The new version does not attempt to read from a 
floppy device in grub_util_biosdisk_open.  Indeed, on NetBSD, reading 
from the floppy device (e.g. cat /dev/fd0a) takes a _long_ time to abort 
when there is no floppy in the drive.


With this patch (and with a few other changes [1] discussed here or on 
bug-grub), I could successfully install grub with grub-install on a USB 
stick (and then boot from it) on NetBSD 5.0 i386 and amd64.


Regards,

Grégoire

[1] 
http://pkgsrc-wip.cvs.sourceforge.net/viewvc/pkgsrc-wip/wip/grub2-current/patches/
--- util/getroot.c.orig 2009-12-29 00:43:31.0 +0100
+++ util/getroot.c
@@ -266,8 +266,14 @@ find_root_device (const char *dir, dev_t
  char *cwd;
 
  cwd = xgetcwd ();
+#if defined(__NetBSD__)
+ /* Convert this block device to its character (raw) device */
+ res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 3);
+ sprintf (res, "%s/r%s", cwd, ent->d_name);
+#else
  res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 2);
  sprintf (res, "%s/%s", cwd, ent->d_name);
+#endif
  strip_extra_slashes (res);
  free (cwd);
 
--- util/grub-probe.c.orig  2009-12-29 00:43:31.0 +0100
+++ util/grub-probe.c
@@ -111,7 +111,7 @@ probe (const char *path, char *device_na
 
   if (path == NULL)
 {
-#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__NetBSD__)
   if (! grub_util_check_char_device (device_name))
 grub_util_error ("%s is not a character device.\n", device_name);
 #else
--- util/hostdisk.c.orig2009-12-29 00:43:31.0 +0100
+++ util/hostdisk.c
@@ -97,6 +97,15 @@ struct hd_geometry
 # include 
 #endif
 
+#if defined(__NetBSD__)
+# include 
+# include /* struct disklabel */
+# include /* getrawpartition */
+# ifndef RAW_FLOPPY_MAJOR
+#  define RAW_FLOPPY_MAJOR 9
+# endif /* ! RAW_FLOPPY_MAJOR */
+#endif
+
 struct
 {
   char *drive;
@@ -191,16 +200,20 @@ grub_util_biosdisk_open (const char *nam
 return GRUB_ERR_NONE;
   }
 #elif defined(__linux__) || defined(__CYGWIN__) || defined(__FreeBSD__) || \
-  defined(__FreeBSD_kernel__) || defined(__APPLE__)
+  defined(__FreeBSD_kernel__) || defined(__APPLE__) || defined(__NetBSD__)
   {
+# if defined(__NetBSD__)
+struct disklabel label;
+# else
 unsigned long long nr;
+# endif
 int fd;
 
 fd = open (map[drive].device, O_RDONLY);
 if (fd == -1)
   return grub_error (GRUB_ERR_BAD_DEVICE, "cannot open `%s' while 
attempting to get disk size", map[drive].device);
 
-# if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__APPLE__)
+# if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__APPLE__) 
|| defined(__NetBSD__)
 if (fstat (fd, &st) < 0 || ! S_ISCHR (st.st_mode))
 # else
 if (fstat (fd, &st) < 0 || ! S_ISBLK (st.st_mode))
@@ -214,6 +227,11 @@ grub_util_biosdisk_open (const char *nam
 if (ioctl (fd, DIOCGMEDIASIZE, &nr))
 # elif defined(__APPLE__)
 if (ioctl (fd, DKIOCGETBLOCKCOUNT, &nr))
+# elif defined(__NetBSD__)
+/* Do not attempt to read from a floppy device as it may take a long
+   time before aborting if there is no floppy in the drive.  */
+if ((major(st.st_rdev) == RAW_FLOPPY_MAJOR) ||
+   ioctl (fd, DIOCGDINFO, &label))
 # else
 if (ioctl (fd, BLKGETSIZE64, &nr))
 # endif
@@ -224,14 +242,16 @@ grub_util_biosdisk_open (const char *nam
 
 close (fd);
 
-#if defined (__APPLE__)
+# if defined (__APPLE__)
 disk->total_sectors = nr;
-#else
+# elif defined(__NetBSD__)
+disk->total_sectors = label.d_secperunit;
+# else
 disk->total_sectors = nr / 512;
 
 if (nr % 512)
   grub_util_error ("unaligned device size");
-#endif
+# endif
 
 grub_util_info ("the size of %s is %llu", name, disk->total_sectors);
 
@@ -683,7 +703,7 @@ make_device_name (int drive, int dos_par
 dos_part_str = xasprintf (",%d", dos_part + 1);
 
   if (bsd_part >= 0)
-bsd_part_str = xasprintf (",%c", dos_part + 'a');
+bsd_part_str = xasprintf (",%c", bsd_part + 'a');
 
   ret = xasprintf ("%s%s%s", map[drive].drive,
dos_part_str ? : "",
@@ -853,6 +873,26 @@ convert_system_partition_to_system_disk 
 }
   return path;
 
+#elif defined(__NetBSD__)
+  /* NetBSD uses "/dev/r[wsc]d[0-9]+[a-z]".  */
+  char *path = xstrdup (os_dev);
+  if (strncmp ("/dev/rwd", path, 8) == 0 ||
+  strncmp ("/dev/rsd", path, 8) == 0 ||
+  strncmp ("/dev/rcd", path, 8) == 0)
+{
+  char *q;
+  q = path + strlen(path) - 1;/* last character */
+  if (grub_isalpha(*q) && grub_isdigit(*(q-1)))
+{
+  int rawpart;
+  rawpart = getrawpartition();
+  if (rawpart < 0)
+rawpart = 3;/* default on i386 */
+  *q = 'a' + rawpart;
+}
+}
+  retu

Re: Compilation under MacOSX

[Grégoire Sutre]

Hi Yves,


the main trunk (mainline) can be compile on Mac OSX (snow leopard) but i need to add a 
LDFLAGS="-lintl" so utils like grub-mkimage can link.


I experienced a similar problem when compiling trunk on NetBSD:

http://savannah.gnu.org/bugs/?28356

Adding LDFLAGS="-lintl" also solved the problem, but a better (?) option 
might be to use the variable LIBINTL that is set by the macro 
AM_GNU_GETTEXT (see the above bug report for some details and for a 
possible patch).


Best,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Config file generation failure on Linux

[Grégoire Sutre]

Hi,

Config file generation with grub-mkconfig fails on Linux when grub is 
not installed in the same prefix as gettext, since the script 
util/grub.d/10_linux.in contains:


. ${bindir}/gettext.sh

A possible patch is given below, but there are surely other ways to deal 
with it.  The patch simply falls back to echo instead of gettext if the 
gettext binary cannot be found.


Thanks for your time,

Grégoire

--- util/grub.d/10_linux.in.orig2009-12-30 15:37:01.0 +0100
+++ util/grub.d/10_linux.in
@@ -22,7 +22,12 @@ bind...@bindir@
 libd...@libdir@
 . ${libdir}/grub/grub-mkconfig_lib

-. ${bindir}/gettext.sh
+if $(which gettext >/dev/null 2>/dev/null) ; then
+  gettext="gettext"
+else
+  gettext="echo"
+fi
+
 export textdoma...@package@
 export textdomaind...@localedir@

@@ -54,9 +59,9 @@ linux_entry ()
   recovery="$3"
   args="$4"
   if ${recovery} ; then
-title="$(gettext "%s, with Linux %s (recovery mode)")"
+title="$(${gettext} "%s, with Linux %s (recovery mode)")"
   else
-title="$(gettext "%s, with Linux %s")"
+title="$(${gettext} "%s, with Linux %s")"
   fi
   printf "menuentry \"${title}\" {\n" "${os}" "${version}"
   if [ -z "${prepare_boot_cache}" ]; then


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Handling of TARGET_IMG_LDFLAGS_AC in configure (bug #28335)

[Grégoire Sutre]

Hi,

The following patch fixes bug report #28335.

http://savannah.gnu.org/bugs/?28335

The definitions and uses of TARGET_IMG_LDFLAGS_AC have been changed so 
that, in all cases, a number is supposed to be appended to it.


In particular, in the case where a linker script if present (lines 
311-314), the definition of TARGET_IMG_LDFLAGS_AC is now the same as the 
definition of TARGET_IMG_LDFLAGS.


Best regards,

Grégoire
2009-12-30 Gregoire Sutre  

* configure.ac: all definitions and uses of TARGET_IMG_LDFLAGS_AC now
expect a number appended to it.
* acinclude.m4 (grub_PROG_OBJCOPY_ABSOLUTE): ${TARGET_IMG_LDFLAGS_AC}
expects a number appended to it.
--- acinclude.m4.orig   2009-12-30 22:05:08.0 +0100
+++ acinclude.m42009-12-30 21:56:05.0 +0100
@@ -93,7 +93,7 @@
 fi
 grub_cv_prog_objcopy_absolute=yes
 for link_addr in 0x2000 0x8000 0x7C00; do
-  if AC_TRY_COMMAND([${CC-cc} ${CFLAGS} -nostdlib ${TARGET_IMG_LDFLAGS_AC} 
-Wl,-Ttext -Wl,$link_addr conftest.o -o conftest.exec]); then :
+  if AC_TRY_COMMAND([${CC-cc} ${CFLAGS} -nostdlib 
${TARGET_IMG_LDFLAGS_AC}$link_addr conftest.o -o conftest.exec]); then :
   else
 AC_MSG_ERROR([${CC-cc} cannot link at address $link_addr])
   fi
--- configure.ac.orig   2009-12-30 22:34:06.0 +0100
+++ configure.ac2009-12-30 22:37:04.0 +0100
@@ -311,7 +311,7 @@
 if test -f "${srcdir}/conf/${target_cpu}-${platform}-${host_os}-img-ld.sc"; 
then
   
TARGET_IMG_LDSCRIPT='$(top_srcdir)'"/conf/${target_cpu}-${platform}-${host_os}-img-ld.sc"
   TARGET_IMG_LDFLAGS="-Wl,-T${TARGET_IMG_LDSCRIPT}  -Wl,-Ttext,"
-  
TARGET_IMG_LDFLAGS_AC="-Wl,-T${srcdir}/conf/${target_cpu}-${platform}-${host_os}-img-ld.sc"
+  
TARGET_IMG_LDFLAGS_AC="-Wl,-T${srcdir}/conf/${target_cpu}-${platform}-${host_os}-img-ld.sc
  -Wl,-Ttext,"
 else
   TARGET_IMG_LDSCRIPT=
   TARGET_IMG_LDFLAGS='-Wl,-N  -Wl,-Ttext,'
@@ -446,7 +446,7 @@
 if test "x$target_cpu" = xi386; then
   if test ! -z "$TARGET_IMG_LDSCRIPT"; then
 # Check symbols provided by linker script.
-CFLAGS="$TARGET_CFLAGS -nostdlib $TARGET_IMG_LDFLAGS_AC 
-Wl,-Ttext,8000,--defsym,___main=0x8100"
+CFLAGS="$TARGET_CFLAGS -nostdlib 
${TARGET_IMG_LDFLAGS_AC}8000,--defsym,___main=0x8100"
   fi
   if test "x$TARGET_APPLE_CC" != x1 ; then
 grub_CHECK_BSS_START_SYMBOL
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] grub-probe support for NetBSD

[Grégoire Sutre]

Robert Millan wrote:

On Tue, Dec 29, 2009 at 02:31:46AM +0100, Grégoire Sutre wrote:

+#if defined(__NetBSD__)
+ /* Convert this block device to its character (raw) device */
+ res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 3);
+ sprintf (res, "%s/r%s", cwd, ent->d_name);
+#else
  res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 2);
  sprintf (res, "%s/%s", cwd, ent->d_name);
+#endif


Can you avoid code duplication here?  Something like:

#ifdef __NetBSD__
  const char *template = "%s/r%s";
#else
  const char *template = "%s/%s";
#endif


Indeed, it's better.  But we also need a variable for the extra length 
(3 for NetBSD and 2 otherwise).  I have updated the patch.


http://pkgsrc-wip.cvs.sourceforge.net/viewvc/*checkout*/pkgsrc-wip/wip/grub2-current/patches/patch-grub-probe-netbsd

The patch is not finished yet: as discussed on IRC, I'll try to 
factorize Linux and NetBSD code in grub_util_biosdisk_get_grub_dev, and 
to fix the floppy problem.


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] grub-probe support for NetBSD

[Grégoire Sutre]

Vladimir 'φ-coder/phcoder' Serbinenko wrote:


One byte is cheap. No need for gimmicks just to save one byte. You can
always allocate with +3


Ok :-)

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Disable -Werror when error attribute generates warnings

[Grégoire Sutre]

Hi,

With an older version of gcc that does not understand the error 
attribute, gcc generates warnings when compiling files that include 
include/grub/list.h.  Since TARGET_CFLAGS contains -Werror by default, 
the build of modules fails.


The following patch checks whether the C compiler supports the error 
attribute without warning, and disables -Werror if that is not the case 
(as otherwise the build will fail).


http://pkgsrc-wip.cvs.sourceforge.net/viewvc/*checkout*/pkgsrc-wip/wip/grub2-current/patches/patch-gcc-warning-on-error-attribute

This is merely a suggestion.  I'm no autoconf expert, and I'm not even 
sure that this kind of check is a good idea, as configure has the option 
--disable-werror that can be used anyway.


Grégoire

p.s. I did not see this problem when I tested the patch discussed in the 
thread [1] as I focused on building utilities at that time, sorry.


[1] http://lists.gnu.org/archive/html/grub-devel/2009-12/msg00324.html


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Config file generation failure on Linux (gettext.sh)

[Grégoire Sutre]

Robert Millan wrote:

A possible patch is given below, but there are surely other ways to deal  
with it.  The patch simply falls back to echo instead of gettext if the  
gettext binary cannot be found.


It'd be better if this was an autoconf check.


You mean with an AC_PATH_PROG(GETTEXTBIN, gettext, echo) and substitute 
GETTEXTBIN in 10_linux?


But, in order to have a robust configuration script, it would still be 
desirable to check in 10_linux whether @GETTEXTBIN@ exists and revert to 
echo otherwise, wouldn't it?


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Disable -Werror when error attribute generates warnings

[Grégoire Sutre]

Colin Watson wrote:


Instead of this, why not only use the attribute if it's available? I
couldn't find an entry about it in GCC's human-readable change
summaries, but support was committed on 2007-09-23 so I think it's
available from GCC 4.3.

I use this GNUC_PREREQ approach in other projects and rather like it. It
could be extended to cover our other uses of attributes quite easily.


Nice trick :-)

I confirm that this patch works with gcc 4.1.3 on NetBSD.  And it's 
definitely better than completely disabling -Werror...


Thanks,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Check for device type (block/character) in grub-setup?

[Grégoire Sutre]

Hi,

I'm wondering why there is no check for device type (block/character) in 
grub-setup.c whereas the function probe() in grub-probe.c exits with 
error if the device is not of the expected type. Shouldn't there be a 
similar check in grub-setup?


Currently, on NetBSD (with in-progress patches), grub-setup fails with 
an `out of disk error' on a block device as the detected disk size is 0, 
whereas grub-probe exits gracefully with an error message telling that 
the input device is not a character device.  I guess that a similar 
behavior could be observed on FreeBSD, but I can't actually test this.


Thanks,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Config file generation failure on Linux (gettext.sh)

[Grégoire Sutre]

Robert Millan wrote:

On Sun, Jan 03, 2010 at 05:50:08PM +0100, Robert Millan wrote:

As long as the script complains appropiately and exits non-zero if gettext
was installed, I would consider it robust (keep in mind grub.cfg isn't

^^^

I meant "wasn't" of course.


Ok, here is the new patch.  This one also takes care of 10_kfreebsd, as 
it uses gettext in the same way as 10_linux.  Please let me know if I 
missed something.


Thanks,

Grégoire
--- configure.ac.orig   2010-01-03 23:34:44.0 +0100
+++ configure.ac
@@ -179,6 +179,7 @@ test "x$GCC" = xyes || AC_MSG_ERROR([GCC
 
 AC_GNU_SOURCE
 AM_GNU_GETTEXT([external])
+AC_PATH_PROG([GETTEXTBIN], [gettext], [echo])
 AC_SYS_LARGEFILE
 
 # Identify characteristics of the host architecture.
--- util/grub.d/10_kfreebsd.in.orig 2010-01-02 14:42:38.0 +0100
+++ util/grub.d/10_kfreebsd.in
@@ -20,9 +20,14 @@ pref...@prefix@
 exec_pref...@exec_prefix@
 bind...@bindir@
 libd...@libdir@
+gette...@gettextbin@
 . ${libdir}/grub/grub-mkconfig_lib
 
-. ${bindir}/gettext.sh
+if [ "x${gettext}" != "xecho" ] && ! test -x "${gettext}" ; then
+  echo "10_kfreebsd: error: \`${gettext}' is missing." >&2
+  exit 1
+fi
+
 export textdoma...@package@
 export textdomaind...@localedir@
 
@@ -37,7 +42,7 @@ kfreebsd_entry ()
   version="$2"
   recovery="$3"# not used yet
   args="$4"# not used yet
-  title="$(gettext "%s, with kFreeBSD %s")"
+  title="$(${gettext} "%s, with kFreeBSD %s")"
   printf "menuentry \"${title}\" {\n" "${os}" "${version}"
   if [ -z "${prepare_boot_cache}" ]; then
 prepare_boot_cache="$(prepare_grub_to_access_device ${GRUB_DEVICE_BOOT} | 
sed -e "s/^/\t/")"
--- util/grub.d/10_linux.in.orig2010-01-02 14:42:38.0 +0100
+++ util/grub.d/10_linux.in
@@ -20,9 +20,14 @@ pref...@prefix@
 exec_pref...@exec_prefix@
 bind...@bindir@
 libd...@libdir@
+gette...@gettextbin@
 . ${libdir}/grub/grub-mkconfig_lib
 
-. ${bindir}/gettext.sh
+if [ "x${gettext}" != "xecho" ] && ! test -x "${gettext}" ; then
+  echo "10_linux: error: \`${gettext}' is missing." >&2
+  exit 1
+fi
+
 export textdoma...@package@
 export textdomaind...@localedir@
 
@@ -54,9 +59,9 @@ linux_entry ()
   recovery="$3"
   args="$4"
   if ${recovery} ; then
-title="$(gettext "%s, with Linux %s (recovery mode)")"
+title="$(${gettext} "%s, with Linux %s (recovery mode)")"
   else
-title="$(gettext "%s, with Linux %s")"
+title="$(${gettext} "%s, with Linux %s")"
   fi
   printf "menuentry \"${title}\" {\n" "${os}" "${version}"
   if [ -z "${prepare_boot_cache}" ]; then
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] grub-probe support for NetBSD

[Grégoire Sutre]

Hi,

This the new version of the patch adding grub-probe (and grub-setup) 
support for NetBSD.  The main changes are:


- in function grub_util_biosdisk_get_grub_dev(), merge of the new
  NetBSD code with the original Linux code.

- re-enabled disk size detection for floppy.  This is done as for the
  other devices via the disklabel.  Hence, the floppy must have been
  labeled before, which is the recommended usage according to [1].
  I couldn't find another way to get the disk size for floppies.
  For the record, an example of successfull installation on a floppy
  (with grub-install) is also attached.

- as the floppy driver takes a long time to abort when the floppy drive
  is empty, the option NORETRY is set on the floppy driver.  The running
  time of grub-probe gets divided by 6 (30s instead of 3 minutes).  It
  is still long, but is also due to the fact that grub-probe tries to
  read the floppy 4 times.  Of course, this is only if the device map
  contains a floppy device, and grub-install --no-floppy does not suffer
  from this problem.

Thanks,

Grégoire

[1] http://www.netbsd.org/docs/guide/en/chap-rmmedia.html
Description: adds support for NetBSD in GRUB utilities.
See: http://lists.gnu.org/archive/html/grub-devel/2009-12/msg00432.html

Change log
--
2010-??-??  Gregoire Sutre  

Add grub-probe support for NetBSD.

* util/getroot.c (find_root_device): Convert block device to
character device on NetBSD.
* util/probe.c (probe): Require character device on NetBSD.
* util/hostdisk.c: NetBSD specific headers.
(configure_device_driver): new function to tune device driver
parameters (currently only for NetBSD floppy driver).
(grub_util_biosdisk_open): NetBSD specific code (get disk size
via disklabel ioctl).
(open_device): call configure_device_driver on NetBSD.
(convert_system_partition_to_system_disk): NetBSD specific code.
(device_is_wholedisk): Likewise.
(grub_util_biosdisk_get_grub_dev): Likewise.
(make_device_name): Fixed a typo in bsd_part_str.
* configure.ac: check for opendisk() and getrawpartition() on
NetBSD and set LIBUTIL.
* Makefile.in: add LIBUTIL to LIBS.

--- util/getroot.c.orig 2010-01-02 14:42:38.0 +0100
+++ util/getroot.c
@@ -264,10 +264,17 @@ find_root_device (const char *dir, dev_t
  /* Found!  */
  char *res;
  char *cwd;
+#if defined(__NetBSD__)
+ /* Convert this block device to its character (raw) device.  */
+ const char *template = "%s/r%s";
+#else
+ /* Keep the device name as it is.  */
+ const char *template = "%s/%s";
+#endif
 
  cwd = xgetcwd ();
- res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 2);
- sprintf (res, "%s/%s", cwd, ent->d_name);
+ res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 3);
+ sprintf (res, template, cwd, ent->d_name);
  strip_extra_slashes (res);
  free (cwd);
 
--- util/grub-probe.c.orig  2010-01-02 14:42:38.0 +0100
+++ util/grub-probe.c
@@ -111,7 +111,7 @@ probe (const char *path, char *device_na
 
   if (path == NULL)
 {
-#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__NetBSD__)
   if (! grub_util_check_char_device (device_name))
 grub_util_error ("%s is not a character device.\n", device_name);
 #else
--- util/hostdisk.c.orig2010-01-02 14:42:38.0 +0100
+++ util/hostdisk.c
@@ -97,6 +97,18 @@ struct hd_geometry
 # include 
 #endif
 
+#if defined(__NetBSD__)
+# include 
+# include /* struct disklabel */
+# ifdef HAVE_GETRAWPARTITION
+#  include /* getrawpartition */
+# endif /* HAVE_GETRAWPARTITION */
+# include 
+# ifndef RAW_FLOPPY_MAJOR
+#  define RAW_FLOPPY_MAJOR 9
+# endif /* ! RAW_FLOPPY_MAJOR */
+#endif /* defined(__NetBSD__) */
+
 struct
 {
   char *drive;
@@ -121,6 +133,31 @@ have_devfs (void)
 }
 #endif /* __linux__ */
 
+#if defined(__NetBSD__)
+/* Adjust device driver parameters.  This function should be called just
+   after successfully opening the device.  For now, it simply prevents the
+   floppy driver from retrying operations on failure, as otherwise the
+   driver takes a while to abort when there is no floppy in the drive.  */
+static void
+configure_device_driver (int fd)
+{
+  struct stat st;
+
+  if (fstat (fd, &st) < 0 || ! S_ISCHR (st.st_mode))
+return;
+  if (major(st.st_rdev) == RAW_FLOPPY_MAJOR)
+{
+  int floppy_opts;
+
+  if (ioctl (fd, FDIOCGETOPTS, &floppy_opts) == -1)
+   return;
+  floppy_opts |= FDOPT_NORETRY;
+  if (ioctl (fd, FDIOCSETOPTS, &floppy_opts) == -1)
+   return;
+}
+}
+#endif /* defined(__NetBSD__) */
+
 static int
 find_grub_drive (const char *name)
 {
@@ -191,16 +228,20 @@ grub_util_biosdisk_open (const char *nam
 return GRUB_ERR_NONE;
   }
 #elif defined(__li

Invalid symbol table on NetBSD boot

[Grégoire Sutre]

Hi,

When booting a NetBSD 5.0 (i386 or amd64) kernel with knetbsd (as per
docs/grub.cfg), the first message displayed by the NetBSD kernel is:

[ Kernel symbol table invalid! ]

and after boot, the device /dev/ksyms is not configured.  The same
problem occurs with multiboot instead of knetbsd (for i386 only, as the
NetBSD kernel does not support multiboot on amd64).

However, the system after boot seems to work fine.  This issue
apparently only affects the in-kernel debugger.

A similar problem is mentioned in the archives of the mailing list [1]. 
 I tried to adapt the patch given in [1] to the current trunk, but with 
no luck.  And the patch focuses on multiboot, so I guess it would not 
solve the problem with knetbsd.  This stuff is way above my head 
anyway...  But I'd happy to help by testing :-)


Thanks for your help,

Grégoire

[1] http://lists.gnu.org/archive/html/grub-devel/2008-09/msg00194.html



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Check for device type (block/character) in grub-setup?

[Grégoire Sutre]

Robert Millan wrote:

On Sun, Jan 03, 2010 at 03:54:49AM +0100, Grégoire Sutre wrote:

Hi,

I'm wondering why there is no check for device type (block/character) in  
grub-setup.c whereas the function probe() in grub-probe.c exits with  
error if the device is not of the expected type. Shouldn't there be a  
similar check in grub-setup?


Yes.

I guess that a similar  
behavior could be observed on FreeBSD, but I can't actually test this.


Please make the code generic if possible (i.e. accept both character and
block devices).


I don't understand what you mean here.  I assumed that it's better to 
use character devices when accessing disks in GRUB utils, isn't it so? 
Moreover, at least on NetBSD, when a block device is mounted, it cannot 
be opened (device busy), and this would be a problem with grub-probe.


However, from a user view-point, it could be nice on NetBSD to accept 
stripped device names (e.g. only `wd0d' or even `wd0') and automatically 
get the associated device file (with opendisk(3)). This is what system 
tools do, e.g. `disklabel wd0' or `fdisk wd0' actually opens 
`/dev/rwd0d' (on i386).


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Invalid symbol table on NetBSD boot

[Grégoire Sutre]

Robert Millan wrote:


I suggest you test if GRUB Legacy's Multiboot loader supports this
properly, as the code I used derives from that.


Yes, the problem disappears with GRUB Legacy's multiboot.  Moreover, I 
noticed another issue: the command line is stripped (first word missing) 
with GRUB 2's multiboot. Here are the logs.  After setting the root 
variable appropriately, I get:


--- With GRUB Legacy ---

grub> kernel /netbsd.generic -z root=wd0a
   [Multiboot-elf, ...]
grub> boot

And everything works as expected: silent boot (-z), root device not 
asked by the kernel, /dev/ksyms working.  The kernel says (dmesg):


multiboot: Information structure flags: 0x03e7
multiboot: Boot loader: GNU GRUB 0.97
multiboot: Command line: /netbsd.generic -z root=wd0a
multiboot: 638 KB lower memory, 1562048 KB upper memory
multiboot: Symbol table at 0xc0b980d4, length 519152 bytes
multiboot: String table at 0xc0c16cc4, length 505776 bytes

--- With GRUB 2 ---

grub> multiboot /netbsd.generic -z root=wd0a
grub> boot

And the kernel here does not boot silently, but takes into account 
`root=wd0a'.  As reported before, /dev/ksyms does not work.  The kernel 
says (dmesg):


multiboot: Information structure flags: 0x0247
multiboot: Boot loader: GRUB 1.97
multiboot: Command line: -z root=wd0a
multiboot: 638 KB lower memory, 1562048 KB upper memory

Nothing regarding symbol or string tables.  See also how the command 
line seen by the kernel is missing `/netbsd.generic' w.r.t. GRUB Legacy. 
 If I use


grub> multiboot /netbsd.generic dummy -z root=wd0a

then the kernel boots silently (-z), and command line reported by the 
kernel is `dummy -z root=wd0a'.

---



I believe NetBSD kernel developers (that'd be jmmv) implemented Multiboot
support in order to avoid duplication of efforts.


AFAICS, it's implemented only for i386 in NetBSD stable.


I think it's appropiate
to consider knetbsd loader as "legacy" and resolve this problem in our
multiboot loader.


Ok.

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Invalid symbol table on NetBSD boot

[Grégoire Sutre]

Robert Millan wrote:


grub> multiboot /netbsd.generic -z root=wd0a


There was an intentional backward-incompatible (but still compatible with
the specification) change.  The equivalent command on GRUB 2 would be:

grub> multiboot /netbsd.generic /netbsd.generic -z root=wd0a


Ok.


First argument is the file being open;  in GRUB Legacy it's implicitly also
the first arg passed to payload, which is less flexible than letting user
specify it.  It doesn't have to be the filename at all, and usually the
payload doesn't need this information.


I don't know how it is used in NetBSD, the only difference I could 
observe is the sysctl parameter `machdep.booted_kernel' which is set to 
the file name of the booted kernel (or empty if the information could 
not be derived from the multiboot command-line).



I'm not sure if this explains your missing word problem, but it sounds like
it could be related.


Yes, it sure explains the problem, thanks!  I had a look at the 
multiboot command-line parsing in the NetBSD kernel: the first argument 
in the command-line is ignored (here `-z') as it is assumed to be the 
kernel file name.


Anyway, the above solution (duplicating the kernel file name) is simple 
enough. :-)


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Build error: "ENABLE_NLS" is not defined

[Grégoire Sutre]

Hi,

When NLS is disabled or not supported, the macro ENABLE_NLS is not 
defined, and this breaks compilation of GRUB modules as they are 
compiled with -Werror -Wundef by default.  This is with bazaar trunk.


$ ./autogen.sh && ./configure && gmake
[...]
gcc -Ikern -I./kern -nostdinc -isystem /usr/include -I./include -I. 
-I./include -Wall -W  -Os -DGRUB_MACHINE_PCBIOS=1 -Wall -W -Wshadow 
-Wpointer-arith -Wmissing-prototypes-Wundef 
-Wstrict-prototypes -g -falign-jumps=1 -falign-loops=1 
-falign-functions=1 -mno-mmx -mno-sse -mno-sse2 -mno-3dnow -m32 
-fno-stack-protector -mno-stack-arg-probe -Werror -fno-builtin -mrtd 
-mregparm=3 -m32   -MD -c -o kernel_img-kern_err.o kern/err.c

In file included from kern/err.c:23:
./include/grub/i18n.h:29:5: error: "ENABLE_NLS" is not defined
gmake: *** [kernel_img-kern_err.o] Error 1

A possible fix consists in replacing the #if ENABLE_NLS by #if 
defined(ENABLE_NLS).  There are not many instances, so they can be 
changed manually, but the following command did the trick for me 
(assuming sed is GNU sed):


find . -name '*.[ch]' -exec sed -i -e 's, ENABLE_NLS, 
defined(ENABLE_NLS),g' '{}' ';'


Best,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Build error: "ENABLE_NLS" is not defined

[Grégoire Sutre]

richardvo...@gmail.com wrote:


This fix breaks when ENABLE_NLS is defined as 0.


AFAICS, in the implementation of AM_GNU_GETTEXT, ENABLE_NLS is either 
not defined or set to 1.


But I agree that my proposed fix is too dependent on the implementation 
of AM_GNU_GETTEXT, which may change.


Another option would be to replace #if ENABLE_NLS by #if 
defined(ENABLE_NLS) && ENABLE_NLS.



This gcc bug is blocking a better solution:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38105


I didn't know that this was a gcc bug.  It felt like a normal behavior 
to me to get an error with both -Wundef and -Werror in that case.  From 
the gcc man page:


 -Werror
 Make all warnings into errors.

 -Wundef
 Warn if an undefined identifier is evaluated in an #if directive.


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Build error: "ENABLE_NLS" is not defined

[Grégoire Sutre]

richardvo...@gmail.com wrote:


Another option would be to replace #if ENABLE_NLS by #if defined(ENABLE_NLS)
&& ENABLE_NLS.


I know the C compiler short-circuits &&, if the preprocessor does also
then this looks like the best solution.  If not, then nested #if.


Yes the preprocessor also short-circuits (I tested a small example to be 
sure, with gcc 4.1 and gcc 4.4).  So the automatic replacement command 
becomes:


find . -name '*.[ch]' -exec sed -i -e 's, ENABLE_NLS, 
(defined(ENABLE_NLS) \&\& ENABLE_NLS),g' '{}' ';'


Grégoire

p.s. By the way, this kind of contruct appears in the autoconf manual 
for a similar problem (middle of page):


http://www.gnu.org/software/autoconf/manual/html_node/Generic-Declarations.html#Generic-Declarations


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[multiboot] abstractmbi, modules

[Grégoire Sutre]

Hi,

I made some tests with the people/phcoder/abstractmbi branch, but I'm a 
bit late reporting them...  Better late than never I guess :-)


I compiled the branch and installed GRUB from Linux on a USB stick, and 
then booted my NetBSD box from the stick, with multiboot.  It worked 
like a charm. :-)


Regarding the question (on irc) whether it also works with modules, it 
turns out that NetBSD/i386 kernel multiboot code lacks module support 
(in -current).  Therefore, to test modules, I tried booting Xen 3.3 
(with NetBSD as DOM0), with:


grub> multiboot (...)/xen.gz console=vga
grub> module (...)/netbsd-XEN3PAE_DOM0 console=pc
grub> boot

and I got an error "Panic on CPU0" from Xen (before booting DOM0).  This 
is with the abstractmbi branch, I didn't try with GRUB trunk.  There are 
some reports in the archives of grub-devel regarding problems with Xen 
3.3, so maybe this is a known issue with GRUB.  Note that with NetBSD's 
boot loader, which is also able to multiboot with modules, doing:


load /netbsd-XEN3PAE_DOM0 console=pc
multiboot /xen.gz console=vga

works fine.

Grégoire



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[multiboot] command-line format

[Grégoire Sutre]

Hi,

Vladimir just sent a patch regarding this issue, but I had this long 
(sorry!) email almost finished already, so here it is.  And this might 
give more context for Vladimir's email.


As mentioned in a previous thread, I had some problems getting multiboot 
options recognized by the NetBSD kernel, and this was actually due to 
the fact that GRUB Legacy implicitly passes the booted file as first 
argument whereas GRUB 2 doesn't [1].


I started a thread on NetBSD's port-i386 mailing list [2] on the format 
of the multiboot command-line, and the discussion moved towards the 
change in GRUB regarding the first argument (booted kernel in GRUB 
Legacy, removed in GRUB 2).  I ended up doing tests with Xen, which is 
also multiboot-compliant, and Xen also skips the first argument in its 
command-line parsing code (well, it's a bit more complex, see [3] for 
details if you're interested).


In the end, it may be the case that most multiboot-compliant kernels out 
there still assume that the first argument is the booted file name, and 
skip it without even looking at it.  Do you know of any kernel that does 
not make this assumption?


There are also other multiboot-compliant boot loaders (e.g. NetBSD's 
boot loader).  Apart from GRUB 2, do you know of another boot-loader 
that does not put the booted file name as first argument?


I understand that the specification does not enforce the boot loader to 
follow a specific command-line format.  And, in a perfect world, 
developers should follow the specification, disregarding any particular 
implementation of the specification.  But I guess we don't live in a 
perfect world ;-)


The reference implementation (GRUB legacy) for the specification led 
kernel developers to assume a specific command-line format.  And for the 
sake of backward compatibility, all multiboot command-lines for those 
kernels should contain the booted file name as first argument.  So why 
not change the specification in this way, i.e. require that the 
command-line starts with the booted file name?  I believe that this 
would merely amount to writing down what most users of the specification 
had already in mind...


Grégoire

p.s. I guess that there are not many GRUB developers on the NetBSD
mailing lists, and, for my part, this topic is new...  Feel free to
intervene :-)

[1] http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00146.html
[2] http://mail-index.netbsd.org/port-i386/2010/01/09/msg001747.html
[3] http://mail-index.netbsd.org/port-i386/2010/01/14/msg001768.html



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [multiboot] abstractmbi, modules

[Grégoire Sutre]

Vladimir 'φ-coder/phcoder' Serbinenko wrote:


It's known problem: xen makes inappropriate assumptions about mbi placement


I'm wondering why it works with the NetBSD boot-loader.  What are these 
assumptions?


Grégoire



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [multiboot] command-line format

[Grégoire Sutre]

Seth Goldberg wrote:

  Solaris uses the first argument (and it is essential to the kernel 
loading process actually).


That's good to know, thanks.  So the list of multiboot-compliant kernels 
that (are known to) assume a GRUB Legacy command-line format becomes: 
Xen, Solaris, NetBSD.


Does anybody know how Linux works with respect to this?

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [multiboot] command-line format

[Grégoire Sutre]

Seth Goldberg wrote:

 Linux doesn't use multiboot -- it has its own convention for passing 
information between the boot loader and the kernel.


Ok.  I was refering to Robert's email mentioning wraplinux:

http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00046.html

I glanced over the code, and it looks like they only add a multiboot 
header.  I did not see any code to access the multiboot information 
structure (which contains the multiboot command line).


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Wiki page on NetBSD for GRUB developers

[Grégoire Sutre]

Hi,

I added to the wiki a NetBSD page aimed at GRUB developers with:

- how to download sources and cross-build a NetBSD kernel, and
- the status of booting NetBSD from GRUB (this has improved a lot 
recently thanks to Vladimir's work).


http://grub.enbug.org/NetBSD

Don't hesitate to send your comments or to edit the page.

Best regards,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [multiboot] command-line format

[Grégoire Sutre]

Hi Robert,

Thanks for your detailed explanation, it was really helpful to me.  I 
understand that for compatibility with some platforms, GRUB must provide 
a way to specify two potentially different file parameters:


(a) the GRUB path to the booted file; this path does not appear in the 
multiboot command-line.
(b) the path that shall be passed to the booted file; this path (if any) 
is the first argument in the multiboot command-line.


However, my first post in this thread was more about the multiboot 
specification itself.  In light of your explanations, I would re-phrase 
my suggestion as follows: in the multiboot specification, require that 
the first argument of the MB command-line be a path to the booted file 
*in a notation that is specific to the booted kernel*.  Or, if this is 
too constraining or too confusing, simply ask that the first argument is 
an informative string that does not contain kernel options (i.e. it can 
safely be skipped).  This way, the specification would be closer to the 
reference implementation (GRUB Legacy), and, more importantly, to what 
multiboot-compliant kernels already *assume* about the format of the 
command-line (NetBSD, OpenSolaris, Xen, others?).



For GRUB 2, this could be implemented by a multiboot command that, by 
default, behaves as GRUB Legacy, but also offers a way to modify the 
implicit first argument of the multiboot command-line.  Something like:


multiboot $path[;$ospath] ...

(a) $path is the GRUB path to the booted file.
(b) $ospath is the path that shall be passed to the booted file.

By default, if there is no ';' in the first argument, $ospath is set to 
the value that GRUB Legacy would have used.  Maybe a GRUB shell variable 
`multiboot_ospath' would be better than this ';' marker.


This way, we keep the extra flexibility of having different paths, but 
we also:


- keep backward compatibility with GRUB Legacy: kernels can still assume 
that the first argument of the multiboot command-line is not a kernel 
option.
- don't force users to repeat the path for kernels that are happy with 
the way GRUB Legacy worked (all of them except OpenSolaris?).  I agree 
that this point is of no concern to most users, but in some cases 
grub-mkconfig does not work (e.g. grub-mkconfig does not support 
--root-directory at the moment), and some users prefer the flexibility 
of the command line anyway.


Grégoire



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Trouble booting from a large USB hard drive

[Grégoire Sutre]

Daniel Richard G. wrote:

Knowing that Windows would only mount the 
first partition when the drive was plugged in


Does this only apply to external hard drives?  I've always put Linux as 
my first partition on my single hard drive without any problem (now it's 
with XP, but AFAIR also with 98SE before).  I mean first partition both 
``logically'' and ``physically'', e.g. what I have now is:


   Device Boot  Start End  Blocks   Id  System
/dev/sda1   *   1261120972826   83  Linux
/dev/sda226124569157276357  HPFS/NTFS

And if I remember right, the Windows installer (booting from a windows 
install CD) had the bad habit of renumbering the primary partitions if 
their logical order was not consistent with their physical order.  At 
least it did it once, and I've never played with out-of-order partition 
tables since ;-)


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [multiboot] command-line format

[Grégoire Sutre]

Hi Robert,


On Sun, Jan 17, 2010 at 01:39:48PM -0600, richardvo...@gmail.com wrote:

I think a bootloader with "universal" in its name should be doing
everything possible to avoid this.  If I want to multiboot between
Linux, NetBSD, OpenSolaris, and OpenBSD, do I load my MBR with the BSD
fork of GRUB, the Linux fork of GRUB, or the Solaris fork of GRUB?


It doesn't matter, because whichever version of GRUB you use should
generate a grub.cfg that uses multiboot command with appropiate
parameters.


According to my little experience, I believe that GRUB should not rely 
too much on an automatic generation of a correct grub.cfg.  This 
requires an os-prober command that (1) is available on the system 
installing GRUB, which is not always the case, and (2) can detect other 
installed Oses correctly, which is not always the case as well.


What if I want to install GRUB to some removable drive or floppy disk 
for the purpose of rescuing systems with a damaged MBR?  You cannot hope 
to generate a grub.cfg that will universally work, so in that case you 
must use the GRUB shell.


As you mentionned in a previous email, it is important to keep the GRUB 
shell user-friendly.  Requiring users to duplicate the first argument of 
multiboot commands is not my idea of user-friendliness.  But I agree 
that it works.


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Build error: "ENABLE_NLS" is not defined

[Grégoire Sutre]

Robert Millan wrote:


This affects gnulib/error.c and gnulib/gettext.h which would be much better
not to change, as they're being imported semi-automatically.


I understand.

But could this be actually a bug in gnulib?  The problem only occurs 
when gettext is not found and when compiling with -Wundef -Werror, so 
maybe it went unnoticed?



Perhaps you could solve this at its source?  (i.e. by defining ENABLE_NLS to
0 when gettext is unavailable).


Indeed, the problem is gone when the following line is added before the 
call to AM_GNU_GETTEXT in configure.ac:


AC_DEFINE([ENABLE_NLS], [0])

It does not really solve the problem at its source, though.  ENABLE_NLS 
is defined in AM_GNU_GETTEXT and the documentation of this macro [1] 
does not require ENABLE_NLS to be defined when gettext is not available.


Best regards,

Grégoire

[1] 
http://www.gnu.org/software/hello/manual/gettext/AM_005fGNU_005fGETTEXT.html



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [multiboot] abstractmbi, modules

[Grégoire Sutre]

Seth Goldberg wrote:

  The latest version of Solaris xVM (based on Xen 3.4) is bootable from 
GRUB2, so that confirms that the bug was fixed in 3.4, so please give 
that a try.


Indeed, Xen 3.4.2 (i386) boots fine with GRUB 2.  For the record, the 
command I used is:


grub> multiboot (...)/xen.gz
grub> module (...)/netbsd-XEN3PAE_DOM0.gz /netbsd-XEN3PAE_DOM0.gz console=pc
grub> boot

Thanks,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] nested partitions

[Grégoire Sutre]

Robert Millan wrote:


With this approach, the burden is no longer in GRUB.  Then I don't care
how weird disk layouts can become, because GRUB doesn't have to probe
them.  We can even support things like this if it makes users happy:

  (hd0,bsd2,msdos1,sun1,apple4,msdos1)


I like this generic approach very much.  And as you said, in 
non-straightforward disk layouts, the responsibility of finding the 
appropriate path to access a given partition is left to the user.


In all generality, the links between labels is a graph, for instance in 
your example above, the two occurrences of msdos1 could be the same 
partition.  How does the probing code work regarding this?  Does 
partition_iterate terminate if the graph has cycles?


Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Fix use of ENABLE_NLS (which is not always defined)

[Grégoire Sutre]

Hi,

This message concerns both gnulib and grub.  As discussed on irc and on 
the list [1], ENABLE_NLS is not used correctly, which leads to a build 
failure when gettext is not detected (or with configure option 
--disable-nls).


ENABLE_NLS is defined in AM_GNU_GETTEXT and the documentation of this 
macro [2] does not require ENABLE_NLS to be defined when gettext is not 
available.  However, uses of ENABLE_NLS assume it to be defined, which 
is not correct.


The attached patch fixes the problem for grub.

For gnulib, running the following command on the source tree should 
solve the problem:


find . -name '*.[ch]' -exec sed -i -e 's, ENABLE_NLS, 
(defined(ENABLE_NLS) \&\& ENABLE_NLS),g' '{}' ';'


Best regards,

Grégoire

[1] http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00288.html
[2] 
http://www.gnu.org/software/hello/manual/gettext/AM_005fGNU_005fGETTEXT.html 

=== modified file 'ChangeLog'
--- ChangeLog	2010-01-25 09:06:55 +
+++ ChangeLog	2010-01-25 09:24:09 +
@@ -1,5 +1,18 @@
 2010-01-25  Grégoire Sutre  
 
+	Fix a build failure (-Wundef -Werror) when ENABLE_NLS is not defined,
+	which is the case with --disabled-nls.
+
+	* gnulib/error.c: use (defined(ENABLE_NLS) && ENABLE_NLS) instead of
+	ENABLE_NLS in all #if preprocessor macros.
+	* gnulib/gettext.h: likewise.
+	* include/grub/i18n.h: likewise.
+	* util/misc.c: likewise.
+	* util/mkisofs/mkisofs.c: likewise.
+	* util/mkisofs/mkisofs.h: likewise.
+
+2010-01-25  Grégoire Sutre  
+
 	* configure.ac: Check for `limits.h'.
 	* util/misc.c: Include `' (for PATH_MAX).
 

=== modified file 'gnulib/error.c'
--- gnulib/error.c	2009-11-16 18:49:44 +
+++ gnulib/error.c	2010-01-25 09:22:09 +
@@ -28,7 +28,7 @@
 #include 
 #include 
 
-#if !_LIBC && ENABLE_NLS
+#if !_LIBC && (defined(ENABLE_NLS) && ENABLE_NLS)
 # include "gettext.h"
 # define _(msgid) gettext (msgid)
 #endif

=== modified file 'gnulib/gettext.h'
--- gnulib/gettext.h	2009-11-09 19:16:09 +
+++ gnulib/gettext.h	2010-01-25 09:22:09 +
@@ -19,7 +19,7 @@
 #define _LIBGETTEXT_H 1
 
 /* NLS can be disabled through the configure --disable-nls option.  */
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
 
 /* Get declarations of GNU message catalog functions.  */
 # include 

=== modified file 'include/grub/i18n.h'
--- include/grub/i18n.h	2010-01-01 20:32:30 +
+++ include/grub/i18n.h	2010-01-25 09:22:08 +
@@ -26,7 +26,7 @@
 extern const char *(*EXPORT_VAR(grub_gettext)) (const char *s);
 
 /* NLS can be disabled through the configure --disable-nls option.  */
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
 
 # ifdef GRUB_UTIL
 
@@ -35,7 +35,7 @@
 
 # endif /* GRUB_UTIL */
 
-#else /* ! ENABLE_NLS */
+#else /* ! (defined(ENABLE_NLS) && ENABLE_NLS) */
 
 /* Disabled NLS.
The casts to 'const char *' serve the purpose of producing warnings
@@ -48,7 +48,7 @@
 #  define grub_gettext(str) ((const char *) (str))
 # endif /* GRUB_UTIL */
 
-#endif /* ENABLE_NLS */
+#endif /* (defined(ENABLE_NLS) && ENABLE_NLS) */
 
 #ifdef GRUB_UTIL
 # define _(str) gettext(str)

=== modified file 'util/misc.c'
--- util/misc.c	2010-01-25 09:06:55 +
+++ util/misc.c	2010-01-25 09:22:07 +
@@ -598,9 +598,9 @@
 void
 grub_util_init_nls (void)
 {
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
   setlocale (LC_ALL, "");
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
-#endif /* ENABLE_NLS */
+#endif /* (defined(ENABLE_NLS) && ENABLE_NLS) */
 }

=== modified file 'util/mkisofs/mkisofs.c'
--- util/mkisofs/mkisofs.c	2010-01-08 15:22:40 +
+++ util/mkisofs/mkisofs.c	2010-01-25 09:22:06 +
@@ -640,11 +640,11 @@
   char *log_file = 0;
 
   set_program_name (argv[0]);
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
   setlocale (LC_ALL, "");
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
-#endif /* ENABLE_NLS */
+#endif /* (defined(ENABLE_NLS) && ENABLE_NLS) */
 
   if (argc < 2)
 usage();

=== modified file 'util/mkisofs/mkisofs.h'
--- util/mkisofs/mkisofs.h	2010-01-01 20:32:30 +
+++ util/mkisofs/mkisofs.h	2010-01-25 09:22:06 +
@@ -30,12 +30,12 @@
 #include 
 #include 
 
-#if ENABLE_NLS
+#if (defined(ENABLE_NLS) && ENABLE_NLS)
 
 #  include 
 #  include 
 
-#else /* ! ENABLE_NLS */
+#else /* ! (defined(ENABLE_NLS) && ENABLE_NLS) */
 
 /* Disabled NLS.
The casts to 'const char *' serve the purpose of producing warnings
@@ -43,7 +43,7 @@
On pre-ANSI systems without 'const', the config.h file is supposed to
contain "#define const".  */
 #  define gettext(Msgid) ((const char *) (Msgid))
-#endif /* ENABLE_NLS */
+#endif /* (defined(ENABLE_NLS) && ENABLE_NLS) */
 
 #define _(str) gettext(str)
 #define N_(str) str

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Multiboot's boot_device field specification + implementation bug

[Grégoire Sutre]

Hi,

I'm trying to understand the specification of the multiboot boot_device 
field.  How should this information be interpreted by a kernel that uses 
a native (non-DOS) disk label?  For instance, if the MBI passed to the 
NetBSD kernel says part1=5 and part2=part3=0xFF, does this mean:


- 6th partition in the NetBSD disk-label, or
- 6th partition in the DOS disk label (MBR)?

The specification says that part1 specifies the top-level partition 
number.  However there may be several top-level disk-labels.  For 
instance: take a disk with a DOS disk-label (in the MBR) but without any 
 UFS partition in it, and install a NetBSD disk-label on the disk.  The 
NetBSD disk-label will be written in the second sector of the disk, and 
both disk-labels will be independent and top-level.  For a NetBSD 
kernel, the top-level disklabel will be the NetBSD one, and for other 
kernels the top-level disk-label may be the DOS one.



On a related note, experiments with the multiboot example kernel show 
that setting root in GRUB has an impact on the value of the boot_device 
field:


(a) set root=(hd1,2,a) ; multiboot /mbtest ; boot
--> boot_device = 0x810100ff

(b) multiboot (hd1,2,a)/mbtest ; boot
--> boot_device = 0x8000

According to the spec, (a) is correct but (b) is wrong.  It looks like 
the boot_device field of MBI is set using the value of $root.


Best regards,

Grégoire



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Multiboot's boot_device field specification + implementation bug

[Grégoire Sutre]

Hi,


(a) set root=(hd1,2,a) ; multiboot /mbtest ; boot
--> boot_device = 0x810100ff

(b) multiboot (hd1,2,a)/mbtest ; boot
--> boot_device = 0x8000


Out of curiosity, I tried equivalent commands with GRUB Legacy, and I 
get boot_device = 0x810100ff in both cases.


Best regards,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Is it possible to have grub2's boot.img as my MBR, but have it look in a separate partition for core.img?

[Grégoire Sutre]

Wesley Smith wrote:


Thanks for your help, but no, it appears you cannot do this by simply using
"grub-install /dev/sdx". I tried this, and yes, it did make the target
partition bootable (it contained a Grub loader in its PBR). However, to ensure
the disk MBR had also been updated and was now getting its core.img file from
the /boot partition and not from the sectors immediately following it, I built
a dummy track containing just the MBR post-"grub-install" and nothing but zeros
after, and then I copied it to /dev/sda:

dd if=/dev/zero of=/track.image bs=512 count=63
dd if=/dev/sda of=/track.image bs=512 count=1 conv=notrunc
dd if=/track.image of=/dev/sda bs=512 count=63


I believe that you should replace the GRUB MBR code with an ``MS-DOS 
like'' MBR that looks for the first active partition and boots it.  Here 
you keep the old GRUB MBR code that will try to load core.img from the 
first track.


I guess that you can get such an MBR boot code from many places (I 
personally use the NetBSD boot selector) and then you simply:


dd if= of=/dev/sda bs=446 count=1

Well, I'm no expert, but that always worked for me.

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] --foo bar cmdline argument parsing support for grub-* scripts

[Grégoire Sutre]

BVK Chaitanya wrote:

I assume you are talking about /usr/bin/getopt (not libc getopt).
/usr/bin/getopt comes with util-linux package (in debian), and yes, I
too dont think it would be available on other platforms as bash or
coreutils does.


You might want to use the POSIX shell built-in getopts instead of the 
getopt binary.


http://www.opengroup.org/onlinepubs/9699919799/utilities/getopts.html

This built-in is supported by bash, ksh, NetBSD sh, and surely many 
other shells.


Best regards,

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Lexer sed post-processing instead of pragmas

[Grégoire Sutre]

Hi,

The lexer code in script/yylex.l uses pragmas that are not supported by
gcc < 4.2.  However, without these pragmas, the build fails because of
warnings in the lexer code generated by flex (recall that some targets
are built with -Werror by default).

I've discussed this issue with sethmeisterg on irc.  Of course, dropping
compatibility with gcc 4.1.3 is a possible solution, but we believe that
a better alternative would be to modify the generated lexer so that it
does not trigger any warning.

There are already some sed commands in conf/common.rmk that remove
#includes from the generated lexer.  The attached patch extends the sed
post-processing of the lexer to get rid of the warnings, and removes the
pragmas.  This way, grub can still be compiled with gcc 4.1.3 (which is
the current required version in INSTALL).

The patch also replaces the non-portable -i sed option with temporary
files.

Thanks for your comments,

Grégoire

=== modified file 'conf/common.rmk'
--- conf/common.rmk	2010-03-14 16:50:55 +
+++ conf/common.rmk	2010-03-16 14:13:32 +
@@ -95,10 +95,21 @@
 grub_bin2h_SOURCES = gnulib/progname.c util/bin2h.c
 
 # For the lexer.
+lex_fix_includes = '/^\#include[[:space:]]*<(stdio|string|errno|stdlib|unistd)\.h>/d'
+lex_fix_h  = '/^(static)?[[:space:]]*void[[:space:]]*yy_flex_strncpy[[:space:]]*\(.*\)[[:space:]]*;$$/d'
+lex_fix_c1 = '/^(static)?[[:space:]]*void[[:space:]]*yy_fatal_error[[:space:]]*\(.*\)[[:space:]]*;$$/d'
+lex_fix_c2 = '/^(static)?[[:space:]]*(int|void)[[:space:]]*yy(get_column|set_column|_fatal_error)[[:space:]]*\(.*\)$$/,/^}$$/d'
+lex_fix_c3 = '/^static[[:space:]]*void[[:space:]]*yy_flex_strncpy[[:space:]]*\(.*\)$$/{\
+s/yyscanner/yyscanner __attribute__ ((unused))/;\
+};'
+
 grub_script.yy.c grub_script.yy.h: script/yylex.l
 	$(LEX) -o grub_script.yy.c --header-file=grub_script.yy.h $(srcdir)/script/yylex.l
-	sed -i 's/^#include.*\(\|\|\|\|\)//g' grub_script.yy.h
-	sed -i 's/^#include.*\(\|\|\|\|\)//g' grub_script.yy.c
+	mv grub_script.yy.h grub_script.yy.h.tmp
+	mv grub_script.yy.c grub_script.yy.c.tmp
+	sed -r -e $(lex_fix_includes) -e $(lex_fix_h) grub_script.yy.h.tmp > grub_script.yy.h
+	sed -r -e $(lex_fix_includes) -e $(lex_fix_c1) -e $(lex_fix_c2) -e $(lex_fix_c3) grub_script.yy.c.tmp > grub_script.yy.c
+	rm grub_script.yy.h.tmp grub_script.yy.c.tmp
 DISTCLEANFILES += grub_script.yy.c grub_script.yy.h
 
 # For grub-script-check.

=== modified file 'script/yylex.l'
--- script/yylex.l	2010-01-25 16:31:14 +
+++ script/yylex.l	2010-03-16 14:11:34 +
@@ -94,12 +94,6 @@
 #define fprintf(...) 0
 #define exit(...)
 
-#pragma GCC diagnostic warning "-Wunused-variable"
-#pragma GCC diagnostic warning "-Wunused-function"
-#pragma GCC diagnostic warning "-Wunused-parameter"
-#pragma GCC diagnostic warning "-Wstrict-prototypes"
-#pragma GCC diagnostic warning "-Wmissing-prototypes"
-
 }
 
 %option ecs

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix use of ENABLE_NLS (which is not always defined)

[Grégoire Sutre]

Hi,

I just discovered your answers [1, 2] on this subject (I'm not
registered to the bug-gnulib mailing list).  Thanks for the
explanations.  I take note that Gnulib does not support use of -Wundef.

I just want to add that I find it good practice to avoid evaluating
undefined identifiers in #if directives.

In fact, any package which makes good use of Autoconf cannot support 
-Wundef.


I don't see why.  Could you please elaborate?

Best regards,

Grégoire Sutre

[1] http://lists.gnu.org/archive/html/bug-gnulib/2010-01/msg00316.html
[2] http://lists.gnu.org/archive/html/bug-gnulib/2010-01/msg00319.html


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: update-grub & Xen boot stanza generation

[Grégoire Sutre]

Bruce Edge wrote:

I actually had the kernel arg=val repeated twice originally, but the 
xen-devel guys said to use dummy=dummy.


Maybe the xen-devel guys meant the following grub command:

multiboot /boot/xen.gz dummy=dummy [other options]

which will pass to xen the following multiboot-protocol command-line in
the multiboot information structure:

dummy=dummy [other options]

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Option handling in grub-mkconfig

[Grégoire Sutre]

Hi,

The processing of option `-o' in grub-mkconfig only works when
it is the first option.  The code is:

# Check the arguments.
for option in "$@"; do
case "$option" in
[...]
-o)
shift
grub_cfg=$1
;;
--output=*)
grub_cfg=`echo "$option" | sed 's/--output=//'`
;;


For instance:

grub-mkconfig -o /tmp/tata -o /tmp/titi
rm: invalid option -- 'o'
Try `rm --help' for more information.

grub-mkconfig --output=/tmp/tata -o /tmp/titi
rm: invalid option -- 'o'
Try `rm --help' for more information.


AFAICS, other scripts (grub-install, grub-mkrescue) only support the
long option (--output=FILE).

The simpler solution would be to get rid of `-o' in grub-mkconfig,
but this would break some user/distribution scripts (e.g. Debian's
update-grub).

What do you think?

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Option handling in grub-mkconfig

[Grégoire Sutre]

On 05/21/2010 12:57 PM, Colin Watson wrote:

Thanks for the patch, it works fine.  I'm just wondering: Is keeping
`-o' worth the extra complication?

Grégoire


How about:

=== modified file 'util/grub-mkconfig.in'
--- util/grub-mkconfig.in   2010-04-19 19:25:41 +
+++ util/grub-mkconfig.in   2010-05-21 10:54:18 +
@@ -50,7 +50,13 @@ EOF
  }

  # Check the arguments.
+next_grub_cfg=false
  for option in "$@"; do
+if $next_grub_cfg; then
+   grub_cfg=$option
+   next_grub_cfg=false
+   continue
+fi
  case "$option" in
  -h | --help)
usage
@@ -59,8 +65,7 @@ for option in "$@"; do
echo "$0 (GNU GRUB ${package_version})"
exit 0 ;;
  -o)
-   shift
-   grub_cfg=$1
+   next_grub_cfg=:
;;
  --output=*)
grub_cfg=`echo "$option" | sed 's/--output=//'`
@@ -72,6 +77,11 @@ for option in "$@"; do
;;
  esac
  done
+if $next_grub_cfg; then
+echo "Missing argument to \`-o'" 1>&2
+usage
+exit 1
+fi

  . ${libdir}/grub/grub-mkconfig_lib





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Patch] [bug #26237] multiple problems with usb devices

[Grégoire Sutre]

On 05/22/2010 01:46 AM, Aleš Nesrsta wrote:


I cannot get actual bzr code because of this error:
$ bzr get http://bzr.savannah.gnu.org/r/grub
bzr: ERROR: Not a branch:
"http://bzr.savannah.gnu.org/r/grub/.bzr/branch/";.


For trunk, use:

bzr branch http://bzr.savannah.gnu.org/r/grub/trunk/grub

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[Patch] Unifont search in configure.ac

[Grégoire Sutre]

Hi,

With the attached patch, unifont is searched by extension first (pcf
being the preferred one), and then by directory.

The first directory being searched is `.', which allows non-root users
to install ascii.pf2 and unifont.pf2 by first downloading unifont in
the build directory.

Grégoire
=== modified file 'ChangeLog'
--- ChangeLog	2010-05-21 18:22:29 +
+++ ChangeLog	2010-05-22 13:03:00 +
@@ -1,3 +1,7 @@
+2010-05-22  Grégoire Sutre  
+
+	* configure.ac: Add `.' to the directories searched for unifont.
+
 2010-05-21  Vladimir Serbinenko  
 
 	* kern/i386/pc/mmap.c (grub_machine_mmap_iterate): Zero-fill entry

=== modified file 'configure.ac'
--- configure.ac	2010-05-18 11:33:35 +
+++ configure.ac	2010-05-22 13:06:31 +
@@ -179,11 +179,13 @@
   AC_MSG_ERROR([bison is not found])
 fi
 
-for file in /usr/src/unifont.bdf /usr/share/fonts/X11/misc/unifont.pcf.gz /usr/share/fonts/unifont/unifont.pcf.gz; do
-  if test -e $file ; then
-AC_SUBST([FONT_SOURCE], [$file])
-break
-  fi
+for ext in pcf pcf.gz bdf bdf.gz ttf ttf.gz; do
+  for dir in . /usr/src /usr/share/fonts/X11/misc /usr/share/fonts/unifont; do
+if test -f "$dir/unifont.$ext"; then
+  AC_SUBST([FONT_SOURCE], [$dir/unifont.$ext])
+  break 2
+fi
+  done
 done
 
 AC_PROG_INSTALL

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Are BSD partitions not supported?

[Grégoire Sutre]

On 05/19/2010 09:20 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

[snip]
-  delta = grub_partition_get_start (disk->partition);
+  delta = grub_le_to_cpu32 (whole_disk_be.offset);

As dicsussed on irc, this makes the delta completely dependent on the
c: entry of the disklabel, which could be bogus.

For instance, on NetBSD/i386, the system seems to work fine with random
entries for c: in the disk label stored on the disk. Even a null offset
is fine.  The in-core disklabel shown by the command disklabel (without
-r) is the correct one.  I give an example below, which was obtained on
system booted from wd0.

IMHO, the current code is better for the cases where the offsets in the
disklabel are absolute addresses, since it performs exactly the inverse
translation of the one done in grub_partition_get_start(), which AFAICS
is supposed to return the absolute address.

What we want here is to diverge from that code when the disklabel
offsets are relative.  I believe that testing whether c: has a null
offset gives the answer.  I changed Vladimir's second patch to do that.

We still have the problem that NetSBD uses c: for the whole-disk
partition on many ports (but it's d: on i386 and amd64), see [1].  For
those ports, the normal offset for c: is 0.  But maybe it's fair to
assume that, on those ports, the NetBSD slice is never embedded in
another partition?

Grégoire

[1] http://nxr.netbsd.org/source/s?defs=RAW_PART&project=/src

On-disk label
niagara# disklabel -r wd0
16 partitions:
#sizeoffset fstype
 a:263088  10233405 4.2BSD
 b:   2097648  10496493   swap
 c:  82345673 0 unused
 d: 117210240 0 unused
.
.
.

-
In-core label

niagara# disklabel wd0
# /dev/rwd0d:
.
.
.
16 partitions:
#sizeoffset fstype
 a:263088  10233405 4.2BSD
 b:   2097648  10496493   swap
 c:  58605120  10233405 unused
 d: 117210240 0 unused
.
.
.
=== modified file 'include/grub/bsdlabel.h'
--- include/grub/bsdlabel.h	2010-02-06 17:43:37 +
+++ include/grub/bsdlabel.h	2010-05-23 09:21:04 +
@@ -63,6 +63,8 @@
 #define	GRUB_PC_PARTITION_OPENBSD_TYPE_NTFS	18
 #define	GRUB_PC_PARTITION_OPENBSD_TYPE_RAID	19
 
+#define GRUB_PC_PARTITION_BSD_LABEL_WHOLE_DISK_PARTITION 2
+
 /* The BSD partition entry.  */
 struct grub_partition_bsd_entry
 {

=== modified file 'partmap/bsdlabel.c'
--- partmap/bsdlabel.c	2010-03-26 14:44:13 +
+++ partmap/bsdlabel.c	2010-05-23 09:30:11 +
@@ -49,15 +49,37 @@
   if (label.magic != grub_cpu_to_le32 (GRUB_PC_PARTITION_BSD_LABEL_MAGIC))
 return grub_error (GRUB_ERR_BAD_PART_TABLE, "no signature");
 
+  /* A kludge for disklabels with relative offsets.  */
+  if (GRUB_PC_PARTITION_BSD_LABEL_WHOLE_DISK_PARTITION
+  < grub_cpu_to_le16 (label.num_partitions))
+{
+  struct grub_partition_bsd_entry whole_disk_be;
+
+  pos = sizeof (label) + GRUB_PC_PARTITION_BSD_LABEL_SECTOR
+	* GRUB_DISK_SECTOR_SIZE + sizeof (struct grub_partition_bsd_entry)
+	* GRUB_PC_PARTITION_BSD_LABEL_WHOLE_DISK_PARTITION;
+
+  if (grub_disk_read (disk, pos / GRUB_DISK_SECTOR_SIZE,
+			  pos % GRUB_DISK_SECTOR_SIZE, sizeof (whole_disk_be),
+			  &whole_disk_be))
+	return grub_errno;
+
+  if (grub_le_to_cpu32 (whole_disk_be.offset) == 0)
+	delta = 0;
+}
+
   pos = sizeof (label) + GRUB_PC_PARTITION_BSD_LABEL_SECTOR
 * GRUB_DISK_SECTOR_SIZE;
 
   for (p.number = 0;
p.number < grub_cpu_to_le16 (label.num_partitions);
-   p.number++)
+   p.number++, pos += sizeof (struct grub_partition_bsd_entry))
 {
   struct grub_partition_bsd_entry be;
 
+  if (p.number == GRUB_PC_PARTITION_BSD_LABEL_WHOLE_DISK_PARTITION)
+	continue;
+
   p.offset = pos / GRUB_DISK_SECTOR_SIZE;
   p.index = pos % GRUB_DISK_SECTOR_SIZE;
 
@@ -68,11 +90,9 @@
   p.len = grub_le_to_cpu32 (be.size);
   p.partmap = &grub_bsdlabel_partition_map;
 
-  if (be.fs_type != GRUB_PC_PARTITION_BSD_TYPE_UNUSED)
+  if (p.len != 0)
 	if (hook (disk, &p))
 	  return grub_errno;
-
-  pos += sizeof (struct grub_partition_bsd_entry);
 }
 
   return GRUB_ERR_NONE;

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Are BSD partitions not supported?

[Grégoire Sutre]

On 05/23/2010 05:44 PM, C. P. Ghost wrote:


With bsdlabel_v3.diff, and the image I've posted here, I get dropped
into grub_rescue. This breaks Vladimir's second patch on FreeBSD.
:-(


Thanks for testing the patch.  But I honestly don't understand why it
does not work on your system.

I could reproduce the problem with trunk on your image by installing
grub (trunk) on a USB key and by running:

qemu -hda /dev/sdf -hdb /mnt/exchange/bsddisk.img
grub> insmod part_bsd
grub> ls (hd1,msdos2,bsd1)
Partition hd1,msdos2,bsd1: Unknown filesystem


However, if I build the patched version (with bsdlabel_v3.diff) and
re-install grub on the USB key, I get:

qemu -hda /dev/sdf -hdb /mnt/exchange/bsddisk.img
grub> insmod part_bsd
grub> ls (hd1,msdos2,bsd1)
Partition hd1,msdos2,bsd1: Filesystem type ufs2 - Last
modification time
 2010-05-20 16:11:12 Thursday, UUID 4bf55bb9a21a9565
grub> ls (hd1,msdos2,bsd1)/
README boot/
grub> cat (hd1,msdos2,bsd1)/README
This file is on a UFS2 filesystem.


Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Option handling in grub-mkconfig

[Grégoire Sutre]

On 05/24/2010 06:42 AM, BVK Chaitanya wrote:

2010/5/21 Grégoire Sutre:


The processing of option `-o' in grub-mkconfig only works when
it is the first option.



IMO this issue might have been fixed in experimental branch, by
fix-cmdline-parsing (or similar) branch.  I am on travel, so I cannot
confirm this now :-(


I confirm that the fix-cmdline-arg-parsing branch does not have this
problem.

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[Patch] File name transformations & grub-mkconfig --root-directory

[Grégoire Sutre]

Hi,

The attached patch fixes problems with file name transformations, which
are partly broken in trunk.  With this patch, several installations of
grub can coexist with no conflict (at least they should), sharing
however the same configuration directory (etc/grub.d).

It also adds --root-directory support to grub-mkconfig, and makes
grub-mkconfig_lib's prepare_grub_to_access_device load partmap modules.


The main changes in the patch are:

- configure defines three new variables, accounting for transformations:
  . pkgdatadir   (${datadir}/grub)
  . pkglibrootdir(${libdir}/grub)
  . defaultbootdir   (/boot/grub, or /grub if OS is NetBSD or OpenBSD)

- these variables are used in Makefile and in util/... scripts

- output of --help and of --version in scripts uses `basename $0` and
  the same format as the one in binaries.

- in Makefile, the help2man, .info, and .mo rules also account for
  file name transformations.

- grub-mkconfig exports GRUB_DIR, which is used by grub-mkconfig_lib
  and by 00_header.

- prepare_grub_to_access_device also loads necessary part_* modules,
  which is required when e.g. only part_msdos is included in core.img
  but the file to be accessed is in (hd0,msdos1,bsd5).


The other changes are cosmetic (with no intended functional change).

Note: this patch also contains my small Unifont search patch previously
sent to the list.

Grégoire

p.s. This corresponds to my branch gsutre/fixes, where the
 modifications are split into small commits.
=== added file 'ChangeLog.fixes'
--- ChangeLog.fixes	1970-01-01 00:00:00 +
+++ ChangeLog.fixes	2010-05-23 14:40:00 +
@@ -0,0 +1,69 @@
+2010-05-23  Grégoire Sutre  
+
+	* util/grub-install.in: Save the basename of $0 in $self, and use the
+	latter in informational messages.  Use the same format for --version
+	as the binary programs.
+	* util/grub-mkconfig.in: Likewise.
+	* util/grub-mkrescue.in: Likewise.
+	* util/grub-reboot.in: Likewise.
+	* util/grub-set-default.in: Likewise.
+	* util/i386/efi/grub-install.in: Likewise.
+	* util/ieee1275/grub-install.in: Likewise.
+	* util/powerpc/ieee1275/grub-mkrescue.in: Likewise.
+
+2010-05-22  Grégoire Sutre  
+
+	* configure.ac: Add `.' to the directories searched for unifont.
+	* util/grub-mkconfig_lib.in (prepare_grub_to_access_device): Load
+	partmap modules.
+	* util/grub.d/00_header.in: Extra sanity checks for locale support.
+	* util/grub.d/10_netbsd.in: Added a comment.
+
+2010-05-22  Grégoire Sutre  
+
+	* Makefile.in: apply file name transformation to .mo files.
+	* util/grub-install.in: Likewise.
+	* util/i386/efi/grub-install.in: Likewise.
+
+2010-05-21  Grégoire Sutre  
+
+	* util/grub-install.in: Remove the dependency on grub-mkconfig_lib.
+	* util/grub-mkconfig.in: New option --root-directory=DIR.
+	(GRUB_DIR): Export new variable.
+	* util/grub-mkconfig_lib.in: Let grub-probe use ${GRUB_DIR}/device.map.
+	* util/grub.d/00_header.in: Use GRUB_DIR passed by grub-mkconfig.
+
+2010-05-21  Grégoire Sutre  
+
+	* Makefile.in: Apply program name transformation to generated info file.
+
+2010-05-21  Grégoire Sutre  
+
+	* configure.ac: Set and subsitute pkgdatadir, pkglibrootdir and
+	defaultbootdir.
+	* Makefile.in: Use pkgdatadir, pkglibrootdir and defaultbootdir.
+	* util/grub-install.in: Likewise.
+	* util/grub-mkconfig.in: Likewise.
+	* util/grub-mkconfig_lib.in: Likewise.
+	* util/grub-mkrescue.in: Likewise.
+	* util/grub-reboot.in: Likewise.
+	* util/grub-set-default.in: Likewise.
+	* util/grub.d/00_header.in: Likewise.
+	* util/grub.d/10_hurd.in: Likewise.
+	* util/grub.d/10_kfreebsd.in: Likewise.
+	* util/grub.d/10_linux.in: Likewise.
+	* util/grub.d/10_netbsd.in: Likewise.
+	* util/grub.d/10_windows.in: Likewise.
+	* util/grub.d/30_os-prober.in: Likewise.
+	* util/i386/efi/grub-install.in: Likewise.
+	* util/ieee1275/grub-install.in: Likewise.
+	* util/powerpc/ieee1275/grub-mkrescue.in: Likewise.
+	* util/update-grub_lib.in: Likewise.
+
+2010-05-19  Grégoire Sutre  
+
+	* Makefile.in: Use installed files on help2man command line.
+	* util/grub-mkimage.c (main): Use `program_name' instead of
+	hardcoded string.
+	* util/i386/pc/grub-setup.c (main): Likewise.
+	* util/sparc64/ieee1275/grub-setup.c (parse_options): Likewise.

=== modified file 'Makefile.in'
--- Makefile.in	2010-05-18 11:55:26 +
+++ Makefile.in	2010-05-22 00:31:13 +
@@ -39,8 +39,9 @@ localedir = @localedir@
 infodir = @infodir@
 mandir = @mandir@
 includedir = @includedir@
-pkgdatadir = $(datadir)/`echo @PACKAGE_TARNAME@ | sed '$(transform)'`
-pkglibdir =  $(libdir)/`echo @PACKAGE_TARNAME@/$(target_cpu)-$(platform) | sed '$(transform)'`
+pkgdatadir = @pkgdatadir@
+pkglibrootdir = @pkglibrootdir@
+pkglibdir = $(pkglibrootdir)/$(target_cpu)-$(platform)
 
 # Internationalization library.
 LIBINTL = @LIBINTL@
@@ -90,7 +91,7 @@ GNULIB_CFLAGS = $(GNULIB_UTIL_CFLAGS) $(
 ASFLAGS = @ASFLAGS@
 LDFLAGS = @LDFLAGS@ $(LIBS)
 CPPFLAGS = @CPPFLAGS@ -I$(builddir) -I$(builddir)/include -I$(srcdir)/g

grub-setup failure with (primary) BSD disklabel

[Grégoire Sutre]

Hi,

When there is both a MBR partition table and a (primary) BSD disklabel,
grub-setup returns an error: No DOS-style partitions found.  Yet, the
partition containing the GRUB images is in the MBR.  But we are
actually lucky, since it could be worse.  Below is a detailed example
on NetBSD/i386 starting with an empty disk (actually, a virtual disk
vnd(4)).

First disklabel the disk for NetBSD use, say we get:

#sizeoffset fstype
 a:  4000   100 4.2BSD
 d: 16384 0 unused

The disklabel is stored in the second sector of the disk (at offset 1).

Now, assume that we also want to use the disk with another OS.  We
create an MBR partition for it, and add the partition to the NetBSD
disklabel.  We initialize its filesystem.  We get:

Partition table:
0: Linux native (sysid 131)
start 8064, size 8064 (4 MB, Cyls 3/60/1-7/55/32)
PBR is not bootable: All bytes are identical (0x00)
1: 

5 partitions:
#sizeoffset fstype
 a:  4000   100 4.2BSD
 d: 16384 0 unused
 e:  8064  8064 Linux Ext2


Let us now try to install grub on the disk:

$ mount /dev/vnd0e /mnt/disk
$ grub2-install --debug --root-directory=/mnt/disk /dev/rvnd0d

grub2-setup: info: setting the root device to `/dev/rvnd0d,5'.
grub2-setup: info: dos partition is 4, bsd partition is -1.
grub2-setup: error: No DOS-style partitions found.


AFAICS, grub-setup sees e: as (vnd0d,bsd5) and therefore complains
that the (primary) partmap is not msdos.

This is actually not so bad.  Indeed, e: could have been detected as
(vnd0d,msdos1), in which case grub-setup would have destroyed the
NetBSD disklabel (stored at offset 1).

AFAICS, the only reason why e: is detected as (vnd0d,bsd5) is the order
of the partmap_modules in the file grub_setup_init.lst generated by the
build.  This order depends on the order of the .c files obtained by the
make command $(filter %.c,$^).

For instance, if all occurences of $(filter %.c,...) are replaced by
$(sort $(filter %.c,...)) in conf/common.rmk, then e: is detected as
(vnd0d,msdos1), grub-setup succeeds and the NetBSD disklabel is
effectively destroyed (but grub successfully boots -- tested in qemu).


IMHO, implicitly relying on the order of $(filter %.c,...) is not
robust.  I thought of several options to solve this issue:

(a) declare that the order in which the partmap modules are loaded is
critical, and make sure that we get the right order.

(b) check that each sector to be written by grub-setup does not contain
a disklabel, and abort if any is found (unless e.g. --force was
specified).  We have at most 62 sectors to check, and there is room
for optimization (by taking into account the size of core.img).

(c) as (b), but do not abort if there is a sufficiently large contiguous
embedding area.

I would prefer (c) as it would allow the installation of grub when a BSD
disklabel is at offset 1 (we would only lose one sector as embedding
space).  That is, if grub-setup searches a DOS-style partmap in all
primary partmaps instead of stoping at the first one.

Thanks for reading all of this :-)

Comments welcome,

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[Patch] Discard incorrect nested partitions (fixes #29956)

[Grégoire Sutre]

Hi,

Regarding the nested partition code, there is an implicit assumption
that each partition should be contained in its parent, i.e. its sectors
 should also be sectors of its parent.

This ``physical nesting'' is checked in grub_disk_read, but it would
be better to check it before that.

The attached patch discards partitions that are invalid w.r.t. physical
nesting.  This solves, in particular, a problem related to NetBSD (and
OpenBSD) disklabels.

With this patch, ``external'' partitions in a disklabel simply do not
show up as BSD partitions.  For instance (see bug #29956 for an image):

MBR Partition table:
0: NetBSDstart 32, size 1000
1: DOS   start 1040, size 1000

NetBSD Disklabel (stored in MBR partition 0)
5 partitions:
#sizeoffset fstype [fsize bsize cpg/sgs]
 a:  100032 4.2BSD
 c:  100032 unused
 d:  2048 0 unused
 e:  1000  1040  MSDOS


The e: partition is external: it is not contained in MBR NetBSD
partition.

Without the patch, we get:

$ grub-probe -m /dev/null -t drive -d /dev/rvnd0e
(/dev/rvnd0d,1,5)   # this is (/dev/rvnd0d,msdos1,bsd5)
$ grub-probe -m /dev/null -t fs -d /dev/rvnd0e
grub-probe: error: unknown filesystem.

With the patch, we get:

niagara# grub-probe -m /dev/null -t drive -d /dev/rvnd0e
(/dev/rvnd0d,2)   # this is (/dev/rvnd0d,msdos2)
niagara# grub-probe -m /dev/null -t fs -d /dev/rvnd0e
fat


The patch still accepts sub-partitions that start at the same
(absolute) offset as the parent.  For instance, in the above example,
ls -l in grub gives both (hd1,msdos1) and (hd1,msdos1,bsd1).  Should
we discard (hd0,msdos1,bsd1), i.e. require that sub-partitions start
at a strictly positive relative offset?

Grégoire

--- grub_trunk/kern/partition.c	2010-05-28 01:50:37.0 +0200
+++ grub_trunk_new/kern/partition.c	2010-05-28 01:53:13.0 +0200
@@ -16,32 +16,50 @@
  *  along with GRUB.  If not, see .
  */
 
 #include 
 #include 
 #include 
 #include 
 
 grub_partition_map_t grub_partition_map_list;
 
+/*
+ * Checks that a partition is contained in its parent.
+ */
+static int
+grub_partition_validate (const grub_disk_t disk,
+			 const grub_partition_t partition)
+{
+  grub_disk_addr_t parent_len;
+
+  if (disk->partition)
+parent_len = grub_partition_get_len (disk->partition);
+  else
+parent_len = disk->total_sectors;
+
+  return (partition->start + partition->len <= parent_len);
+}
+
 static grub_partition_t
 grub_partition_map_probe (const grub_partition_map_t partmap,
 			  grub_disk_t disk, int partnum)
 {
   grub_partition_t p = 0;
 
   auto int find_func (grub_disk_t d, const grub_partition_t partition);
 
-  int find_func (grub_disk_t d __attribute__ ((unused)),
+  int find_func (grub_disk_t dsk,
 		 const grub_partition_t partition)
 {
-  if (partnum == partition->number)
+  if (partnum == partition->number &&
+	  grub_partition_validate (dsk, partition))
 	{
 	  p = (grub_partition_t) grub_malloc (sizeof (*p));
 	  if (! p)
 	return 1;
 
 	  grub_memcpy (p, partition, sizeof (*p));
 	  return 1;
 	}
 
   return 0;
@@ -131,20 +149,24 @@ grub_partition_iterate (struct grub_disk
  const grub_partition_t partition))
 {
   int ret = 0;
 
   auto int part_iterate (grub_disk_t dsk, const grub_partition_t p);
 
   int part_iterate (grub_disk_t dsk,
 		const grub_partition_t partition)
 {
   struct grub_partition p = *partition;
+
+  if (!(grub_partition_validate (dsk, partition)))
+	return 0;
+
   p.parent = dsk->partition;
   dsk->partition = 0;
   if (hook (dsk, &p))
 	{
 	  ret = 1;
 	  return 1;
 	}
   if (p.start != 0)
 	{
 	  const struct grub_partition_map *partmap;
--- grub_trunk/partmap/bsdlabel.c	2010-05-28 01:50:39.0 +0200
+++ grub_trunk_new/partmap/bsdlabel.c	2010-05-28 02:10:26.0 +0200
@@ -47,39 +47,46 @@ bsdlabel_partition_map_iterate (grub_dis
 
   /* Check if it is valid.  */
   if (label.magic != grub_cpu_to_le32 (GRUB_PC_PARTITION_BSD_LABEL_MAGIC))
 return grub_error (GRUB_ERR_BAD_PART_TABLE, "no signature");
 
   pos = sizeof (label) + GRUB_PC_PARTITION_BSD_LABEL_SECTOR
 * GRUB_DISK_SECTOR_SIZE;
 
   for (p.number = 0;
p.number < grub_cpu_to_le16 (label.num_partitions);
-   p.number++)
+   p.number++, pos += sizeof (struct grub_partition_bsd_entry))
 {
   struct grub_partition_bsd_entry be;
 
   p.offset = pos / GRUB_DISK_SECTOR_SIZE;
   p.index = pos % GRUB_DISK_SECTOR_SIZE;
 
   if (grub_disk_read (disk, p.offset, p.index, sizeof (be),  &be))
 	return grub_errno;
 
-  p.start = grub_le_to_cpu32 (be.offset) - delta;
+  p.start = grub_le_to_cpu32 (be.offset);
+  if (p.start < delta)
+	continue;
+  p.start -= delta;
   p.len = grub_le_to_cpu32 (be.size);
   p.partmap = &grub_bsdlabel_partition_map;
 
+  grub_dprintf ("partition",
+		"partition %d: type 0x%x, start 0x%llx, len 0x%

Re: Are BSD partitions not supported?

[Grégoire Sutre]

On 05/31/2010 11:16 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


Looks like we have to have different behaviour for different BSDs.
Since it looks like generic code would fail for ThisBSD or ThatBSD,
I propose to go by as-needed basis. So I think we should: -restrict
bsdlabel to being embed into msdos partitions of types a5, a6 or a9.
 -conditionalise the behaviour and use my patch for a5 (FreeBSD) and
 current behaviour for a6 (OpenBSD) and a9 (NetBSD).


We could be a bit more general and assume that BSD label offsets are
always relative except for NetBSD and OpenBSD msdos-embedded partitions.
In particular, we should support BSD labels that are not embedded in an
msdos partition.

So bsdlabel_partition_map_iterate would do (in pseudo code):

if   disk->partition is not NULL/* embedded partition */
 and
 disk->partition's partmap is msdos
 and
 disk->partition's msdos partition type is NetBSD or OpenBSD
then
 interpret BSD label offsets as absolute/* (a) */
else
 interpret BSD label offsets as relative/* (b) */
fi

In short, the normal interpretation of BSD label offsets would be the
relative one (b), and we would make an exception to handle NetBSD and
OpenBSD (a).

Now, when BSD label offsets are detected absolute (a), should we
consider them as absolute:
- w.r.t. to the start of the disk (as is done in the code right now),
  or
- w.r.t to the location of the msdos partmap?
I prefer the second option since it is compatible with dd-ing an entire
disk into an msdos partition.  Maybe the loopback feature also requires
the second option (I don't know the internals).

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[Patch] Use absolute offset for embedding area in grub-setup (i386-pc)

[Grégoire Sutre]

Hi,

The attached patch uses grub_partition_get_start (p) instead of p->start
in grub-setup's iteration over partitions.  While we are at it, also
use grub_partition_get_len (p) instead of p->len.

An alternative would be to assume that partitions are properly nested,
and therefore iterate only over the top-level ones (i.e. those with
p->parent == NULL).  In that case, we could equivalently use p->start.


In the check for non-empty embedding area, the patch uses <= instead of
== (as we may have embed_region.start == 1 && embed_region.end == 0).

Comments?

Grégoire
=== modified file 'ChangeLog'
--- ChangeLog	2010-06-03 08:48:23 +
+++ ChangeLog	2010-06-05 00:14:38 +
@@ -1,3 +1,8 @@
+2010-06-05  Grégoire Sutre  
+
+	* util/i386/pc/grub-setup.c (setup): Use absolute offsets for start of
+	embedding area.  Use <= instead of == when checking for non-emptyness.
+
 2010-06-03  Colin Watson  
 
 	* INSTALL: Document several build requirements for optional features

=== modified file 'util/i386/pc/grub-setup.c'
--- util/i386/pc/grub-setup.c	2010-05-17 19:26:16 +
+++ util/i386/pc/grub-setup.c	2010-06-04 23:53:08 +
@@ -126,8 +126,8 @@ setup (const char *dir,
   /* There's always an embed region, and it starts right after the MBR.  */
   embed_region.start = 1;
 
-  if (embed_region.end > p->start)
-	embed_region.end = p->start;
+  if (embed_region.end > grub_partition_get_start (p))
+	embed_region.end = grub_partition_get_start (p);
 
   return 0;
 }
@@ -147,8 +147,8 @@ setup (const char *dir,
   /* If there's an embed region, it is in a dedicated partition.  */
   if (! memcmp (&gptdata.type, &grub_gpt_partition_type_bios_boot, 16))
 	{
-	  embed_region.start = p->start;
-	  embed_region.end = p->start + p->len;
+	  embed_region.start = grub_partition_get_start (p);
+	  embed_region.end = grub_partition_get_start (p) + grub_partition_get_len (p);
 
 	  return 1;
 	}
@@ -361,7 +361,7 @@ setup (const char *dir,
   else
 grub_util_error (_("No DOS-style partitions found"));
 
-  if (embed_region.end == embed_region.start)
+  if (embed_region.end <= embed_region.start)
 {
   if (! strcmp (dest_partmap, "msdos"))
 	grub_util_warn (_("This msdos-style partition label has no post-MBR gap; embedding won't be possible!"));

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Are BSD partitions not supported?

[Grégoire Sutre]

On 06/01/2010 01:21 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


For FreeBSD we have to investigate 'c' partition to determine delta.


Right.


In short, the normal interpretation of BSD label offsets would be the
relative one (b), and we would make an exception to handle NetBSD and
OpenBSD (a).

The bottom line is: bsdlabel is broken concept. Unless support of
$config is required I would omit it to disencourage further propagation
of broken concepts.


I am not sure I understand what you mean here.

Regarding bsdlabel: it is not broken by itself.  The problem comes from
the fact that several OSes use it as (native) disklabel, but do not
interpret the fields in the same way.

I guess it's easier to maintain coherence when the disklabel is used
(natively) by a single OS.


Now, when BSD label offsets are detected absolute (a), should we
consider them as absolute:
- w.r.t. to the start of the disk (as is done in the code right now),
   or
- w.r.t to the location of the msdos partmap?
I prefer the second option since it is compatible with dd-ing an entire
disk into an msdos partition.  Maybe the loopback feature also requires
the second option (I don't know the internals).


Second one is cleaner. But perhaps it's pointless to support such config
since no BSD will be able to bootstrap from such a config


You could at least boot the kernel from such a config. And, for NetBSD,
I believe that the use of wedges should allow the kernel to mount the
root file system (but I'm not an expert on wedges), provided that grub
gives the wedge information in the bootinfo structure.

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Which partitioning schemes should be supported by GRUB?

[Grégoire Sutre]

Hi,

Tests of GRUB on NetBSD (and FreeBSD) have raised several issues (most
of them reported on the list) regarding partition detection.  However,
I have the feeling that some of these issues are not considered as real
issues since the test configuration is not supported by GRUB.  This
surprises me since I naively thought that most user configurations
should be supported.

So I ask the question: Which partitioning schemes are (or shall be)
supported by GRUB on i386-pc (with standard BIOS)?

To start the discussion, I'll focus on a few examples (the list is
surely not exhaustive).  Maybe some configurations simply cannot exist,
in which case please let me know.

1. A single top-level partition map
   (a) MS-DOS
   (b) GPT
   (c) BSD disklabel
   (d) Apple partition map
   (e) Sun label

2. Hybrid: top-level MS-DOS + another *top-level* partition map
   (a) MS-DOS + GPT
   (i.e. GPT + at-least one non 0xEE MS-DOS partition)
   (b) MS-DOS + BSD
   (c) ...

If I read the code correctly, grub-setup (on i386-pc) only supports
1(a) and 1(b).  However, on NetBSD, 1(c) is very common, and 2(b) is
not rare.  Also, some OSes are fine with 2(a), e.g. FreeBSD.

Personally, I would rather support all possible configurations, unless
some technical reason prevents it.  So grub-setup would not test for
some specific configurations, but would instead use a generic
(and simple) approach.  If it fails, it should be for a good reason,
and not because "No DOS-style partitions [were] found".

What's your opinion?

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] File name transformations (split into 7 diffs)

[Grégoire Sutre]

On 06/07/2010 11:16 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Regarding file-name-transformations_1.diff:
phco...@debian.bg45.phnet:~$ /usr/local/bin/grub-mkfont --help
Usage: /usr/local/bin/grub-mkfont [OPTIONS] FONT_FILES

So I would prefer not to use basename for uniformity. Otherwise patch 1
is ok.


For the record:

As discussed on irc, we will keep the basename since otherwise we would
get, for scripts:

$ grub-reboot -h
Usage: /usr/local/sbin/grub-reboot [OPTION] entry

whereas:

$ grub-probe -h
Usage: grub-probe [OPTION]... [PATH|DEVICE]

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Improve man page headers

[Grégoire Sutre]

On 06/08/2010 19:46, Colin Watson wrote:

In a recent message sent to the list [1], I proposed the following
modification [2] to account for filename transformations in man pages:

+   * Makefile.in (install-local): Use installed files on help2man command
+   line to account for file name transformations.


-	  $(HELP2MAN) --section=1 -o $(DESTDIR)$(mandir)/man1/$$dest.1 
$(builddir)/$$file; \
+	  $(HELP2MAN) --section=1 -o $(DESTDIR)$(mandir)/man1/$$dest.1 
$(DESTDIR)$(bindir)/$$dest; \



Could your approach also support filename transformations?

Grégoire

[1] http://lists.gnu.org/archive/html/grub-devel/2010-06/msg00017.html

[2] http://lists.gnu.org/archive/html/grub-devel/2010-06/txt4Y87EGyb6H.txt

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Improve man page headers

[Grégoire Sutre]

On 06/08/2010 20:26, Colin Watson wrote:


Could your approach also support filename transformations?


I'm not sure how that would work.  I guess you'd need
program_transform_name applied to the .h2m files.  Do you want me to
put my patch in a bzr branch so that you can experiment with that?
I'm not sure I have time right now.


I'm afraid I won't be able to look at it for several days, but I'll keep
in mind.  Moreover, I now realize that we should discuss the way we want
to deal with program name transformations in general.

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Which partitioning schemes should be supported by GRUB?

[Grégoire Sutre]

On 06/07/2010 10:46 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


There are two parts of this question:
1) Which partition schemes should GRUB be able to read modules and
payloads from ? It's platform-indepedent


Agreed.


and 2 conditions apply:
- Usage. There are OS which are able to boot from such OS and such
configuration isn't considered obscure by them.
- Non-confusability. The risk of false positive of this partition config
which would prevent normal function is small.
If at least one condition is met it's worth considering. If both
conditions are met it should be supported.


Ok.  Regarding confusability, I can see potential problems in the
interpretation of offsets (absolute or relative?), such as for nested
BSD labels (discussed in another thread).  Do you see other potential
causes for confusion?



2) Support for embedding.
Embedding is a potentially dangerous operation so we have to be
cautious. Using a dedicated embedding partition if it can be
unambiguously identified as such is a sane solution.


Sure.  As discussed on irc, this would require in-depth changes to
grub-setup, and it's worth another thread...

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[grub-setup] New procedure to choose the embedding area

[Grégoire Sutre]

Hi,

[ This is an extended summary of discussions that took place on irc. ]

The current version of grub-setup requires an msdos or gpt partitioning
scheme, and is not compatible with hybrid partitioning schemes (i.e. two
top-level disklabels).

Some disklabels have a specific partition type for bootstrap code, e.g.
gpt and bsd.  Apple partitions have a type that is actually a string (32
chars), so we could use a specific type for grub bootstrap code (e.g.
`Grub_Bootstrap' or simply `Grub').  Maybe there are other disklabels
(among those supported by GRUB) with a specific partition type for boot
partitions.

As you know, there isn't a specific partition type for bootstrap code in
msdos disklabels.  The current implementation of grub-setup embeds after
the MBR when the detected disklabel is msdos.  But this is fragile:
many tools do not hesitate to write in this area regardless of what is
there.

Ideally, grub-setup should use a dedicated boot partition if it finds
one, and embed after the MBR only as a last resort.  Still, we must be
careful with dedicated boot partitions when there are several top-level
disklabels: the disklabel containing the dedicated boot partition could
be obsolete (e.g. a leftover from some prior installation).  In that
case, we may overwrite user data when embedding in the boot partition.
So we should check that the boot partition does not overlap another
partition.

To sum up, the new procedure to select the embedding area would be:

 1. FOREACH top-level partition p/* i.e. p->parent == NULL */
 2.IF p is a dedicated (gpt|bsd|apple|...) boot partition AND
  p does not overlap with another top-level partition
 3.THEN
 4.   return p
/* No appropriate top-level dedicated boot partition. */
/* Handle the standard msdos case as an exception. */
 5. IF the disk contains a single top-level disklabel, and this
   disklabel is an msdos one
 6. THEN
 7.return [1, n-1] where n is the minimal start sector of the
   top-level partitions
/* Do not try to be too smart...  Abort! :-) */
 8. explain to the user why no embedding area could be found
 9. return NULL

The first FOREACH loop discards nested partitions, so it would miss for
instance a dedicated boot partition (hd0,msdos2,bsd7).  It would
instead try to embed in the post-MBR gap, which may fail or be more
risky than in the nested dedicated boot partition.

What do you think regarding this issue?  Should we accept any dedicated
boot partition, even if it is nested?

This message is only a proposition, and I look forward to your comments
and suggestions.

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Patch] Discard incorrect nested partitions (fixes #29956)

[Grégoire Sutre]

On 05/31/2010 08:35 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


There are few ramifications of this patch. First of all some
partitions which are just barely outside of the host partition will
lead to something like "partition not found" errors in grub-probe.


It's not ideal, but IMHO it's better than getting "unknown filesystem"
in grub-probe -t fs while the partition is detected fine by
-t drive.


This message should be more informative (the easiest way is to issue
a warning in grub-probe if partitions are discarded except some cases
where it's known not to affect the functionality like 'd'
"subpartitions", probably such a warning in grub proper would be too
annoying though).


A grub_dprintf when the partition is discarded (in the proposed patch)
would only print the message for relevant partitions.


Then if you check partitions when iterating no need to recheck in
adjust_range.


Agreed.


The patch still accepts sub-partitions that start at the same
(absolute) offset as the parent.  For instance, in the above
example, ls -l in grub gives both (hd1,msdos1) and
(hd1,msdos1,bsd1).  Should we discard (hd0,msdos1,bsd1), i.e.
require that sub-partitions start at a strictly positive relative
offset?

No. SUN partitions comonly start at offset 0.


I don't understand what you mean: here bsd1 also starts at (relative)
offset 0, and the above example actually assumed that.

By the way, when several partition identifiers denote the same
partition, the MBI boot_device field can have different values for the
same physical partition.  This means more work (or assumptions) on the
kernel side to identify the root partition.  In the above example,
(hd1,msdos1,bsd1) would be more explicit to the NetBSD kernel than
(hd1,msdos1).

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Which partitioning schemes should be supported by GRUB?

[Grégoire Sutre]

On 06/07/2010 10:46 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


There are two parts of this question:
1) Which partition schemes should GRUB be able to read modules and
payloads from ? It's platform-indepedent and 2 conditions apply:
- Usage. There are OS which are able to boot from such OS and such
configuration isn't considered obscure by them.
- Non-confusability. The risk of false positive of this partition config
which would prevent normal function is small.
If at least one condition is met it's worth considering. If both
conditions are met it should be supported.


According to these rules, hybrid msdos+gpt partitioning schemes should
be supported.  Grub should be able to read files from a GPT partition
even if the protective GPT entry in the MBR is not in the first slot.
It's also the conclusion of the thread [1], but I admit that this thread
is two years old.

Grégoire

[1] http://lists.gnu.org/archive/html/grub-devel/2008-09/msg00101.html

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix grub-probe partition naming on FreeBSD

[Grégoire Sutre]

Hi Colin,


The following patch is aimed at fixing this Debian bug:

   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585068

I've tested it on Debian GNU/kFreeBSD and it seems to be producing sensible
output now.


In another thread [1], it was observed that offsets are not absolute in
FreeBSD disklabels, whereas they are absolute in NetBSD disklabels.
I believe that find_partition_start is supposed to return absolute
offsets.  Does DIOCGDINFO convert the on-disk label into absolute
offsets on FreeBSD?


The one glitch is that if you ask it to probe /dev/ad0s1a, it returns
(hd0,msdos1) rather than (hd0,msdos1,bsd1): this is because both /dev/ad0s1
and /dev/ad0s1a have the same start sector, and it just uses the first one
it finds.  When I set prefix to (hd0,msdos1)/boot/grub, GRUB can read from
that perfectly well, so can I ignore this glitch on the basis that it
doesn't cause a practical problem?


We get a similar behavior on NetBSD.  As I mentioned in [2], this may
have an impact when the kernel is (a) loaded with the multiboot
protocol, and (b) relies on the MBI boot_device field to find its
root -- which it shouldn't anyway, so it's not a big deal.  I am not
aware of other impacts.

I know that hybrid msdos+gpt are not recommended, but, for the record,
I guess that another glitch could happen if /dev/ad0s1X (msdos) and
/dev/ad0p1Y (gpt) are actually the same partition: grub-probe would
return the same answer for both, thus ignoring the user's desire to use
a specific partitioning scheme.  I believe that such a partitioning is
supported by FreeBSD, but I may be wrong (please tell me if so), I did
not test this myself.

Grégoire

[1] http://lists.gnu.org/archive/html/grub-devel/2010-05/msg00065.html
[2] http://lists.gnu.org/archive/html/grub-devel/2010-06/msg00075.html

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Which partitioning schemes should be supported by GRUB?

[Grégoire Sutre]

On 06/12/2010 07:26 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


Any "hybrid" cofiguration fails the criteria of non confusability.


I was assuming the new partition notation.  The old notation is clearly
ambiguous when there are multiple partmaps, and AFAIR the new notation
was introduced precisely to solve that problem:

http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00320.html

By the way, the old notation is worse than ambiguous when there are
multiple partmaps: the meaning of partition identifiers depends on the
partmap modules that are loaded, and on the order in which they are
loaded.


Let's consider a following situation: - I format disk with scheme A
and partitions A1, A2, A3 - I get bored and reformat with scheme B
and partitios B1, B2, B3, B4. When I did this filesystem on A2 may
stay intact - I use grub which supposes that it'shybrid system A+B
and save_env's to A2 since it's a valid partition on valid
filesystem. But by a bad luck save_env overlaps with superblock of B3
which becomes corrupted.


If you save_env with -f then, with new notation, you know that you are
using the old scheme A.  If you didn't use -f, then it means that grub
modules were installed into A2 and survived the reformat, but then,
how could GRUB know that A is obsolete?  IMHO corrupting the superblock
of B3 is acceptable in that case.  An alternative would be to check
that partitions do not overlap (with the exception of identical
partitions).  But even this would work only if the partmap module for
B was loaded, which is likely not the case (as grub was installed at
the time A was used).


And currently grub isn't changed to new partition notation
completely. E.g. during startup prefix is calculated with old syntax
and confusing A+B with either A or B is likely to make user drop into
rescue shell


Is someone working on making the startup prefix use the new notation?

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Which partitioning schemes should be supported by GRUB?

[Grégoire Sutre]

On 06/14/2010 18:43, Colin Watson wrote:


Do you have any suggestions on how to deal with that?  I'm not familiar
with multiboot and need guidance.


A possible solution would be to use the multiboot-command line.  AFAIK,
the boot_device field of the multiboot information structure is supposed
to pass this kind of partition information, but you cannot specify the
partmaps in this field, hence its interpretation is ambiguous.

Grégoire

p.s. This issue (with boot_device field) was discussed a bit in:

http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00351.html
http://lists.gnu.org/archive/html/grub-devel/2010-02/msg00070.html

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Which partitioning schemes should be supported by GRUB?

[Grégoire Sutre]

On 06/15/2010 01:21 PM, Colin Watson wrote:


A possible solution would be to use the multiboot-command line.  AFAIK,
the boot_device field of the multiboot information structure is supposed
to pass this kind of partition information, but you cannot specify the
partmaps in this field, hence its interpretation is ambiguous.


That would potentially allow a user to override things, but doesn't help
with users who don't change their configuration.  Unless the user
explicitly configures things, boot_device is all we've got.


Ok.


Thus, I guess we end up with a two-part fix:

   1) Honour key=value pairs in the multiboot command line when booting
  GRUB itself as a multiboot image.  These would simply become
  environment variables.


This would be nice.


   2) If multiboot_trampoline needs to change install_dos_part or
  install_bsd_part based on the value of boot_device in the MBI, then
  we know that the drive/partition part of prefix (which was
  calculated in the same way as install_dos_part and install_bsd_part
  when grub-setup was run) is now invalid and should be ignored.
  This fact needs to be passed on to make_install_device.


Since the command-line processing of the MBI is done after startup.S (in 
grub_machine_init()),


- the MBI (only the relevant portions for simplicity) needs to be copied
  to a safe place.  The patch does it at the end of grub_machine_init(),
  but I'm afraid this might be too late: the MBI may have been
  overwritten before we reach that point.  Or is the code (startup.S and
  grub_machine_init()) designed to guarantee that all memory writes are
  in safe locations, outside of the whole MBI?

- couldn't the complete processing of the MBI be performed in the same
  place, i.e. grub_machine_init()?  The assembly multiboot part would
  only check whether GRUB was booted by multiboot, and set (or copy)
  the MBI in that case.  Then the procedure to set grub_prefix would be
  coded in one place.  This would require though, for multiboot, to get
  grub_boot_drive from the boot_device field (the current assembly code
  takes care of this).


  void
  grub_machine_init (void)
  {
@@ -214,6 +279,15 @@ grub_machine_init (void)
if (! grub_os_area_addr)
  grub_fatal ("no upper memory");

+  if (startup_multiboot_info)
+{
+  /* Move MBI to a safe place.  */
+  grub_memmove (&kern_multiboot_info, startup_multiboot_info,
+   sizeof (struct multiboot_info));


Moving the MBI is more complex, since the structure contains pointers
to other structures (themselves containing pointers etc.).  But in our
case it's not too painful since we only need to copy the struct
multiboot_info and the string pointed to by its cmdline field (and set
the field to the new address).

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Which partitioning schemes should be supported by GRUB?

[Grégoire Sutre]

Hi,

I made several tests, and the patch works fine with standard boot.  When
multibooting core.img, the command-line is taken into account correctly.
However, if no multiboot command-line is given, the prefix is set as
before (old partition naming style).

This comes from the fact that the modifications to the prefix done in
grub-setup only apply to the embedded image, and not to the core.img
file.  The grub-mkimage command used by grub-install is:

grub-mkimage -O i386-pc --output=/path/to/core.img --prefix=/boot/grub

Would it make sense to put the full prefix here?



- couldn't the complete processing of the MBI be performed in the same
   place, i.e. grub_machine_init()?  The assembly multiboot part would
   only check whether GRUB was booted by multiboot, and set (or copy)
   the MBI in that case.  Then the procedure to set grub_prefix would be
   coded in one place.  This would require though, for multiboot, to get
   grub_boot_drive from the boot_device field (the current assembly code
   takes care of this).


My investigations suggest that this is very difficult.  Relocating the
GRUB kernel, which is almost the first thing multiboot_entry does, is
liable to overwrite the MBI, and you can't get at C code until you've
done that.  That's why multiboot_entry has to copy out the boot device
field even before it relocates the kernel.


What I meant is: the assembly part could be restricted to the copy of
the relevant MBI information, i.e. flags, boot_device and cmdline.  And
leave the actual decisions regarding the contents of those fields to
grub_machine_init().  This would mean that the part dealing with
grub_install_dos/bsd_part (multiboot_trampoline) would be part of the C
code (probably in make_install_device()).



  grub_machine_set_prefix (void)
  {
/* Initialize the prefix.  */
-  grub_env_set ("prefix", make_install_device ());
+  if (!found_multiboot_prefix)
+grub_env_set ("prefix", make_install_device ());


You could avoid the variable found_multiboot_prefix and use
grub_env_find("prefix").  The intention would be that if someone
(multiboot or someone else) has already set the prefix in grub_env,
then we shouldn't override this choice here.  This would simplify
a bit grub_parse_multiboot_cmdline(), which would become purely
generic.  But this is essentially cosmetic...


 === modified file 'kern/i386/pc/startup.S'

--- kern/i386/pc/startup.S  2010-04-05 13:59:32 +
+++ kern/i386/pc/startup.S  2010-06-16 12:55:05 +
@@ -145,6 +145,32 @@ multiboot_entry:
/* obtain the boot device */
movl12(%ebx), %edx


The boot_device field should be used only if the MULTIBOOT_INFO_BOOTDEV
flag is set.  This problem is already present in trunk.


+   /* if so, copy it to a safe place; do this before relocating code to
+  make sure that we don't lose it */
+   movl16(%ebx), %edi
+   pushl   %edi


Is it safe to push?  Maybe we should start by setting %esp as is done a
few lines below.

I honestly do not understand all the details regarding the assembly code
or the memory management, so I can't provide useful feedback on that.
I just noticed that GRUB_MEMORY_MACHINE_MULTIBOOT_FLAGS is copied into a
``regular'' variable, but GRUB_MEMORY_MACHINE_MULTIBOOT_CMDLINE is not.

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[Patch] Do not embed with multiple partmaps

[Grégoire Sutre]

Hi,

As a (temporary) solution to the problem of embedding with multiple
top-level partmaps, the attached patch simply disables embedding when
there are multiple partmaps (i.e. apply the same behavior as when there
is no partmap).

This prevents grub-setup from overwriting e.g. a BSD label if it finds
an msdos label first.

The only drawback I see is that grub-setup may complain about an old
obsolete disklabel, but in that case the user simply has to destroy the
obsolete label, and IMHO this is more acceptable than potentially
destroying valuable data.

Grégoire
=== modified file 'ChangeLog'
--- ChangeLog	2010-06-17 20:54:04 +
+++ ChangeLog	2010-06-17 22:06:58 +
@@ -1,3 +1,8 @@
+2010-06-17  Grégoire Sutre  
+
+	* util/i386/pc/grub-setup.c (setup): Do not embed when there are
+	multiple (top-level) partmaps.
+
 2010-06-17  Colin Watson  
 
 	* util/i386/pc/grub-setup.c (usage): Pass an extra `program_name'

=== modified file 'util/i386/pc/grub-setup.c'
--- util/i386/pc/grub-setup.c	2010-06-17 20:54:04 +
+++ util/i386/pc/grub-setup.c	2010-06-17 22:04:48 +
@@ -93,6 +93,7 @@
   grub_uint16_t core_sectors;
   grub_device_t root_dev, dest_dev;
   const char *dest_partmap;
+  int multiple_partmaps;
   grub_uint8_t *boot_drive;
   grub_disk_addr_t *kernel_sector;
   grub_uint16_t *boot_drive_check;
@@ -354,10 +355,17 @@
 {
   if (p->parent)
 	return 0;
-  dest_partmap = p->partmap->name;
-  return 1;
+  if (dest_partmap == NULL)
+	dest_partmap = p->partmap->name;
+  else if (strcmp (dest_partmap, p->partmap->name) != 0)
+	{
+	  multiple_partmaps = 1;
+	  return 1;
+	}
+  return 0;
 }
   dest_partmap = 0;
+  multiple_partmaps = 0;
   grub_partition_iterate (dest_dev->disk, identify_partmap);
 
   if (! dest_partmap)
@@ -365,6 +373,11 @@
   grub_util_warn (_("Attempting to install GRUB to a partitionless disk.  This is a BAD idea."));
   goto unable_to_embed;
 }
+  if (multiple_partmaps)
+{
+  grub_util_warn (_("Attempting to install GRUB to a disk with multiple partition labels.  This is a BAD idea."));
+  goto unable_to_embed;
+}
 
   if (strcmp (dest_partmap, "msdos") == 0)
 grub_partition_iterate (dest_dev->disk, find_usable_region_msdos);

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Patch] Do not embed with multiple partmaps

[Grégoire Sutre]

On 07/01/2010 10:52 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


Go ahead. Just replace "BAD IDEA" with "not supported yet"


Thanks, I replaced "BAD IDEA" with "not supported yet" and committed.

Grégoire.

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Patch] Discard incorrect nested partitions (fixes #29956)

[Grégoire Sutre]

Hi,

This is the reworked version of the patch.


There are few ramifications of this patch. First of all some partitions
which are just barely outside of the host partition will lead to
something like "partition not found" errors in grub-probe. This message
should be more informative (the easiest way is to issue a warning in
grub-probe if partitions are discarded except some cases where it's
known not to affect the functionality like 'd' "subpartitions", probably
such a warning in grub proper would be too annoying though).


There is now a grub_dprintf where the partition is discarded.


Then if you check partitions when iterating no need to recheck in
adjust_range.


I simplified the check in grub_disk_adjust_range: no need to check for
the ``ancestor'' partitions, but we still check for the given partition.


The patch still accepts sub-partitions that start at the same
(absolute) offset as the parent.  For instance, in the above example,
ls -l in grub gives both (hd1,msdos1) and (hd1,msdos1,bsd1).  Should
we discard (hd0,msdos1,bsd1), i.e. require that sub-partitions start
at a strictly positive relative offset?


I left this out.  Even if it introduces redundancy, it's actually nice
to see (hd1,msdos1,bsd1) in ls -l (e.g., if bsd1 is the only partition
in the bsd label).

Grégoire
=== added file 'ChangeLog.strict-partition-nesting'
--- ChangeLog.strict-partition-nesting	1970-01-01 00:00:00 +
+++ ChangeLog.strict-partition-nesting	2010-07-06 21:25:28 +
@@ -0,0 +1,14 @@
+2010-07-06  Grégoire Sutre  
+
+	* kern/partition.c (grub_partition_validate): New function to check
+	that a partition is physically contained in its parent.  Since offsets
+	are relative (and non-negative), this reduces to checking that the
+	partition ends before its parent.
+	(grub_partition_map_probe): Discard invalid partitions.
+	(grub_partition_iterate): Likewise.
+	* kern/disk.c (grub_disk_adjust_range): Simplify out-of-partition
+	check (partitions are assumed to be properly nested).
+	* include/grub/partition.h (grub_partition_map): Slightly more detailed
+	comments.
+	* partmap/bsdlabel.c (bsdlabel_partition_map_iterate): Discard
+	partitions that start before their parent, and add debug printfs.

=== modified file 'include/grub/partition.h'
--- include/grub/partition.h	2010-06-12 13:33:09 +
+++ include/grub/partition.h	2010-07-06 21:16:05 +
@@ -48,7 +48,7 @@ struct grub_partition
   /* The partition number.  */
   int number;
 
-  /* The start sector.  */
+  /* The start sector (relative to parent).  */
   grub_disk_addr_t start;
 
   /* The length in sector units.  */
@@ -60,7 +60,7 @@ struct grub_partition
   /* The index of this partition in the partition table.  */
   int index;
 
-  /* Parent partition map.  */
+  /* Parent partition (physically contains this partition).  */
   struct grub_partition *parent;
 
   /* The type partition map.  */

=== modified file 'kern/disk.c'
--- kern/disk.c	2010-02-07 00:48:38 +
+++ kern/disk.c	2010-07-06 21:22:27 +
@@ -361,12 +361,10 @@ grub_disk_adjust_range (grub_disk_t disk
   *sector += *offset >> GRUB_DISK_SECTOR_BITS;
   *offset &= GRUB_DISK_SECTOR_SIZE - 1;
 
-  for (part = disk->partition; part; part = part->parent)
+  part = disk->partition;
+  if (part)
 {
-  grub_disk_addr_t start;
   grub_uint64_t len;
-
-  start = part->start;
   len = part->len;
 
   if (*sector >= len
@@ -374,7 +372,7 @@ grub_disk_adjust_range (grub_disk_t disk
 			  >> GRUB_DISK_SECTOR_BITS))
 	return grub_error (GRUB_ERR_OUT_OF_RANGE, "out of partition");
 
-  *sector += start;
+  *sector += grub_partition_get_start (part);
 }
 
   if (disk->total_sectors <= *sector

=== modified file 'kern/partition.c'
--- kern/partition.c	2010-02-06 20:00:53 +
+++ kern/partition.c	2010-07-06 21:20:04 +
@@ -23,6 +23,23 @@
 
 grub_partition_map_t grub_partition_map_list;
 
+/*
+ * Checks that a partition is contained in its parent.
+ */
+static int
+grub_partition_validate (const grub_disk_t disk,
+			 const grub_partition_t partition)
+{
+  grub_disk_addr_t parent_len;
+
+  if (disk->partition)
+parent_len = grub_partition_get_len (disk->partition);
+  else
+parent_len = disk->total_sectors;
+
+  return (partition->start + partition->len <= parent_len);
+}
+
 static grub_partition_t
 grub_partition_map_probe (const grub_partition_map_t partmap,
 			  grub_disk_t disk, int partnum)
@@ -31,20 +48,26 @@ grub_partition_map_probe (const grub_par
 
   auto int find_func (grub_disk_t d, const grub_partition_t partition);
 
-  int find_func (grub_disk_t d __attribute__ ((unused)),
+  int find_func (grub_disk_t dsk,
 		 const grub_partition_t partition)
 {
-  if (partnum == partition->number)
-	{
-	  p = (grub_partition_t) grub_malloc (sizeof (*p));
-	  if (! p)
-	return 1;
+  if (partnum != partition->number)
+	return 0;
 
-	  grub_memcpy (p, partition, sizeof (*p));
-	  return 1;
+  if (!(grub_partition_validate (dsk, part

Re: [Patch] Discard incorrect nested partitions (fixes #29956)

[Grégoire Sutre]

On 07/08/2010 02:28, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Attached is the new version of the patch.


As I already told you in real dprintf isn't seen by user. One need to
grub_dprintf ();
#ifdef GRUB_UTIL
grub_util_warn (...);
#endif


Ok.  For partition.c, this is now done in the checking function to avoid
code duplication (and it makes the code easier to read).

For bsdlabel.c, I re-ordered the code so that no warning is shown for
partitions of type unused (such as the raw partition).


I simplified the check in grub_disk_adjust_range: no need to check for
the ``ancestor'' partitions, but we still check for the given partition.


You're right. This means we can't screw this test to save space. In this
case it's better to do the complete check for early bug catch.


I removed the simplification from the patch.


We shouldn't check for partitions being outside of disk since BIOS disk
size limitations are common. Consider following situation:
(hd0,msdos1,bsd1)   /boot
BIOS LIMIT
(hd0,msdos1,bsd2)  /
This system is perfectly capable of booting but with your patch it won't.


Right.  The patch now only checks sub-partitions (no check is done on
top-level partitions).

> We must always exercice best effort strategy. If something can bee
> booted, boot it.

Here, we discard improperly nested partitions, even though they could
be accessed.  So one may argue that this breaks the best effort
strategy.  However, such improperly nested partitions can in general be
accessed by a properly nested identifier, so I guess it's fine.


We should warn if a used final-nestedness partition is partialy outside
the limit. Simple message usually scrolls way too fast and so usually
ignored (if someone sees boot process at all). Perhaps we need a way to
pass such warnings to kernel which then can take appropriate action
(e.g. notify sysadmin)


I'm not sure where we should check that.  If one attempts to load a
kernel or module that is beyond the disk limit, grub_disk_read will
fail anyway.

Grégoire

=== added file 'ChangeLog.strict-partition-nesting'
--- ChangeLog.strict-partition-nesting  1970-01-01 00:00:00 +
+++ ChangeLog.strict-partition-nesting  2010-07-09 00:26:38 +
@@ -0,0 +1,12 @@
+2010-07-06  Grégoire Sutre  
+
+   * kern/partition.c (grub_partition_check_containment): New function to
+   check that a partition is physically contained in a parent.  Since
+   offsets are relative (and non-negative), this reduces to checking that
+   the partition ends before its parent.
+   (grub_partition_map_probe): Discard out-of-range sub-partitions.
+   (grub_partition_iterate): Likewise.
+   * include/grub/partition.h (grub_partition_map): Slightly more detailed
+   comments.
+   * partmap/bsdlabel.c (bsdlabel_partition_map_iterate): Discard
+   partitions that start before their parent, and add debug printfs.

=== modified file 'include/grub/partition.h'
--- include/grub/partition.h2010-06-12 13:33:09 +
+++ include/grub/partition.h2010-07-09 00:26:38 +
@@ -48,7 +48,7 @@ struct grub_partition
   /* The partition number.  */
   int number;
 
-  /* The start sector.  */
+  /* The start sector (relative to parent).  */
   grub_disk_addr_t start;
 
   /* The length in sector units.  */
@@ -60,7 +60,7 @@ struct grub_partition
   /* The index of this partition in the partition table.  */
   int index;
 
-  /* Parent partition map.  */
+  /* Parent partition (physically contains this partition).  */
   struct grub_partition *parent;
 
   /* The type partition map.  */

=== modified file 'kern/partition.c'
--- kern/partition.c2010-02-06 20:00:53 +
+++ kern/partition.c2010-07-09 00:26:38 +
@@ -23,6 +23,37 @@
 
 grub_partition_map_t grub_partition_map_list;
 
+/*
+ * Checks that disk->partition contains part.  This function assumes that the
+ * start of part is relative to the start of disk->partition.  Returns 1 if
+ * disk->partition is null.
+ */
+static int
+grub_partition_check_containment (const grub_disk_t disk,
+ const grub_partition_t part)
+{
+  if (disk->partition == NULL)
+return 1;
+
+  if (part->start + part->len > disk->partition->len)
+{
+  char *partname;
+
+  partname = grub_partition_get_name (disk->partition);
+  grub_dprintf ("partition", "sub-partition %s%d of (%s,%s) ends after 
parent.\n",
+   part->partmap->name, part->number + 1, disk->name, 
partname);
+#ifdef GRUB_UTIL
+  grub_util_warn ("Discarding improperly nested partition (%s,%s,%s%d)",
+ disk->name, partname, part->partmap->name, part->number + 
1);
+#endif
+  grub_free (partname);
+
+  return 0;
+}
+
+  return 1;
+}
+
 static grub_partition_t
 grub_partition_map_probe (const grub_partition_map_t partmap,
  grub_disk_t disk, int partnum)
@@ -31,20 +62,21 @@ grub_partition_map_probe (const grub_par
 
   auto int find_func (

[Patch] strip disk->name of partition suffix in grub_disk_open

[Grégoire Sutre]

Hi,

When the argument passed to grub_disk_open is a string containing a
partition, e.g. "hd0,msdos3,bsd7", disk->name is set to the full
string instead of just the prefix up to the first ',' (here "hd0").
This leads to incorrect partition identifiers when the disk name and
the partition name are concatenated (I observed this in the debug
messages of my nested partitions patch).

The attached patch fixes this issue.

Grégoire
=== modified file 'kern/disk.c'
--- kern/disk.c 2010-02-07 00:48:38 +
+++ kern/disk.c 2010-07-09 16:44:06 +
@@ -248,11 +248,16 @@ grub_disk_open (const char *name)
   if (! disk)
 return 0;
 
-  disk->name = grub_strdup (name);
+  p = find_part_sep (name);
+
+  if (p)
+disk->name = grub_strndup (name, p - name);
+  else
+disk->name = grub_strdup (name);
+
   if (! disk->name)
 goto fail;
 
-  p = find_part_sep (name);
   if (p)
 {
   grub_size_t len = p - name;

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Patch] Discard incorrect nested partitions (fixes #29956)

[Grégoire Sutre]

On 07/13/2010 11:53 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


I mean that the files we need are below the limit but some parts of
partition are above the limit. It will work the current boot but will
break in the future.


I see.



Go ahead for trunk.


Thanks, I committed the patch.

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Patch] strip disk->name of partition suffix in grub_disk_open

[Grégoire Sutre]

On 07/20/2010 03:52 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


Which functions concatenate disk->name with anything else? It looks like
disk->name is with partitions by design.


In several places, the (full) grub partition name is obtained by
concatenating disk->name with the result of grub_partition_get_name.

If I read the code correctly, this is what is done in:

function iterate_partition, file normal/completion.c.
function grub_device_iterate, file kern/device.c
function grub_efidisk_get_device_name, file disk/efi/efidisk.c

and I did the same in

function grub_util_biosdisk_get_grub_dev, file kern/emu/hostdisk.c

and more recently, for warnings/dprintfs, in

function grub_partition_check_containment, file kern/partition.c
function bsdlabel_partition_map_iterate, file partmap/bsdlabel.c


Assuming that the above are correct and that disk->name is correctly
set in grub_disk_open (e.g. hd0,msdos3,bsd7), then shouldn't
disk->partition have a NULL parent (which is not the case right now)?

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Improperly nested partitions, help needed!

[Grégoire Sutre]

On 09/21/2010 09:31, Svante Signell wrote:


When installing a new kernel or a new version of grub I get a
warning that /dev/sda1 (windows rescue) and /dev/sda3 (linux root /)
are improperly nested: /usr/sbin/grub-probe: warn: Discarding
improperly nested partition (hd1,msdos3,msdos1).


Assuming that hd1 is sda, this means that (1) grub-probe detects an
MSDOS partition table T in the first sector of sda3, and (2) the first
partition descriptor in T describes a partition that ends after sda3.

It does not say anything about sda1, which is (hd1,msdos1).

Correct detection of MSDOS partition tables is not obvious.  If I read
the code correctly, grub-probe detects an MSDOS partition table (at the
beggining of a disk/partition) if (a) the sector ends with the correct
signature and (b) the first byte of each partition descriptor contains
a valid boot flag.

Actually, for (b), the check that it's a valid boot flag is:

  for (i = 0; i < 4; i++)
if (mbr.entries[i].flag & 0x7f)
  return grub_error (GRUB_ERR_BAD_PART_TABLE, "bad boot flag");

Couldn't this be made stronger, with:

  for (i = 0; i < 4; i++)
if (mbr.entries[i].flag != 0x00 && mbr.entries[i].flag != 0x80)
  return grub_error (GRUB_ERR_BAD_PART_TABLE, "bad boot flag");

?


How to resolve this problem?


It's likely that the first sector of sda3 contains useless, leftover
data.  If that's the case (I mean if you are absolutely sure that it's
the case) then you could simply fill this sector with zeroes.


Additionally, does the same warning have to be repeated so many times
for every kernel entry???


Well, I guess so, since grub-probe is run independently for each kernel
entry (several times actually).

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Improperly nested partitions, help needed!

[Grégoire Sutre]

On 09/21/2010 16:54, Lennart Sorensen wrote:


What does improperly nested mean: overlapping, or something else?

How to resolve this problem? According to fdisk the sda1 and sda3 partitions 
are _not_ overlapping:
Additionally, does the same warning have to be repeated so many times for every 
kernel entry???


It did not say they overlapped.  It said it found a partition table
inside sda3 which is improper given it is not an extended partition
(sda4 is).


I'm pretty sure that the warning message ``improperly nested'' really
is about partition nesting as discussed e.g. in [1].  It's not about
extended partitions.


But I agree that the problem at hand may come from this:


If so, then there is a chance that the first sector of sda3 contains
the old extended partition table, and grub probe might be detecting that.



Grégoire

[1] http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00320.html

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Improperly nested partitions, help needed!

[Grégoire Sutre]

On 09/21/2010 09:42 PM, Svante Signell wrote:


# file /tmp/sda3.mbr /tmp/sda3.mbr: x86 boot sector; GRand Unified
Bootloader, stage1 version 0x3, 1st sector stage2 0xdd29b38;
partition 1: ID=0x83, starthead 239, startsector 63, 35153937
sectors, extended partition table (last)\011, code offset 0x48


I'm surprised that file is able to tell that this is an extended
partition table.  If we could reliably distinguish MBR partition tables
from EBR ones, then we could prevent GRUB from considering such EBR
partition tables.  But I don't see how a such a distinction could be
made: as far as I know, any EBR partition table could appear in an MBR.

So out of curiosity, I looked a bit deeper at the behavior of the file
command, and I'm afraid that the result of file is not 100% reliable in
this respect: for a partition table with only one used entry, file
detects it as an EBR partition table if the entry is not active, as
shown by this example (one sector in /tmp/sect.bin):

   Device  BootStart   End   #sectors  Id  System
/tmp/sect.bin1   *63  41945714   41945652  83  Linux
/tmp/sect.bin2 0 -  0   0  Empty
/tmp/sect.bin3 0 -  0   0  Empty
/tmp/sect.bin4 0 -  0   0  Empty

$ od -Ax -t x1 /tmp/sect.bin
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
*
0001b0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 80 01
0001c0 01 00 83 fe ff ff 3f 00 00 00 34 0a 80 02 00 00
0001d0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
*
0001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa
000200

$ file /tmp/sect.bin
/tmp/sect.bin: x86 boot sector; partition 1: ID=0x83, active, starthead
1, startsector 63, 41945652 sectors, code offset 0x0


Everything looks good.  But after making the partition inactive,
we get:

$ od -Ax -t x1 /tmp/sect.bin
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
*
0001b0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01
0001c0 01 00 83 fe ff ff 3f 00 00 00 34 0a 80 02 00 00
0001d0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
*
0001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa
000200

$ file /tmp/sect.bin
/tmp/sect.bin: x86 boot sector; partition 1: ID=0x83, starthead 1,
startsector 63, 41945652 sectors, extended partition table (last)\011,
code offset 0x0

Still, this partition table may well be contained in an MBR.

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Improperly nested partitions, help needed!

[Grégoire Sutre]

On 09/22/2010 18:44, Lennart Sorensen wrote:


After all msdos partitions tables may only exist in MBR and extended
partitions


According to which standard?

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Fix getline name clash

[Grégoire Sutre]

Hi,

On NetBSD 5, the latest trunk does not build:

$ ./autogen.sh ; ./configure CFLAGS='-std=gnu99' ; gmake
[...]
gcc -DHAVE_CONFIG_H -I.  -Wall -W -I./include -DGRUB_UTIL=1 
-DGRUB_LIBDIR=\"/usr/local/lib/grub\" 
-DLOCALEDIR=\"/usr/local/share/locale\"  -DGRUB_MACHINE_PCBIOS=1 
-DGRUB_MACHINE=I386_PC -DGRUB_FILE=\"grub-core/script/lexer.c\" -I. -I. 
-I. -I. -I./include -I./include -I./grub-core/gnulib 
-I./grub-core/gnulib -I./grub-core/lib/libgcrypt_wrap-Wno-undef 
-Wno-sign-compare -Wno-unused -Wno-error -Wno-missing-field-initializers 
 -std=gnu99 -MT grub-core/script/libgrub_a-lexer.o -MD -MP -MF 
grub-core/script/.deps-util/libgrub_a-lexer.Tpo -c -o 
grub-core/script/libgrub_a-lexer.o `test -f 'grub-core/script/lexer.c' 
|| echo './'`grub-core/script/lexer.c

grub-core/script/lexer.c: In function 'grub_script_lexer_yywrap':
grub-core/script/lexer.c:136: error: 'struct grub_lexer_param' has no 
member named 'rpl_getline'
grub-core/script/lexer.c:144: error: 'struct grub_lexer_param' has no 
member named 'rpl_getline'

grub-core/script/lexer.c: In function 'grub_script_lexer_init':
grub-core/script/lexer.c:222: error: 'struct grub_lexer_param' has no 
member named 'rpl_getline'



I believe that this comes from grub-core/gnulib/stdio.in.h, which
contains:

#if @GNULIB_GETLINE@
/* [...] */
# if @REPLACE_GETLINE@
#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
#   undef getline
#   define getline rpl_getline
#  endif

On my system, both @GNULIB_GETLINE@ and @REPLACE_GETLINE@ get replaced
by 1 in the generated file grub-core/gnulib/stdio.h.  This file gets
included in the above compilation command, which leads to a
substitution of `getline' by `rpl_getline' in lexer.c, even for the
accesses to the field `getline' of the struct grub_lexer_param.

The attached patch simply renames this field by `getnewline'.

Grégoire
=== modified file 'ChangeLog'
--- ChangeLog   2010-09-23 00:10:44 +
+++ ChangeLog   2010-09-23 11:18:51 +
@@ -1,3 +1,10 @@
+2010-09-23  Grégoire Sutre  
+
+   * include/grub/script_sh.h (grub_lexer_param): Renamed field `getline'
+   into `getnewline'.
+   * grub-core/script/lexer.c (grub_script_lexer_yywrap): Likewise.
+   (grub_script_lexer_init): Likewise.
+
 2010-09-23  Vladimir Serbinenko  
 
Support xz compression on yeeloong.

=== modified file 'grub-core/script/lexer.c'
--- grub-core/script/lexer.c2010-09-04 05:26:23 +
+++ grub-core/script/lexer.c2010-09-23 07:32:35 +
@@ -133,7 +133,7 @@ grub_script_lexer_yywrap (struct grub_pa
   if (! lexerstate->refs && ! lexerstate->prefix && ! input)
 return 1;
 
-  if (! lexerstate->getline && ! input)
+  if (! lexerstate->getnewline && ! input)
 {
   grub_script_yyerror (parserstate, "unexpected end of file");
   return 1;
@@ -141,7 +141,7 @@ grub_script_lexer_yywrap (struct grub_pa
 
   line = 0;
   if (! input)
-lexerstate->getline (&line, 1);
+lexerstate->getnewline (&line, 1);
   else
 line = grub_strdup (input);
 
@@ -219,7 +219,7 @@ grub_script_lexer_init (struct grub_pars
   return 0;
 }
 
-  lexerstate->getline = getline;   /* rest are all zeros already */
+  lexerstate->getnewline = getline;/* rest are all zeros already */
   if (yylex_init (&lexerstate->yyscanner))
 {
   grub_free (lexerstate->text);

=== modified file 'include/grub/script_sh.h'
--- include/grub/script_sh.h2010-09-04 17:04:32 +
+++ include/grub/script_sh.h2010-09-23 07:33:42 +
@@ -160,7 +160,7 @@ struct grub_lexer_param
 {
   /* Function used by the lexer to get a new line when more input is
  expected, but not available.  */
-  grub_reader_getline_t getline;
+  grub_reader_getline_t getnewline;
 
   /* A reference counter.  If this is >0 it means that the parser
  expects more tokens and `getline' should be called to fetch more.

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Portable mktemp invocation?

[Grégoire Sutre]

Hi,

We use `mktemp' or `mktemp -d' (with no argument) in the shell scripts:

- grub-core/genmod.sh.in
- tests/util/grub-shell.in
- tests/util/grub-shell-tester.in
- tests/grub_script_blockarg.in
- tests/partmap_test.in
- util/powerpc/ieee1275/grub-mkrescue.in

But such invocations of mktemp fail on some systems (NetBSD, and,
according to their man pages, FreeBSD and MacOS X).

A simple solution would be to replace those invocations with:

mktemp [-d] ${TMPDIR:-/tmp}/tmp.XX

What do you think?  Is there a better alternative?

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Uninitialized variable

[Grégoire Sutre]

Hi,

This is catched (as a warning) by gcc 4.1.3 -- gcc 4.4 does not
complain (maybe some command-line option is missing?).

Grégoire


2010-09-23  Grégoire Sutre  

* grub-core/commands/acpihalt.c (get_sleep_type): Initialize prev.

=== modified file 'grub-core/commands/acpihalt.c'
--- grub-core/commands/acpihalt.c   2010-09-13 18:29:15 +
+++ grub-core/commands/acpihalt.c   2010-09-23 13:23:46 +
@@ -134,11 +134,11 @@ skip_ext_op (const grub_uint8_t *ptr, co
 }

 static int
 get_sleep_type (grub_uint8_t *table, grub_uint8_t *end)
 {
-  grub_uint8_t *ptr, *prev;
+  grub_uint8_t *ptr, *prev = 0;
   int sleep_type = -1;

   ptr = table + sizeof (struct grub_acpi_table_header);
   while (ptr < end && prev < ptr)
 {



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix getline name clash

[Grégoire Sutre]

On 09/24/2010 12:18, Colin Watson wrote:


This looks good to me as far as it goes, but doesn't it need a bit more?


I guess we should not rename the actual calls to the getline() library
function (otherwise gnulib's replacement won't help).  And I personally
don't mind if function arguments and local variables are substituted.

But I agree that, in general, it would be safer to avoid names that
might be inadvertently replaced by gnulib's #defines.

Grégoire


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Portable mktemp invocation?

[Grégoire Sutre]

On 09/23/2010 06:27 PM, Grégoire Sutre wrote:


A simple solution would be to replace those invocations with:

mktemp [-d] ${TMPDIR:-/tmp}/tmp.XX


The attached patch implements this solution.  Build tested on Debian
GNU/Linux and NetBSD.

Grégoire
=== modified file 'ChangeLog'
--- ChangeLog	2010-09-25 05:18:48 +
+++ ChangeLog	2010-09-25 10:59:05 +
@@ -1,3 +1,16 @@
+2010-09-25  Grégoire Sutre  
+
+	Make mktemp invocations portable.
+
+	* grub-core/genmod.sh.in: Use mktemp with an explicit template.
+	* tests/grub_script_blockarg.in: Likewise.
+	* tests/partmap_test.in: Likewise.
+	* tests/util/grub-shell-tester.in: Likewise.
+	* tests/util/grub-shell.in: Likewise.
+	* util/powerpc/ieee1275/grub-mkrescue.in: Likewise.
+	* Makefile.am: Likewise, and chain shell commands with `&&'
+	instead	of ';'.
+
 2010-09-25  BVK Chaitanya  
 
 	* grub-core/kern/emu/full.c (grub_emu_post_init):  Fix typo.

=== modified file 'Makefile.am'
--- Makefile.am	2010-09-20 12:55:49 +
+++ Makefile.am	2010-09-25 10:45:56 +
@@ -189,31 +189,31 @@ kopenbsd.init.x86_64: $(srcdir)/grub-cor
 	$(TARGET_CC) -o $@ $< -m64 -DTARGET_OPENBSD=1 -nostdlib -nostdinc -DSUCCESSFUL_BOOT_STRING=\"$(SUCCESSFUL_BOOT_STRING)\"
 
 linux-initramfs.i386: linux.init.i386 Makefile
-	TDIR=`mktemp -d`; cp $< $$TDIR/init; (cd $$TDIR; echo ./init | cpio --quiet --dereference -o -H newc) | gzip > $@; rm -rf $$TDIR
+	TDIR=`mktemp -d $${TMPDIR:-/tmp}/tmp.XX` && cp $< $$TDIR/init && (cd $$TDIR && echo ./init | cpio --quiet --dereference -o -H newc) | gzip > $@ && rm -rf $$TDIR
 
 linux-initramfs.x86_64: linux.init.x86_64 Makefile
-	TDIR=`mktemp -d`; cp $< $$TDIR/init; (cd $$TDIR; echo ./init | cpio --quiet --dereference -o -H newc) | gzip > $@; rm -rf $$TDIR
+	TDIR=`mktemp -d $${TMPDIR:-/tmp}/tmp.XX` && cp $< $$TDIR/init && (cd $$TDIR && echo ./init | cpio --quiet --dereference -o -H newc) | gzip > $@ && rm -rf $$TDIR
 
 kfreebsd-mfsroot.i386.img: kfreebsd.init.i386 Makefile
-	TDIR=`mktemp -d`; mkdir $$TDIR/dev; mkdir $$TDIR/sbin; cp $< $$TDIR/sbin/init; makefs -t ffs -s 30m -f 1000 -o minfree=0,version=1 $@ $$TDIR; rm -rf $$TDIR
+	TDIR=`mktemp -d $${TMPDIR:-/tmp}/tmp.XX` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -t ffs -s 30m -f 1000 -o minfree=0,version=1 $@ $$TDIR && rm -rf $$TDIR
 
 knetbsd.image.i386: knetbsd.init.i386 $(srcdir)/grub-core/tests/boot/kbsd.spec.txt
-	TDIR=`mktemp -d` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 64k -f 10 -o minfree=0,version=1 $@ $$TDIR && rm -rf $$TDIR
+	TDIR=`mktemp -d $${TMPDIR:-/tmp}/tmp.XX` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 64k -f 10 -o minfree=0,version=1 $@ $$TDIR && rm -rf $$TDIR
 
 kopenbsd.image.i386: kopenbsd.init.i386 $(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt
-	TDIR=`mktemp -d` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 128k -f 10 -o minfree=0,version=1 $@ $$TDIR && bsdlabel -f -R $@ $(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt && rm -rf $$TDIR || rm -f $@
+	TDIR=`mktemp -d $${TMPDIR:-/tmp}/tmp.XX` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 128k -f 10 -o minfree=0,version=1 $@ $$TDIR && bsdlabel -f -R $@ $(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt && rm -rf $$TDIR || rm -f $@
 
 kopenbsd.image.x86_64: kopenbsd.init.x86_64 $(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt
-	TDIR=`mktemp -d` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 128k -f 10 -o minfree=0,version=1 $@ $$TDIR && bsdlabel -f -R $@ $(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt && rm -rf $$TDIR || rm -f $@
+	TDIR=`mktemp -d $${TMPDIR:-/tmp}/tmp.XX` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 128k -f 10 -o minfree=0,version=1 $@ $$TDIR && bsdlabel -f -R $@ $(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt && rm -rf $$TDIR || rm -f $@
 
 knetbsd.miniroot-image.i386.img: knetbsd.image.i386 $(GRUB_PAYLOADS_DIR)/knetbsd.miniroot.i386
 	$(OBJCOPY) --add-section=miniroot=$< $(GRUB_PAYLOADS_DIR)/knetbsd.miniroot.i386

Re: Guidance on conflicts between GNU GRUB and proprietary software

[Grégoire Sutre]

On 09/28/2010 09:05 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:


Using an embedding partition on msdos as an optional alternative (not as
replacement) to MBR gap is a clear possibility, good idea and was
proposed before but details are very unclear. Like:
- How to create such partition
- How does grub find it and ensures that it's an embedding partition?
Any false positive will result in data loss. Obviously no msdos type is
completely unused by now so it's not a way.


We could borrow a partition type that is reliably used only by boot 
managers (for similar purposes).  Ok, we may overwrite some boot manager 
code sitting in that partition.  But would this be different from what 
happens in the GPT case?


Regarding the question of whether such an MSDOS partition type exists, I 
did not look very hard, but 45h comes to mind.  This type is used by the 
Boot-US boot manager.  It is reported as such by NetBSD fdisk.  It is 
not listed in the known types of Linux fdisk.  Are you aware of other 
uses for this partition type?


If we really want to embed in an MSDOS partition, selecting a partition 
type that is already used for similar purposes is IMHO our best option. 
This would be a step in a direction to set a ``standard'' MSDOS type for 
boot partitions.  We don't need one partition type per boot-loader 
software anyway.


Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Guidance on conflicts between GNU GRUB and proprietary software

[Grégoire Sutre]

On 09/29/2010 00:11, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Let me try to explain more clearly what I have in mind.  In the GPT
case, we have a dedicated partition ID for bootloader software.  This
makes things simpler.  If we had such a dedicated partition ID in the
MSDOS case, then a lot of embedding problems would (a priori) go away.
So it's worth trying to ``establish'' such a partition ID for MSDOS
partitions.  And good candidates are partition IDs that are already
(almost exclusively) used by some bootloader software(s), because

- those partitions are very likely to only contain bootloader code
  and configuration, so the risk of data loss when installing GRUB
  is minimized.

- OSes and partitioning software may already know about them, which
  would limit the risk of losing the embedded GRUB code.



If we really want to embed in an MSDOS partition, selecting a
partition type that is already used for similar purposes is IMHO
our best option. This would be a step in a direction to set a
``standard'' MSDOS type for boot partitions.  We don't need one
partition type per boot-loader software anyway.

OS/2 used type 0x0a. If someone wants to install the OS/2 on his
computer (I wouldn't attempt it on anything newer than Pentium 3)
then he probably wants to chainload it (direct loading OS/2 would
require much work and has no point, adapting ntldr would be possible
 though, but I wouldn't spend any time on it). So he needs 2 boot
managers. Probably there are other similar cases as well.


I didn't mean that we only need one partition for bootloaders.  What I
mean is that we only need one partition type.  Such as, if you install
two Linux distributions, you would have the same ID for all your Linux
partitions.

But I agree that, if we had two MSDOS partitions with the same
``bootloader'' type, then grub-install shouldn't blindly take the first
one.  Still, isn't this what happens in GPT case, when there are two
BIOS Boot partitions?  Or maybe, in the GPT case, there is no point in
having several BIOS Boot partitions?

Anyway, I'm sure that we can find solutions to make sure that, in a
non-interactive mode, a new GRUB installation only overwrites a previous
GRUB installation.



Deleting some other bootloader may also appear unfair and lead to
data loss if its partition contained anything useful.


In the GPT case, doesn't grub-install delete existing data in the GPT
BIOS Boot partition?



I don't mind sharing the embedding partition on the rule "who gets
MBR gets this bonus,


I didn't see it this way.  I assumed we could have different bootloader
software in different partitions of the ``bootloader type''.



all possible occupants have to implement multiboot to be chainloaded
if necessary" but former users of partition type may disagree.


Yes, of course, if we try to go this route, we would have to discuss
it with former users of the partition type.

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Portable mktemp invocation?

[Grégoire Sutre]

This is a new version of the patch.  Changes with respect to the previous
version:

- use double quotes (i.e. "${TMPDIR:-/tmp}/tmp.XX")
- apply the same scheme to util/grub-mkrescue.in (intead of MKTEMP_TEMPLATE)

Grégoire
2010-10-11  Grégoire Sutre  

Make mktemp invocations portable.

* grub-core/genmod.sh.in: Use mktemp with an explicit template, and
exit if mktemp fails.
* tests/grub_script_blockarg.in: Likewise.
* tests/partmap_test.in: Likewise.
* tests/util/grub-shell-tester.in: Likewise.
* tests/util/grub-shell.in: Likewise.
* util/powerpc/ieee1275/grub-mkrescue.in: Likewise.
* Makefile.am: Likewise, and chain shell commands with `&&'
instead of ';'.
* util/grub-mkrescue.in: Use the same explicit template as above, and
exit if mktemp fails.
=== modified file 'Makefile.am'
--- Makefile.am 2010-09-20 12:55:49 +
+++ Makefile.am 2010-10-11 15:16:54 +
@@ -189,31 +189,31 @@ kopenbsd.init.x86_64: $(srcdir)/grub-cor
$(TARGET_CC) -o $@ $< -m64 -DTARGET_OPENBSD=1 -nostdlib -nostdinc 
-DSUCCESSFUL_BOOT_STRING=\"$(SUCCESSFUL_BOOT_STRING)\"
 
 linux-initramfs.i386: linux.init.i386 Makefile
-   TDIR=`mktemp -d`; cp $< $$TDIR/init; (cd $$TDIR; echo ./init | cpio 
--quiet --dereference -o -H newc) | gzip > $@; rm -rf $$TDIR
+   TDIR=`mktemp -d "$${TMPDIR:-/tmp}/tmp.XX"` && cp $< $$TDIR/init 
&& (cd $$TDIR && echo ./init | cpio --quiet --dereference -o -H newc) | gzip > 
$@ && rm -rf $$TDIR
 
 linux-initramfs.x86_64: linux.init.x86_64 Makefile
-   TDIR=`mktemp -d`; cp $< $$TDIR/init; (cd $$TDIR; echo ./init | cpio 
--quiet --dereference -o -H newc) | gzip > $@; rm -rf $$TDIR
+   TDIR=`mktemp -d "$${TMPDIR:-/tmp}/tmp.XX"` && cp $< $$TDIR/init 
&& (cd $$TDIR && echo ./init | cpio --quiet --dereference -o -H newc) | gzip > 
$@ && rm -rf $$TDIR
 
 kfreebsd-mfsroot.i386.img: kfreebsd.init.i386 Makefile
-   TDIR=`mktemp -d`; mkdir $$TDIR/dev; mkdir $$TDIR/sbin; cp $< 
$$TDIR/sbin/init; makefs -t ffs -s 30m -f 1000 -o minfree=0,version=1 $@ 
$$TDIR; rm -rf $$TDIR
+   TDIR=`mktemp -d "$${TMPDIR:-/tmp}/tmp.XX"` && mkdir $$TDIR/dev 
&& mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -t ffs -s 30m -f 1000 
-o minfree=0,version=1 $@ $$TDIR && rm -rf $$TDIR
 
 knetbsd.image.i386: knetbsd.init.i386 
$(srcdir)/grub-core/tests/boot/kbsd.spec.txt
-   TDIR=`mktemp -d` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< 
$$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t 
ffs -s 64k -f 10 -o minfree=0,version=1 $@ $$TDIR && rm -rf $$TDIR
+   TDIR=`mktemp -d "$${TMPDIR:-/tmp}/tmp.XX"` && mkdir $$TDIR/dev 
&& mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F 
$(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 64k -f 10 -o 
minfree=0,version=1 $@ $$TDIR && rm -rf $$TDIR
 
 kopenbsd.image.i386: kopenbsd.init.i386 
$(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt
-   TDIR=`mktemp -d` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< 
$$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t 
ffs -s 128k -f 10 -o minfree=0,version=1 $@ $$TDIR && bsdlabel -f -R $@ 
$(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt && rm -rf $$TDIR || rm -f $@
+   TDIR=`mktemp -d "$${TMPDIR:-/tmp}/tmp.XX"` && mkdir $$TDIR/dev 
&& mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F 
$(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 128k -f 10 -o 
minfree=0,version=1 $@ $$TDIR && bsdlabel -f -R $@ 
$(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt && rm -rf $$TDIR || rm -f $@
 
 kopenbsd.image.x86_64: kopenbsd.init.x86_64 
$(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt
-   TDIR=`mktemp -d` && mkdir $$TDIR/dev && mkdir $$TDIR/sbin && cp $< 
$$TDIR/sbin/init && makefs -F $(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t 
ffs -s 128k -f 10 -o minfree=0,version=1 $@ $$TDIR && bsdlabel -f -R $@ 
$(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt && rm -rf $$TDIR || rm -f $@
+   TDIR=`mktemp -d "$${TMPDIR:-/tmp}/tmp.XX"` && mkdir $$TDIR/dev 
&& mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -F 
$(srcdir)/grub-core/tests/boot/kbsd.spec.txt -t ffs -s 128k -f 10 -o 
minfree=0,version=1 $@ $$TDIR && bsdlabel -f -R $@ 
$(srcdir)/grub-core/tests/boot/kopenbsdlabel.txt && rm -rf $$TDIR || rm -f $@
 
 knetbsd.miniroot-image.i386.img: knetbsd.image.i386 
$(GRUB_PAYLOADS_DIR)/knetbsd.miniroot.i386
$(OBJCOPY) --add-section=miniroot=$< 
$(GRUB_PAYLOADS_DIR)/knetbsd.miniroot.i386 $@
 
 kfreebsd-mfsroot.x86_64.img: kfreebsd.init.x86_64 Makefile
-   TDIR=`mktemp -d`; mkdir $$TDIR/dev; mkdir $$TDIR/sbin; cp $< 
$$TDIR/sbin/init; makefs -t ffs -s 30m -f 1000 -o minfree=0,version=1 $@ 
$$TDIR; rm -rf $$TDIR
+   TDIR=`mktemp -d "$${TMPDIR:-/tmp}/tmp.XX"` && mkdir $$TDIR/dev 
&& mkdir $$TDIR/sbin && cp $< $$TDIR/sbin/init && makefs -t ffs -s 30m -f 1000 
-o minfree=0,version

Re: Portable mktemp invocation?

[Grégoire Sutre]

On 10/16/2010 04:04 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Go ahead for mainline


Thanks, it's committed.

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix getline name clash

[Grégoire Sutre]

On 10/19/2010 11:17, BVK Chaitanya wrote:

How about the attached patch?


The patch fixes the problem. With the patch, grub trunk builds fine
on NetBSD 5.

Thanks,

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFT] nested partition issues

[Grégoire Sutre]

On 09/04/2010 02:07 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Hello. It was reported to me about several issues with nested
partitions. Please try attached patch and report  back any remaining
problems


NetBSD and OpenBSD disklabels that are nested in an MSDOS partition are
now viewed as top-level partitions, i.e. we now have for instance:

(hd0,netbsd5) instead of (hd0,msdos2,bsd5)

for a partition e: in the BSD disklabel contained in (hd0,msdos2).

This makes grub-setup fail since it detects two top-level partition
maps: msdos and netbsd.

Previously, grub's view of the partitions closely matched the actual
nesting on the disk.  I personally prefer this previous view.  In
particular, if the disk had only an MBR partition table and no other
top-level partition map, then the same applied to grub's view of the
partitions.

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFT] nested partition issues

[Grégoire Sutre]

Hi,

The main issue at hand is that, to support NetBSD, we need a working
grub-setup for the (very common) case where the NetBSD disklabel is placed
in an MSDOS partition.

If we need to add yet other ``strcmp (..._partmap->name, ...)'' tests to
grub-setup, then we likely have bad abstractions.



Actually now we follow the actual nesting of partitions. Even though
net-/openbsd label metadate is placed inside a partition it still
describes the whole disk as is manifested by it having entries for
partitions not contained inside the partition containing label metadata.
E.g.
(hd0,netbsd6) may be physically contained within (hd0,msdos3) but still
be described inside the label present in second sector (hd0,msdos2).
Place of metadata is secondary to deciding what the nesting of
partitions is. Primary criteria is what this metadata describes.


So, basically, as soon as a disklabel uses absolute offsets, this disklabel
must be viewed as a top-level disklabel?

I'm afraid that this will make things more complicated than they should
be.  But that's just me.



This is, of course, very unfortunate design but since we support NetBSD
we need such hacks. It's better than being faced with the problems of
kind "My XYZOS handles my partition scheme perfectly but GRUB doesn't
see half of partitions."


I did not see reports of such problems (for NetBSD partitions) since I
merged the code to ``external'' partitions [1].  Could you please point me
to such reports?

In the previous implementation, if partition e: in a NetBSD disklabel is
discarded, it's because e: describes an ``external'' partition.  This
external partition is (in all useful cases) described in another partition
map, where it is properly nested, hence it will be found by GRUB.

Grégoire

[1] http://lists.gnu.org/archive/html/grub-devel/2010-07/msg00109.html

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Build failure on NetBSD

[Grégoire Sutre]

Hi,

Trunk does not build on NetBSD (it's 5.99.41 in case that matters).

$ (./autogen.sh ; ./configure ; gmake) > /tmp/grub-build.log 2>&1

gives, at the end of the log:

grub-core/kern/emu/hostdisk.c:102:27: error: libdevmapper.h: No such file or 
directory
grub-core/kern/emu/misc.c:46:27: error: libdevmapper.h: No such file or 
directory

gmake: *** [libgrub.pp] Error 1

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Build failure on NetBSD

[Grégoire Sutre]

On 01/06/2011 18:51, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

This means that HAVE_DEVICE_MAPPER is defined. It shouldn't be. Can you
post config.log?


(Too big for the list.)

http://pastebin.com/YnFzxHKP

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Build failure on NetBSD

[Grégoire Sutre]

On 01/06/2011 19:18, Grégoire Sutre wrote:

On 01/06/2011 18:51, Vladimir 'φ-coder/phcoder' Serbinenko wrote:

This means that HAVE_DEVICE_MAPPER is defined. It shouldn't be. Can you
post config.log?


On this system, there is a /lib/libdevmapper.so but no libdevmapper.h.
Afaics, configure does not check for libdevmapper.h.

Grégoire

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Build failure on NetBSD

[Grégoire Sutre]

Looks like you actualy do have libdevmapper (I'm surprised it exists for
NetBSD) but not the headers.


I'm not sure what it's for (I don't use LVM).  The utility dmsetup(8)
(ported from Linux?) requires it.

> We should check for headers too.

Attached patch adds this.

Grégoire
=== modified file 'ChangeLog'
--- ChangeLog	2011-01-06 13:24:38 +
+++ ChangeLog	2011-01-06 21:27:31 +
@@ -1,3 +1,7 @@
+2011-01-06  Grégoire Sutre  
+
+	* configure.ac: Check for libdevmapper header.
+
 2011-01-06  Colin Watson  
 
 	* tests/util/grub-shell.in: Set serial terminfo type to `dumb', to

=== modified file 'configure.ac'
--- configure.ac	2010-11-16 15:50:20 +
+++ configure.ac	2011-01-06 21:31:21 +
@@ -867,6 +867,12 @@ if test x"$enable_device_mapper" = xno ;
 fi
 
 if test x"$device_mapper_excuse" = x ; then
+  # Check for device-mapper header.
+  AC_CHECK_HEADER([libdevmapper.h], [],
+   [device_mapper_excuse="need libdevmapper header"])
+fi
+
+if test x"$device_mapper_excuse" = x ; then
   # Check for device-mapper library.
   AC_CHECK_LIB([devmapper], [dm_task_create], [],
[device_mapper_excuse="need devmapper library"])

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel