Bug#239111: Freeze when installing GRUB on XFS boot partition

2009-01-04 Thread Ben Hutchings
Here's another possibility: don't run xfs_freeze at all but instead run
a separate program that does ioctl(FIBMAP).  This works for me on ext3
in that it doesn't break installation.  Please can you test this with a
fresh installation of grub on XFS?

diff -urN grub-0.97.orig/util/grub-install.in grub-0.97/util/grub-install.in
--- grub-0.97.orig/util/grub-install.in 2004-07-24 19:57:31.0 +0100
+++ grub-0.97/util/grub-install.in  2009-01-04 19:26:31.0 +
@@ -30,6 +30,7 @@
 pkglibdir=${libdir}/${PACKAGE}/${host_cpu}-${host_vendor}
 
 grub_shell=${sbindir}/grub
+grub_map_file=${sbindir}/grub-map-file
 grub_set_default=${sbindir}/grub-set-default
 log_file=/tmp/grub-install.log.$$
 img_file=/tmp/grub-install.img.$$
@@ -471,6 +472,7 @@
 test -n $mklog  log_file=`$mklog`
 
 for file in ${grubdir}/stage1 ${grubdir}/stage2 ${grubdir}/*stage1_5; do
+$grub_map_file $file || exit 1
 count=5
 tmp=`echo $file | sed s|^${grubdir}|${grub_prefix}|`
 while test $count -gt 0; do
diff -urN grub-0.97.orig/util/grub-map-file.c grub-0.97/util/grub-map-file.c
--- grub-0.97.orig/util/grub-map-file.c 1970-01-01 01:00:00.0 +0100
+++ grub-0.97/util/grub-map-file.c  2009-01-04 19:19:44.0 +
@@ -0,0 +1,76 @@
+/* grub-map-file - force block-mapping for a file using ioctl(FIBMAP) */
+/*
+ *  Copyright (C) 2008  Ben Hutchings b...@decadent.org.uk.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#define _FILE_OFFSET_BITS 64
+
+#include errno.h
+#include stdio.h
+#include stdint.h
+#include stdlib.h
+#include string.h
+
+#include sys/types.h
+#include unistd.h
+#include fcntl.h
+#include sys/ioctl.h
+#include sys/stat.h
+#include linux/fs.h
+
+int
+main (int argc, char **argv)
+{
+  int fd;
+  struct stat stats;
+  unsigned long n, i;
+
+  if (argc != 2)
+{
+  fprintf (stderr, Usage: grub-map-file FILENAME\n);
+  exit (2);
+}
+
+  fd = open (argv[1], O_RDONLY);
+  if (fd  0)
+{
+  fprintf (stderr, grub-map-file: open %s: %s\n,
+   argv[1], strerror (errno));
+  exit (1);
+}
+
+  if (fstat (fd, stats))
+{
+  fprintf (stderr, grub-map-file: stat %s: %s\n,
+   argv[1], strerror (errno));
+  exit (1);
+}
+  n = (stats.st_size + stats.st_blksize - 1) / stats.st_blksize;
+
+  for (i = 0; i  n; i++)
+{
+  unsigned long temp = i;
+  if (ioctl (fd, FIBMAP, temp))
+   {
+ fprintf (stderr, grub-map-file: FIBMAP %s: %s\n,
+  argv[1], strerror (errno));
+ exit (1);
+   }
+}
+
+  return 0;
+}
diff -urN grub-0.97.orig/util/Makefile.am grub-0.97/util/Makefile.am
--- grub-0.97.orig/util/Makefile.am 2004-06-19 20:52:06.0 +0100
+++ grub-0.97/util/Makefile.am  2009-01-04 19:21:05.0 +
@@ -1,4 +1,5 @@
 bin_PROGRAMS = mbchk
+sbin_PROGRAMS = grub-map-file
 sbin_SCRIPTS = grub-install grub-md5-crypt grub-terminfo \
grub-set-default
 noinst_SCRIPTS = grub-image mkbimage
@@ -10,3 +11,5 @@
 
 mbchk_SOURCES = mbchk.c
 mbchk_LDADD = ../lib/libcommon.a
+
+grub_map_file_SOURCES =  grub-map-file.c
--- END ---

If that doesn't work, please also test my previously proposed patch:

--- grub-0.97.orig/util/grub-install.in 2004-07-24 19:57:31.0 +0100
+++ grub-0.97/util/grub-install.in  2009-01-04 04:57:47.0 +
@@ -418,6 +418,12 @@
 # Make a default file.
 ${grub_set_default} --root-directory=${rootdir} default
 
+# XFS is special; we must run xfs_freeze to flush files as well as sync
+# (which grub itself does).
+if [ $(stat -f --printf=%T ${grubdir}) = xfs ]; then
+xfs_freeze -f ${grubdir}  xfs_freeze -u ${grubdir}
+fi
+
 # Make sure that GRUB reads the same images as the host OS.
 test -n $mkimg  img_file=`$mkimg`
 test -n $mklog  log_file=`$mklog`
--- END ---

Either of these patches should replace debian/patches/xfs_freeze.diff in
the source package.

Ben.

-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949


signature.asc
Description: This is a digitally signed message part


Bug#239111: Freeze when installing GRUB on XFS boot partition

2009-01-03 Thread Ben Hutchings
The grub shell already does sync() - many times - so I think we can just
change xfs_freeze.diff to:

--- grub-0.97.orig/util/grub-install.in 2004-07-24 19:57:31.0 +0100
+++ grub-0.97/util/grub-install.in  2009-01-04 04:57:47.0 +
@@ -418,6 +418,12 @@
 # Make a default file.
 ${grub_set_default} --root-directory=${rootdir} default
 
+# XFS is special; we must run xfs_freeze to flush files as well as sync
+# (which grub itself does).
+if [ $(stat -f --printf=%T ${grubdir}) = xfs ]; then
+xfs_freeze -f ${grubdir}  xfs_freeze -u ${grubdir}
+fi
+
 # Make sure that GRUB reads the same images as the host OS.
 test -n $mkimg  img_file=`$mkimg`
 test -n $mklog  log_file=`$mklog`
--- END ---

It is probably better to use FIBMAP, but it's kind of hard to map a
device name to a mount point.  The following is an attempt to do that.
It does not use the result of FIBMAP but only uses it to force block
allocation.

However, somehow this triggers Error 28: Selected item cannot fit into
memory.  So it should certainly not be applied, but it may be useful to
anyone who wants to investigate further. 

--- grub-0.97.orig/stage2/disk_io.c 2004-05-23 17:35:24.0 +0100
+++ grub-0.97/stage2/disk_io.c  2009-01-04 06:36:26.0 +
@@ -29,6 +29,17 @@
 
 #ifdef GRUB_UTIL
 # include device.h
+# ifdef __linux__
+#   include errno.h
+#   include stdio.h
+#   include stdlib.h
+#   include sys/types.h
+#   include unistd.h
+#   include fcntl.h
+#   include sys/ioctl.h
+#   include sys/stat.h
+#   include linux/fs.h
+# endif
 #endif
 
 /* instrumentation variables */
