Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-24 Thread Robert Millan

Ok, let's try this:  We refuse to install on a partition UNLESS:

  - A filesystem can be identified in it.

  - This filesystem is known to reserve the first block for DOS-style
chainload.

If these conditions aren't met, user will have to override our check.

Patch attached.  Also in people/robertmh/grub-setup-fs-probe.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
=== modified file 'ChangeLog'
--- ChangeLog	2009-10-24 23:13:27 +
+++ ChangeLog	2009-10-24 23:57:58 +
@@ -1,5 +1,15 @@
 2009-10-25  Robert Millan  
 
+	* include/grub/fs.h (struct grub_fs): Add `reserved_first_sector'
+	member.
+	* fs/ext2.c (grub_ext2_fs): Initialize `reserved_first_sector' to 1.
+	* util/i386/pc/grub-setup.c (setup): Add safety check that probes for
+	filesystems which begin at first sector.
+	(options): New option --skip-fs-probe.
+	(main): Handle --skip-fs-probe and pass it to setup().
+
+2009-10-25  Robert Millan  
+
 	* fs/cpio.c [MODE_USTAR]: Finish `tar' module instead of
 	`cpio'.
 	[! MODE_USTAR]: Finish `cpio' module instead of `tar'.

=== modified file 'fs/ext2.c'
--- fs/ext2.c	2009-07-19 13:59:21 +
+++ fs/ext2.c	2009-10-24 23:57:58 +
@@ -924,6 +924,7 @@
 .label = grub_ext2_label,
 .uuid = grub_ext2_uuid,
 .mtime = grub_ext2_mtime,
+.reserved_first_sector = 1,
 .next = 0
   };
 

=== modified file 'include/grub/fs.h'
--- include/grub/fs.h	2009-06-10 21:04:23 +
+++ include/grub/fs.h	2009-10-24 23:57:58 +
@@ -68,6 +68,11 @@
   /* Get writing time of filesystem. */
   grub_err_t (*mtime) (grub_device_t device, grub_int32_t *timebuf);
 
+#ifdef GRUB_UTIL
+  /* Whether this filesystem reserves first sector for DOS-style boot.  */
+  int reserved_first_sector;
+#endif
+
   /* The next filesystem.  */
   struct grub_fs *next;
 };

=== modified file 'util/i386/pc/grub-setup.c'
--- util/i386/pc/grub-setup.c	2009-08-25 08:28:13 +
+++ util/i386/pc/grub-setup.c	2009-10-24 23:57:58 +
@@ -86,7 +86,7 @@
 static void
 setup (const char *dir,
const char *boot_file, const char *core_file,
-   const char *root, const char *dest, int must_embed, int force)
+   const char *root, const char *dest, int must_embed, int force, int fs_probe)
 {
   char *boot_path, *core_path, *core_path_dev;
   char *boot_img, *core_img;
@@ -251,6 +251,21 @@
   if (grub_disk_read (dest_dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, tmp_img))
 grub_util_error ("%s", grub_errmsg);
 
+  if (dest_dev->disk->partition && fs_probe)
+{
+  grub_fs_t fs;
+  fs = grub_fs_probe (dest_dev);
+  if (! fs)
+	grub_util_error ("Unable to identify a filesystem in %s; safety check can't be performed.");
+
+  if (! fs->reserved_first_sector)
+	grub_util_error ("%s appears to contain a %s filesystem which isn't known to "
+			 "reserve space for DOS-style boot.  Installing GRUB there could "
+			 "result in FILESYSTEM DESTRUCTION if valuable data is overwritten "
+			 "by grub-setup (--skip-fs-probe disables this "
+			 "check, use at your own risk).", dest_dev->disk->name, fs->name);
+}
+
   /* Copy the possible DOS BPB.  */
   memcpy (boot_img + GRUB_BOOT_MACHINE_BPB_START,
 	  tmp_img + GRUB_BOOT_MACHINE_BPB_START,
@@ -556,6 +571,7 @@
 {"device-map", required_argument, 0, 'm'},
 {"root-device", required_argument, 0, 'r'},
 {"force", no_argument, 0, 'f'},
+{"skip-fs-probe", no_argument, 0, 's'},
 {"help", no_argument, 0, 'h'},
 {"version", no_argument, 0, 'V'},
 {"verbose", no_argument, 0, 'v'},
@@ -580,6 +596,7 @@
   -m, --device-map=FILE   use FILE as the device map [default=%s]\n\
   -r, --root-device=DEV   use DEV as the root device [default=guessed]\n\
   -f, --force install even if problems are detected\n\
+  -s, --skip-fs-probe do not probe for filesystems in DEVICE\n\
   -h, --help  display this message and exit\n\
   -V, --version   print version information and exit\n\
   -v, --verbose   print verbose messages\n\
@@ -613,7 +630,7 @@
   char *dev_map = 0;
   char *root_dev = 0;
   char *dest_dev;
-  int must_embed = 0, force = 0;
+  int must_embed = 0, force = 0, fs_probe = 1;
 
   progname = "grub-setup";
 
@@ -666,6 +683,10 @@
 	force = 1;
 	break;
 
+	  case 's':
+	fs_probe = 0;
+	break;
+
 	  case 'h':
 	usage (0);
 	break;
@@ -767,7 +788,7 @@
 	  setup (dir ? : DEFAULT_DIRECTORY,
 		 boot_file ? : DEFAULT_BOOT_FILE,
 		 core_file ? : DEFAULT_CORE_FILE,
-		 root_dev, grub_util_get_grub_dev (devicelist[i]), 1, force);
+		 root_dev, grub_util_get_grub_dev (devicelist[i]), 1, force, fs_probe);
 	}
 }
   else
