do not require that device is a partition
PATCH 1/1 do not require that device is a partition
Hi!
Devices where the filesystem is in whole disk are common nowadays [1]; I don't think it's a good idea to require that a filesystem is in a partition in order to access it.
See attached patch. Any comments?
[1] for example, debian provides usb images of the debian-installer with a fat filesystem and no partitions:
http://people.debian.org/~joeyh/d-i/images/daily/hd-media/boot.img.gz
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call, if you are unable to speak? (as seen on /.)
2007-10-05 Robert Millan <[email protected]>
* normal/misc.c (grub_normal_print_device_info): Do not require that device is a partition.
normal/misc.c +1 -1
@@ -40,7 +40,7 @@ dev =grub_device_open (name);if (! dev)grub_printf ("Filesystem cannot be accessed" ); -else if (! dev->disk || ! dev->disk->has_partitions || dev->disk->partition) +else {char *label;grub_fs_t fs;
normal/misc.c line 43
Quoted 2 lines · Robert Millan
normal/misc.c +1
-1-else if (! dev->disk || ! dev->disk->has_partitions || dev->disk->partition) +else
Actually, the idea behind this check seems to be that if the device is a disk, we only want to probe for filesystems if it has no partitions.
The problem with this is that filesystems in general don't garantee us that the first 512 bytes aren't filled with information that, to the MBR partition table parser, is seen as garbage and results in undefined partition layout.
I think disk->has_partitions as a whole is flawed for this reason. Should we get rid of it?
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call, if you are unable to speak? (as seen on /.)
Quoted 7 lines · Robert Millan
normal/misc.c line 43
Quoted 2 lines · Robert Millan
normal/misc.c +1
-1-else if (! dev->disk || ! dev->disk->has_partitions || dev->disk->partition) +else Actually, the idea behind this check seems to be that if the device is a disk, we only want to probe for filesystems if it has no partitions.
Quoted 6 lines
The problem with this is that filesystems in general don't garantee us that the first 512 bytes aren't filled with information that, to the MBR partition table parser, is seen as garbage and results in undefined partition layout.
I think
disk->has_partitionsas a whole is flawed for this reason. Should we get rid of it?
Thinking again, from GRUB POV we can't really tell wether the partition table or the filesystem is right in this situation, so I would suggest to just not use this in this particular check, but leave has_partitions as is, since it has its purpose.
Any comments?
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call, if you are unable to speak? (as seen on /.)
New patch after IRC discussion with Marco. code + ChangeLog should be self-explanatory.
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call, if you are unable to speak? (as seen on /.)
2007-10-14 Robert Millan <[email protected]>
* normal/misc.c (grub_normal_print_device_info): Do not probe for filesystem when dev->disk is unset.
Do probe for filesystem even when dev->disk->has_partitions is set. In case a filesystem is found, always report it.
In case it isn't, if dev->disk->has_partitions is set, report that a partition table was found instead of reporting that no filesystem could be identified.
normal/misc.c +7 -2
@@ -40,7 +40,7 @@ grub_normal_print_device_info (const cha dev =grub_device_open (name);if (! dev)grub_printf ("Filesystem cannot be accessed" ); -else if (! dev->disk || ! dev->disk->has_partitions || dev->disk->partition) +else if (dev->disk) {char *label;grub_fs_t fs;
@@ -49,7 +49,12 @@ grub_normal_print_device_info (const cha/* Ignore all errors. */ grub_errno =0 ; -grub_printf ("Filesystem type %s" , fs ? fs->name :"unknown" ); +if (fs) +grub_printf ("Filesystem type %s" , fs->name); +else if (! dev->disk->has_partitions || dev->disk->partition) +grub_printf ("Unknown filesystem" ); +else +grub_printf ("Partition table" );if (fs && fs->label) {
Quoted 19 lines · Robert Millan
New patch after IRC discussion with Marco. code + ChangeLog should be self-explanatory.
Signature · 15 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call, if you are unable to speak? (as seen on /.) 2007-10-14 Robert Millan <[email protected]> * normal/misc.c (grub_normal_print_device_info): Do not probe for filesystem when dev->disk is unset. Do probe for filesystem even when dev->disk->has_partitions is set. In case a filesystem is found, always report it. In case it isn't, if dev->disk->has_partitions is set, report that a partition table was found instead of reporting that no filesystem could be identified.
"Do not probe for filesystem when dev->disk is unset."
This was already the case, right? So perhaps this line should be removed?
Feel free to commit it.
--
Marco
Quoted 4 lines · Marco Gerards
"Do not probe for filesystem when
dev->diskis unset."This was already the case, right? So perhaps this line should be removed?
No, that was another bug:
17:05 < nyu> we're probing for filesystems on NICs currently 17:06 < nyu> it's one of the entry conditions 17:06 < marco_g> That's wrong :)
Quoted 1 line
Feel free to commit it.
Done.
Signature · 5 lines
Robert Millan <GPLv2> I know my rights; I want my phone call! <DRM> What use is a phone call, if you are unable to speak? (as seen on /.)