@@ -1530,6 +1541,124 @@
 #endif /* STAGE1_5 */
 
 
+#if defined(GRUB_UTIL)  defined(__linux__)
+
+static int
+find_mount_point (dev_t block_dev, char **p_mount_point)
+{
+  static char line[256];
+
+  struct stat stats;
+  FILE *mounts;
+  char *begin, *end, *mount_point;
+  int result = -1;
+
+  mounts = fopen (/proc/mounts, r);
+  if (!mounts)
+return -1;
+
+  errno = 0;
+  mount_point = NULL;
+  while (fgets (line, sizeof(line), mounts))
+{
+  if (line[0] != '/')
+   continue;
+  end = strstr (line,  );
+  if (!end)
+   goto out_close;
+  *end = 0;
+  if (stat (line, stats))
+   goto out_close;
+  if (S_ISBLK (stats.st_mode)  stats.st_rdev == block_dev)
+   {
+ begin = end + 1;
+ end = strstr (begin,  );
+ if (!end)
+   goto out_close;
+ *end = 0;
+ mount_point = malloc (end + 1 - begin);
+ if (!mount_point)
+   goto out_close;
+ strcpy (mount_point, begin);
+ break;
+   }
+}
+  if (errno == 0)
+{
+  free (*p_mount_point);
+  *p_mount_point = mount_point;
+  result = 0;
+}
+
+ out_close:
+  fclose(mounts);
+  return result;
+}
+
+/*
+ * If filename is on a mounted filesystem, get its block list, forcing
+ * block allocation on a filesystem with deferred allocation.
+ */
+static int
+map_file (char *filename)
+{
+  static char *mount_point;
+  static dev_t cached_block_dev;
+  static char full_filename[PATH_MAX];
+
+  dev_t block_dev;
+  int block_fd = buf_geom.flags, fd;
+  struct stat stats;
+  unsigned long block_num;
+  int result = -1;
+
+  if (fstat (block_fd, stats) || !S_ISBLK (stats.st_mode))
+return -1;
+
+  block_dev = stats.st_rdev;
+  if ((current_partition  0x00FF00) == 0x00FF00)
+block_dev += ((current_partition  16)  0xFF) + 1;
+  if (block_dev != cached_block_dev 
+  find_mount_point(block_dev, mount_point))
+return -1;
+  cached_block_dev = block_dev;
+
+  if (!mount_point)
+return 0;
+
+  if (snprintf (full_filename, sizeof(full_filename),
+   %s%s, mount_point, filename) =
+  sizeof(full_filename))
+return -1;
+  fd = open (full_filename, O_RDONLY);
+  if (fd  0)
+return -1;
+
+  if (fstat (fd, stats))
+goto out_close;
+  for (block_num = 0;
+   block_num  (stats.st_size + stats.st_blksize - 1) / stats.st_blksize;
+   block_num++)
+if (ioctl (fd, FIBMAP, block_num)  0)
+  goto out_close;
+  result = 0;
+
+ out_close:
+  close (fd);
+  return result;
+}
+
+#else /* !(GRUB_UTIL  __linux__) */
+
+static int
+map_file (char *filename)
+{
+  return 0;
+}
+
+#endif /* GRUB_UTIL  __linux__ */
+
+
 /*
  *  This is the generic file open function.
  */
@@ -1622,6 +1751,11 @@
   errnum = ERR_BAD_FILENAME;
 #endif /* NO_BLOCK_FILES */
 }
+  else /* normal filename */
+{
+  if (map_file (filename))
+   errnum = ERR_DEV_VALUES;
+}
 
   if (!errnum  fsys_type == NUM_FSYS)
 errnum = ERR_FSYS_MOUNT;
--- END ---

Ben.

-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had thought.
... I realized that a large part of my life from then on was going to be spent
in finding mistakes in my own programs. - Maurice Wilkes, 1949


signature.asc
Description: This is a digitally signed message part