@@ -776,7 +797,7 @@
 setup (dir ? : DEFAULT_DIRECTORY,
 	   boot_file ? : DEFAULT_BOOT_FILE,
 	   core_file ? : DEFAULT_CORE_FILE,
-	   root_dev, dest_dev, must_embed, force);
+	 

Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-20 Thread Vladimir 'phcoder' Serbinenko
Robert Millan wrote:
> On Sun, Oct 18, 2009 at 06:30:11PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>   
>> Robert Millan wrote:
>> 
>>> On Sat, Oct 17, 2009 at 02:09:31PM +0200, Vladimir 'phcoder' Serbinenko 
>>> wrote:
>>>   
>>>   
>> The danger is that fs_probe may reject filesystem as valid just because
>> it's newer than expected.
>> 
>> 
>> 
> What do you mean with "reject filesystem as valid"?
>
>   
>   
>   
 Sorry for being unclear. I just meant that if some XFS structures are
 updated then our xfs driver won't recognise it as xfs
 
 
>>> What do you mean with "updated"?  You mean a new implementation of XFS?  Or
>>> the same instance of XFS that has been modified after use?
>>>
>>>   
>>>   
>> I mean next version on XFS
>> 
>
> Sorry, but what did you expect?  You want to prevent PEBCAK using
> heuristic.  There's no way we can tell if those 512 bytes are valuable
> data, only the user can.  And even if you try to err on the safest side,
> there's no garantee that newer versions of XFS, or other filesystems that
> don't even exist yet will be detectable by us no matter what we do.
>
>   
I think best way is to have whitelist of OS which have first sector free
and possible manual override like it was suggested.
> Why don't we just take a backup like someone suggested a while ago?  I
> think there was even a patch.  This way if valuable data is lost, user can
> restore it (and while at it, learnt his lesson that filesystems and embedded
> code aren't really supposed to be mixed in the same place).
>
>   
The backup is inevitably stored on the filesystem itself which becomes
inaccessible once superblock is destroyed.


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 



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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-20 Thread Robert Millan
On Sun, Oct 18, 2009 at 06:30:11PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Robert Millan wrote:
> > On Sat, Oct 17, 2009 at 02:09:31PM +0200, Vladimir 'phcoder' Serbinenko 
> > wrote:
> >   
>  The danger is that fs_probe may reject filesystem as valid just because
>  it's newer than expected.
>  
>  
> >>> What do you mean with "reject filesystem as valid"?
> >>>
> >>>   
> >>>   
> >> Sorry for being unclear. I just meant that if some XFS structures are
> >> updated then our xfs driver won't recognise it as xfs
> >> 
> >
> > What do you mean with "updated"?  You mean a new implementation of XFS?  Or
> > the same instance of XFS that has been modified after use?
> >
> >   
> I mean next version on XFS

Sorry, but what did you expect?  You want to prevent PEBCAK using
heuristic.  There's no way we can tell if those 512 bytes are valuable
data, only the user can.  And even if you try to err on the safest side,
there's no garantee that newer versions of XFS, or other filesystems that
don't even exist yet will be detectable by us no matter what we do.

Why don't we just take a backup like someone suggested a while ago?  I
think there was even a patch.  This way if valuable data is lost, user can
restore it (and while at it, learnt his lesson that filesystems and embedded
code aren't really supposed to be mixed in the same place).

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-18 Thread Vladimir 'phcoder' Serbinenko
richardvo...@gmail.com wrote:
> On Sat, Oct 17, 2009 at 7:09 AM, Vladimir 'phcoder' Serbinenko
>  wrote:
>   
>> Robert Millan wrote:
>> 
>>> On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko 
>>> wrote:
>>>
>>>   
 Robert Millan wrote:

 
> On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko 
> wrote:
>
>
>   
>>  2009-10-16  Vladimir Serbinenko  
>>
>> +  * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS 
>> superblock.
>> +  (options): New option --destroy-xfs.
>> +  (main): Handle --destroy-xfs.
>>
>>
>> 
> I gave this some more thought, and I think this could be less ad-hoc.  
> We're
> treating XFS as if it were a "weird", unique thing just because it isn't 
> biased
> towards DOS-style boot like most filesystems are.
>
> Instead, I've done something more generic, using our standard filesystem
> probing engine which should be more reliable than a single memcmp.
>
>
>
>   
 The danger is that fs_probe may reject filesystem as valid just because
 it's newer than expected.

 
>>> What do you mean with "reject filesystem as valid"?
>>>
>>>
>>>   
>> Sorry for being unclear. I just meant that if some XFS structures are
>> updated then our xfs driver won't recognise it as xfs
>> 
>
>
> Then instead of blacklisting xfs, why not whitelist filesystems which
> are known to have usable blocks for embedding (doesn't the number of
> blocks vary with filesystem anyway?) and an override parameter
> (--into-unrecognized-fs).
>
>   
In long term it's the best possibility but in this case we're too near
to a release. What I want is just to avoid XFS pitfall many users may
step into
>> --
>> Regards
>> Vladimir 'phcoder' Serbinenko
>> Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
>>
>>
>>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> http://lists.gnu.org/mailman/listinfo/grub-devel
>>
>> 
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>   


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 



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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-18 Thread Vladimir 'phcoder' Serbinenko
Robert Millan wrote:
> On Sat, Oct 17, 2009 at 02:09:31PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>   
 The danger is that fs_probe may reject filesystem as valid just because
 it's newer than expected.
 
 
>>> What do you mean with "reject filesystem as valid"?
>>>
>>>   
>>>   
>> Sorry for being unclear. I just meant that if some XFS structures are
>> updated then our xfs driver won't recognise it as xfs
>> 
>
> What do you mean with "updated"?  You mean a new implementation of XFS?  Or
> the same instance of XFS that has been modified after use?
>
>   
I mean next version on XFS


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 



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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-18 Thread Robert Millan
On Sat, Oct 17, 2009 at 02:09:31PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> >> The danger is that fs_probe may reject filesystem as valid just because
> >> it's newer than expected.
> >> 
> >
> > What do you mean with "reject filesystem as valid"?
> >
> >   
> Sorry for being unclear. I just meant that if some XFS structures are
> updated then our xfs driver won't recognise it as xfs

What do you mean with "updated"?  You mean a new implementation of XFS?  Or
the same instance of XFS that has been modified after use?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-17 Thread richardvo...@gmail.com
On Sat, Oct 17, 2009 at 7:09 AM, Vladimir 'phcoder' Serbinenko
 wrote:
> Robert Millan wrote:
>> On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko 
>> wrote:
>>
>>> Robert Millan wrote:
>>>
 On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko 
 wrote:


>  2009-10-16  Vladimir Serbinenko  
>
> +  * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS 
> superblock.
> +  (options): New option --destroy-xfs.
> +  (main): Handle --destroy-xfs.
>
>
 I gave this some more thought, and I think this could be less ad-hoc.  
 We're
 treating XFS as if it were a "weird", unique thing just because it isn't 
 biased
 towards DOS-style boot like most filesystems are.

 Instead, I've done something more generic, using our standard filesystem
 probing engine which should be more reliable than a single memcmp.



>>> The danger is that fs_probe may reject filesystem as valid just because
>>> it's newer than expected.
>>>
>>
>> What do you mean with "reject filesystem as valid"?
>>
>>
> Sorry for being unclear. I just meant that if some XFS structures are
> updated then our xfs driver won't recognise it as xfs


Then instead of blacklisting xfs, why not whitelist filesystems which
are known to have usable blocks for embedding (doesn't the number of
blocks vary with filesystem anyway?) and an override parameter
(--into-unrecognized-fs).

>
>
> --
> Regards
> Vladimir 'phcoder' Serbinenko
> Personal git repository: http://repo.or.cz/w/grub2/phcoder.git
>
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>


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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-17 Thread Vladimir 'phcoder' Serbinenko
Robert Millan wrote:
> On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>   
>> Robert Millan wrote:
>> 
>>> On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko 
>>> wrote:
>>>   
>>>   
  2009-10-16  Vladimir Serbinenko  
  
 +  * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock.
 +  (options): New option --destroy-xfs.
 +  (main): Handle --destroy-xfs.
 
 
>>> I gave this some more thought, and I think this could be less ad-hoc.  We're
>>> treating XFS as if it were a "weird", unique thing just because it isn't 
>>> biased
>>> towards DOS-style boot like most filesystems are.
>>>
>>> Instead, I've done something more generic, using our standard filesystem
>>> probing engine which should be more reliable than a single memcmp.
>>>
>>>   
>>>   
>> The danger is that fs_probe may reject filesystem as valid just because
>> it's newer than expected.
>> 
>
> What do you mean with "reject filesystem as valid"?
>
>   
Sorry for being unclear. I just meant that if some XFS structures are
updated then our xfs driver won't recognise it as xfs


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 



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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-17 Thread Felix Zielcke
Am Samstag, den 17.10.2009, 14:00 +0200 schrieb Robert Millan:
> On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > Robert Millan wrote:
> > > On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko 
> > > wrote:
> > >   
> > >>  2009-10-16  Vladimir Serbinenko  
> > >>  
> > >> +* util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS 
> > >> superblock.
> > >> +(options): New option --destroy-xfs.
> > >> +(main): Handle --destroy-xfs.
> > >> 
> > >
> > > I gave this some more thought, and I think this could be less ad-hoc.  
> > > We're
> > > treating XFS as if it were a "weird", unique thing just because it isn't 
> > > biased
> > > towards DOS-style boot like most filesystems are.
> > >
> > > Instead, I've done something more generic, using our standard filesystem
> > > probing engine which should be more reliable than a single memcmp.
> > >
> > >   
> > The danger is that fs_probe may reject filesystem as valid just because
> > it's newer than expected.
> 
> What do you mean with "reject filesystem as valid"?

For example with the extN filesystems we reject them as valid if they
use INCOMPAT flags we don't support.
For example external ext3/4 journal devices, which caused a reboot or
segfault or something like that, before we commited Javier's patch for
it.
In that case it doestn't matter because the first sector is still unused
but for other filesystems this could maybe be a problem.



-- 
Felix Zielcke
Proud Debian Maintainer and GNU GRUB developer



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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-17 Thread Robert Millan
On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Robert Millan wrote:
> > On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko 
> > wrote:
> >   
> >>  2009-10-16  Vladimir Serbinenko  
> >>  
> >> +  * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock.
> >> +  (options): New option --destroy-xfs.
> >> +  (main): Handle --destroy-xfs.
> >> 
> >
> > I gave this some more thought, and I think this could be less ad-hoc.  We're
> > treating XFS as if it were a "weird", unique thing just because it isn't 
> > biased
> > towards DOS-style boot like most filesystems are.
> >
> > Instead, I've done something more generic, using our standard filesystem
> > probing engine which should be more reliable than a single memcmp.
> >
> >   
> The danger is that fs_probe may reject filesystem as valid just because
> it's newer than expected.

What do you mean with "reject filesystem as valid"?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-17 Thread Vladimir 'phcoder' Serbinenko
Robert Millan wrote:
> On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko wrote:
>   
>>  2009-10-16  Vladimir Serbinenko  
>>  
>> +* util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock.
>> +(options): New option --destroy-xfs.
>> +(main): Handle --destroy-xfs.
>> 
>
> I gave this some more thought, and I think this could be less ad-hoc.  We're
> treating XFS as if it were a "weird", unique thing just because it isn't 
> biased
> towards DOS-style boot like most filesystems are.
>
> Instead, I've done something more generic, using our standard filesystem
> probing engine which should be more reliable than a single memcmp.
>
>   
The danger is that fs_probe may reject filesystem as valid just because
it's newer than expected.
> I propose the attached patch.
>
>   
> 
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 



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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-17 Thread Robert Millan
On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko wrote:
>  2009-10-16  Vladimir Serbinenko  
>  
> + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock.
> + (options): New option --destroy-xfs.
> + (main): Handle --destroy-xfs.

I gave this some more thought, and I think this could be less ad-hoc.  We're
treating XFS as if it were a "weird", unique thing just because it isn't biased
towards DOS-style boot like most filesystems are.

Instead, I've done something more generic, using our standard filesystem
probing engine which should be more reliable than a single memcmp.

I propose the attached patch.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
=== modified file 'ChangeLog'
--- ChangeLog	2009-10-16 20:21:12 +
+++ ChangeLog	2009-10-17 11:19:35 +
@@ -1,3 +1,13 @@
+2009-10-17  Robert Millan  
+
+	* include/grub/fs.h (struct grub_fs): Add `begins_at_first_sector'
+	member.
+	* fs/xfs.c (grub_xfs_fs): Initialize `begins_at_first_sector' to 1.
+	* util/i386/pc/grub-setup.c (setup): Add safety check that probes for
+	filesystems which begin at first sector.
+	(options): New option --skip-fs-probe.
+	(main): Handle --skip-fs-probe and pass it to setup().
+
 2009-10-16  Vladimir Serbinenko  
 
 	Let user specify OpenBSD root device.

=== modified file 'fs/xfs.c'
--- fs/xfs.c	2009-08-26 14:17:34 +
+++ fs/xfs.c	2009-10-17 11:19:35 +
@@ -805,6 +805,9 @@
 .close = grub_xfs_close,
 .label = grub_xfs_label,
 .uuid = grub_xfs_uuid,
+#ifdef GRUB_UTIL
+.begins_at_first_sector = 1,
+#endif
 .next = 0
   };
 

=== modified file 'include/grub/fs.h'
--- include/grub/fs.h	2009-06-10 21:04:23 +
+++ include/grub/fs.h	2009-10-17 11:19:35 +
@@ -68,6 +68,12 @@
   /* Get writing time of filesystem. */
   grub_err_t (*mtime) (grub_device_t device, grub_int32_t *timebuf);
 
+#ifdef GRUB_UTIL
+  /* Whether this filesystem begins at first sector or reserves it for
+ DOS-style boot.  */
+  int begins_at_first_sector;
+#endif
+
   /* The next filesystem.  */
   struct grub_fs *next;
 };

=== modified file 'util/i386/pc/grub-setup.c'
--- util/i386/pc/grub-setup.c	2009-08-25 08:28:13 +
+++ util/i386/pc/grub-setup.c	2009-10-17 11:19:35 +
@@ -86,7 +86,7 @@
 static void
 setup (const char *dir,
const char *boot_file, const char *core_file,
-   const char *root, const char *dest, int must_embed, int force)
+   const char *root, const char *dest, int must_embed, int force, int fs_probe)
 {
   char *boot_path, *core_path, *core_path_dev;
   char *boot_img, *core_img;
@@ -251,6 +251,16 @@
   if (grub_disk_read (dest_dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, tmp_img))
 grub_util_error ("%s", grub_errmsg);
 
+  if (fs_probe)
+{
+  grub_fs_t fs;
+  fs = grub_fs_probe (dest_dev);
+  if (fs && fs->begins_at_first_sector)
+	grub_util_error ("%s appears to contain a %s filesystem, which would be "
+			 "overwritten by grub-setup (--skip-fs-probe disables this "
+			 "check, use at your own risk).", dest_dev->disk->name, fs->name);
+}
+
   /* Copy the possible DOS BPB.  */
   memcpy (boot_img + GRUB_BOOT_MACHINE_BPB_START,
 	  tmp_img + GRUB_BOOT_MACHINE_BPB_START,
@@ -556,6 +566,7 @@
 {"device-map", required_argument, 0, 'm'},
 {"root-device", required_argument, 0, 'r'},
 {"force", no_argument, 0, 'f'},
+{"skip-fs-probe", no_argument, 0, 's'},
 {"help", no_argument, 0, 'h'},
 {"version", no_argument, 0, 'V'},
 {"verbose", no_argument, 0, 'v'},
@@ -580,6 +591,7 @@
   -m, --device-map=FILE   use FILE as the device map [default=%s]\n\
   -r, --root-device=DEV   use DEV as the root device [default=guessed]\n\
   -f, --force install even if problems are detected\n\
+  -s, --skip-fs-probe do not probe for filesystems in DEVICE\n\
   -h, --help  display this message and exit\n\
   -V, --version   print version information and exit\n\
   -v, --verbose   print verbose messages\n\
@@ -613,7 +625,7 @@
   char *dev_map = 0;
   char *root_dev = 0;
   char *dest_dev;
-  int must_embed = 0, force = 0;
+  int must_embed = 0, force = 0, fs_probe = 1;
 
   progname = "grub-setup";
 
@@ -666,6 +678,10 @@
 	force = 1;
 	break;
 
+	  case 's':
+	fs_probe = 0;
+	break;
+
 	  case 'h':
 	usage (0);
 	break;
@@ -767,7 +783,7 @@
 	  setup (dir ? : DEFAULT_DIRECTORY,
 		 boot_file ? : DEFAULT_BOOT_FILE,
 		 core_file ? : DEFAULT_CORE_FILE,
-		 root_dev, grub_util_get_grub_dev (devicelist[i]), 1, force);
+		 root_dev, grub_util_get_grub_dev (devicelist[i]), 1, force, fs_probe);
 	}
 }
   else
@@ -776,7 +792,7 @@
 setup (dir ? : DEFAULT_DIRECTORY,
 	   boot_file ? : DEFAULT_BOOT_FILE,
 	   core_file ? : DEFAULT_CORE_FILE,
-	   root_dev, dest_dev,

Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Vladimir 'phcoder' Serbinenko
Robert Millan wrote:
> On Fri, Oct 16, 2009 at 11:08:31PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>   
>> Robert Millan wrote:
>> 
>>> On Fri, Oct 16, 2009 at 10:09:41PM +0200, Vladimir 'phcoder' Serbinenko 
>>> wrote:
>>>   
>>>   
 Robert Millan wrote:
 
 
> On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote:
>   
>   
>   
>> On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko 
>> wrote:
>> 
>> 
>> 
>>> +  if (memcmp (tmp_img, "XFSB", 4) == 0)
>>> +grub_util_error ("Can't install on XFS.");
>>>   
>>>   
>>>   
>> Can this error message give some more detail on what the problem is?
>> 
>> 
>> 
> I suggest something like:
>
>   grub_util_warn ("Refusing to overwrite XFS meta-data.");
>
> This is more informative, and with grub_util_warn() user has an 
> opportunity to
> override it if she knows what she's doing.
>
>   
>   
>   
 Installing with blocklists/to partition is considered
 backward-compatibility feature. We never supported a config with XFS why
 we would want bw-compat for it?
 
 
>>> Because we can't reliably tell if it's a config with XFS, only the user can.
>>> This is an issue for both MBR or PBR installs.
>>>
>>> Maybe "XFSB" is only a remnant from one of this disk / partition former
>>> lifes.  Maybe it's a valid XFS but user no longer cares about it.  Or
>>> maybe a DOS-style label was created on top of it, without overwriting the 
>>> first
>>> 440 bytes.  Or maybe another filesystem had overwritten most XFS metadata
>>> but preserved the first block (this is conceivable since other filesystems
>>> tend to avoid using the first block).
>>>
>>> If user has to workaround GRUB heuristics by dd'ing zeros into a partition
>>> before running grub-install, this is a sign GRUB isn't doing the right 
>>> thing.
>>>
>>>   
>>>   
>> Well, ok. But then I would ask to use a separate --force e.g.
>> --force-destroy-xfs since users and distributions tend to use --force
>> too much
>> 
>
> Ok.
>
>   


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 

diff --git a/ChangeLog b/ChangeLog
index 960fc06..cc225a7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2009-10-16  Vladimir Serbinenko  
 
+	* util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock.
+	(options): New option --destroy-xfs.
+	(main): Handle --destroy-xfs.
+
+2009-10-16  Vladimir Serbinenko  
+
 	Let user specify OpenBSD root device.
 
 	* loader/i386/bsd.c (openbsd_root): New variable.
diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c
index ccfbd1d..4ef746b 100644
--- a/util/i386/pc/grub-setup.c
+++ b/util/i386/pc/grub-setup.c
@@ -86,7 +86,8 @@ grub_refresh (void)
 static void
 setup (const char *dir,
const char *boot_file, const char *core_file,
-   const char *root, const char *dest, int must_embed, int force)
+   const char *root, const char *dest, int must_embed, int force,
+   int destroy_xfs)
 {
   char *boot_path, *core_path, *core_path_dev;
   char *boot_img, *core_img;
@@ -251,6 +252,13 @@ setup (const char *dir,
   if (grub_disk_read (dest_dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, tmp_img))
 grub_util_error ("%s", grub_errmsg);
 
+  if (memcmp (tmp_img, "XFSB", 4) == 0)
+{
+  grub_util_warn ("This install would require destroying XFS.");
+  if (! destroy_xfs)
+	grub_util_error ("If you really want to destroy it use --destroy-xfs.");
+}
+
   /* Copy the possible DOS BPB.  */
   memcpy (boot_img + GRUB_BOOT_MACHINE_BPB_START,
 	  tmp_img + GRUB_BOOT_MACHINE_BPB_START,
@@ -556,6 +564,7 @@ static struct option options[] =
 {"device-map", required_argument, 0, 'm'},
 {"root-device", required_argument, 0, 'r'},
 {"force", no_argument, 0, 'f'},
+{"destroy-xfs", no_argument, 0, 'X'},
 {"help", no_argument, 0, 'h'},
 {"version", no_argument, 0, 'V'},
 {"verbose", no_argument, 0, 'v'},
@@ -580,6 +589,7 @@ DEVICE must be a GRUB device (e.g. ``(hd0,1)'').\n\
   -m, --device-map=FILE   use FILE as the device map [default=%s]\n\
   -r, --root-device=DEV   use DEV as the root device [default=guessed]\n\
   -f, --force install even if problems are detected\n\
+  --destroy-xfs   install even destroying XFS superblock\n\
   -h, --help  display this message and exit\n\
   -V, --version   print version information and exit\n\
   -v, --verbose   print verbose messages\n\
@@ -614,6 +624,7 @@ main (int argc, char *argv[])
   char *root_dev = 0;
   char *dest_dev;
   int must_embed = 0, force = 0;
+  int destroy_xfs = 0;
 
   progname = "grub-setup";
 
@@ -666,6 +677,10 @@ main (int argc, char *argv[])
 	force = 1;
 	

Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Robert Millan
On Fri, Oct 16, 2009 at 11:08:31PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Robert Millan wrote:
> > On Fri, Oct 16, 2009 at 10:09:41PM +0200, Vladimir 'phcoder' Serbinenko 
> > wrote:
> >   
> >> Robert Millan wrote:
> >> 
> >>> On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote:
> >>>   
> >>>   
>  On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko 
>  wrote:
>  
>  
> > +  if (memcmp (tmp_img, "XFSB", 4) == 0)
> > +grub_util_error ("Can't install on XFS.");
> >   
> >   
>  Can this error message give some more detail on what the problem is?
>  
>  
> >>> I suggest something like:
> >>>
> >>>   grub_util_warn ("Refusing to overwrite XFS meta-data.");
> >>>
> >>> This is more informative, and with grub_util_warn() user has an 
> >>> opportunity to
> >>> override it if she knows what she's doing.
> >>>
> >>>   
> >>>   
> >> Installing with blocklists/to partition is considered
> >> backward-compatibility feature. We never supported a config with XFS why
> >> we would want bw-compat for it?
> >> 
> >
> > Because we can't reliably tell if it's a config with XFS, only the user can.
> > This is an issue for both MBR or PBR installs.
> >
> > Maybe "XFSB" is only a remnant from one of this disk / partition former
> > lifes.  Maybe it's a valid XFS but user no longer cares about it.  Or
> > maybe a DOS-style label was created on top of it, without overwriting the 
> > first
> > 440 bytes.  Or maybe another filesystem had overwritten most XFS metadata
> > but preserved the first block (this is conceivable since other filesystems
> > tend to avoid using the first block).
> >
> > If user has to workaround GRUB heuristics by dd'ing zeros into a partition
> > before running grub-install, this is a sign GRUB isn't doing the right 
> > thing.
> >
> >   
> Well, ok. But then I would ask to use a separate --force e.g.
> --force-destroy-xfs since users and distributions tend to use --force
> too much

Ok.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Vladimir 'phcoder' Serbinenko
Robert Millan wrote:
> On Fri, Oct 16, 2009 at 10:09:41PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>   
>> Robert Millan wrote:
>> 
>>> On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote:
>>>   
>>>   
 On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko 
 wrote:
 
 
> +  if (memcmp (tmp_img, "XFSB", 4) == 0)
> +grub_util_error ("Can't install on XFS.");
>   
>   
 Can this error message give some more detail on what the problem is?
 
 
>>> I suggest something like:
>>>
>>>   grub_util_warn ("Refusing to overwrite XFS meta-data.");
>>>
>>> This is more informative, and with grub_util_warn() user has an opportunity 
>>> to
>>> override it if she knows what she's doing.
>>>
>>>   
>>>   
>> Installing with blocklists/to partition is considered
>> backward-compatibility feature. We never supported a config with XFS why
>> we would want bw-compat for it?
>> 
>
> Because we can't reliably tell if it's a config with XFS, only the user can.
> This is an issue for both MBR or PBR installs.
>
> Maybe "XFSB" is only a remnant from one of this disk / partition former
> lifes.  Maybe it's a valid XFS but user no longer cares about it.  Or
> maybe a DOS-style label was created on top of it, without overwriting the 
> first
> 440 bytes.  Or maybe another filesystem had overwritten most XFS metadata
> but preserved the first block (this is conceivable since other filesystems
> tend to avoid using the first block).
>
> If user has to workaround GRUB heuristics by dd'ing zeros into a partition
> before running grub-install, this is a sign GRUB isn't doing the right thing.
>
>   
Well, ok. But then I would ask to use a separate --force e.g.
--force-destroy-xfs since users and distributions tend to use --force
too much

-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 



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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Robert Millan
On Fri, Oct 16, 2009 at 10:09:41PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Robert Millan wrote:
> > On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote:
> >   
> >> On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko 
> >> wrote:
> >> 
> >>> +  if (memcmp (tmp_img, "XFSB", 4) == 0)
> >>> +grub_util_error ("Can't install on XFS.");
> >>>   
> >> Can this error message give some more detail on what the problem is?
> >> 
> >
> > I suggest something like:
> >
> >   grub_util_warn ("Refusing to overwrite XFS meta-data.");
> >
> > This is more informative, and with grub_util_warn() user has an opportunity 
> > to
> > override it if she knows what she's doing.
> >
> >   
> Installing with blocklists/to partition is considered
> backward-compatibility feature. We never supported a config with XFS why
> we would want bw-compat for it?

Because we can't reliably tell if it's a config with XFS, only the user can.
This is an issue for both MBR or PBR installs.

Maybe "XFSB" is only a remnant from one of this disk / partition former
lifes.  Maybe it's a valid XFS but user no longer cares about it.  Or
maybe a DOS-style label was created on top of it, without overwriting the first
440 bytes.  Or maybe another filesystem had overwritten most XFS metadata
but preserved the first block (this is conceivable since other filesystems
tend to avoid using the first block).

If user has to workaround GRUB heuristics by dd'ing zeros into a partition
before running grub-install, this is a sign GRUB isn't doing the right thing.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Vladimir 'phcoder' Serbinenko
Robert Millan wrote:
> On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote:
>   
>> On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko 
>> wrote:
>> 
>>> +  if (memcmp (tmp_img, "XFSB", 4) == 0)
>>> +grub_util_error ("Can't install on XFS.");
>>>   
>> Can this error message give some more detail on what the problem is?
>> 
>
> I suggest something like:
>
>   grub_util_warn ("Refusing to overwrite XFS meta-data.");
>
> This is more informative, and with grub_util_warn() user has an opportunity to
> override it if she knows what she's doing.
>
>   
Installing with blocklists/to partition is considered
backward-compatibility feature. We never supported a config with XFS why
we would want bw-compat for it?


-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 



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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Robert Millan
On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote:
> On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > +  if (memcmp (tmp_img, "XFSB", 4) == 0)
> > +grub_util_error ("Can't install on XFS.");
> 
> Can this error message give some more detail on what the problem is?

I suggest something like:

  grub_util_warn ("Refusing to overwrite XFS meta-data.");

This is more informative, and with grub_util_warn() user has an opportunity to
override it if she knows what she's doing.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Jordi Mallach
On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> +  if (memcmp (tmp_img, "XFSB", 4) == 0)
> +grub_util_error ("Can't install on XFS.");

Can this error message give some more detail on what the problem is?

Jordi
-- 
Jordi Mallach Pérez  --  Debian developer http://www.debian.org/
jo...@sindominio.net jo...@debian.org http://www.sindominio.net/
GnuPG public key information available at http://oskuro.net/


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


Re: [PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Vladimir 'phcoder' Serbinenko
Sorry, patch had a problem
Vladimir 'phcoder' Serbinenko wrote:

-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 

diff --git a/ChangeLog b/ChangeLog
index b0864a9..a67fdfd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2009-10-16  Vladimir Serbinenko  
+
+	* util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock.
+
 2009-10-15  Vladimir Serbinenko  
 
 	* loader/i386/pc/xnu.c (grub_xnu_set_video): Fix loading splash image.
diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c
index ccfbd1d..b3f2736 100644
--- a/util/i386/pc/grub-setup.c
+++ b/util/i386/pc/grub-setup.c
@@ -251,6 +251,9 @@ setup (const char *dir,
   if (grub_disk_read (dest_dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, tmp_img))
 grub_util_error ("%s", grub_errmsg);
 
+  if (memcmp (tmp_img, "XFSB", 4) == 0)
+grub_util_error ("Can't install on XFS.");
+
   /* Copy the possible DOS BPB.  */
   memcpy (boot_img + GRUB_BOOT_MACHINE_BPB_START,
 	  tmp_img + GRUB_BOOT_MACHINE_BPB_START,
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Refuse to install on XFS destroying its superblock

2009-10-16 Thread Vladimir 'phcoder' Serbinenko

-- 
Regards
Vladimir 'phcoder' Serbinenko
Personal git repository: http://repo.or.cz/w/grub2/phcoder.git 

diff --git a/ChangeLog b/ChangeLog
index b0864a9..a67fdfd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2009-10-16  Vladimir Serbinenko  
+
+	* util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock.
+
 2009-10-15  Vladimir Serbinenko  
 
 	* loader/i386/pc/xnu.c (grub_xnu_set_video): Fix loading splash image.
diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c
index ccfbd1d..5181e58 100644
--- a/util/i386/pc/grub-setup.c
+++ b/util/i386/pc/grub-setup.c
@@ -205,6 +205,9 @@ setup (const char *dir,
   boot_img = grub_util_read_image (boot_path);
   free (boot_path);
 
+  if (memcmp (boot_img, "XFSB", 4) == 0)
+grub_util_error ("Can't install on XFS.");
+
   /* Set the addresses of variables in the boot image.  */
   boot_drive = (grub_uint8_t *) (boot_img + GRUB_BOOT_MACHINE_BOOT_DRIVE);
   kernel_sector = (grub_disk_addr_t *) (boot_img
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel