Re: [PATCH] ext4 extent support
From: Bean [EMAIL PROTECTED] Sent: Friday, July 04, 2008 7:59 PM To: The development of GRUB 2 grub-devel@gnu.org Subject: [PATCH] ext4 extent support Hi, This patch add support for extents in ext4. Thanks for the patch.I just tested it GRUB now works fine with 31 MB /boot ext4 with extent and flex_bg, though flex_bg shouldn't make a difference on that small partiton With uninit_bg enabled, which isn't by default enabled by e2fsprogs 1.41WIP, GRUB loaded but failed to load my Linux kernel with can't read linux header Once I had GRUB failing with file not found and then going to rescue mode but I think I did something wrong. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Endianness macros capitalization
Just my two (euro) cents: why are the endianness macros written like functions? I'm talking about the grub_Xe_to_cpuNN family, which look like function calls instead of the macros they are. Shouldn't they be capitalized to GRUB_LE_TO_CPU32 and such? signature.asc Description: Esta parte del mensaje está firmada digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] ext4 extent support
El sáb, 05-07-2008 a las 11:39 +0200, Felix Zielcke escribió: GRUB now works fine with 31 MB /boot ext4 with extent Wonderful!! Bean, you're awesome! and flex_bg, though flex_bg shouldn't make a difference on that small partiton I think flex_bg support is unimplemented right now (at least I didn't see it anywhere), but it worked because it's not being used? Just a guess signature.asc Description: Esta parte del mensaje está firmada digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Found two disks with the same number 0?!?
On my raid1-using system, I get the following error at boot:
error: Found two disks with the number 0?!?
Robert Millan suggested I apply a patch to print out the two disks with
this problem; they are (hd1,2) and (hd3,2).
If I comment out this check then I can boot normally. Robert things GRUB
is being too conservative here; perhaps this check should be removed?
Index: disk/raid.c
===
--- disk/raid.c (revision 1691)
+++ disk/raid.c (working copy)
@@ -440,18 +440,6 @@
return 0;
}
-
- if (array-device[sb.this_disk.number] != NULL)
- {
- /* We found multiple devices with the same number. Again,
-this shouldn't happen.*/
-
- grub_error (GRUB_ERR_BAD_NUMBER,
- Found two disks with the number %d?!?,
- sb.this_disk.number);
-
- return 0;
- }
}
/* Add an array to the list if we didn't find any. */
--
Sam Morris
http://robots.org.uk/
PGP key id 1024D/5EA01078
3412 EA18 1277 354B 991B C869 B219 7FDB 5EA0 1078
signature.asc
Description: This is a digitally signed message part
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
On Sat, Jul 05, 2008 at 10:46:56AM +0800, Bean wrote: If we move the option analyzer from normal.mod to kernel, then we can have one unified set of commands. How much space could this represent? About the duplicated commands, we can create a module minicmd to include the most basic command Then we can't have a rescue shell before heap is initialised and minicmd is loaded. Should we be concerned about this? -- Robert Millan GPLv2 I know my rights; I want my phone call! DRM What good is a phone call… if you are unable to speak? (as seen on /.) ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: grub-probe detects ext4 wronly as ext2
On Fri, Jul 04, 2008 at 10:41:35PM +0200, Javier Martín wrote:
Wonderful! I was sick of jumping through hoops with cvs diff.
I wasn't even using cvs diff! (you don't want to know what my replacement
dance was) ;-)
I'd suggest making the RW compatible etc notes a bit more ellaborate to
make
it clear what they mean (I'm confused myself).
Done, though now I might have over-elaborated
I think that's ok, comments don't eat space, so it's better to risk explaining
too much than being short of explaining everything.
Since we know which one applies, why not tell grub_error about it? We could
leave the not an ext2 filesystem call unmodified and add another one for
this particular error.
I may have overstepped a bit, but I've thought it more sensible to
replace all goto fail;s for calls to a new macro MOUNT_FAIL taking a
string argument which is saved in the new variable err_msg, and then
jumps to fail which shows _that_ message instead of the old one. Then, I
wrote informative messages for each error condition instead of just not
an ext2 filesystem.
Looks a bit ugly, but I don't have any objection if it makes code smaller
(by eliminating duped grub_error calls).
However, adding new strings is expensive, since they tend to take size more
easily than code. I would be careful about that.
grub_ext2_mount (grub_disk_t disk)
{
struct grub_ext2_data *data;
+ const char *err_msg = 0;
Is this const right? You're modifiing its value.
grub_disk_read (disk, 1 * 2, 0, sizeof (struct grub_ext2_sblock),
(char *) data-sblock);
if (grub_errno)
-goto fail;
+EXT2_DRIVER_MOUNT_FAIL(could not read the superblock)
This overrides the grub_errno and grub_errmsg provided by grub_disk_read and
replaces them with values that hide the true problem. If there was a disk
read error, we really want to know about it from the lower layer.
/* Make sure this is an ext2 filesystem. */
if (grub_le_to_cpu16 (data-sblock.magic) != EXT2_MAGIC)
-goto fail;
+EXT2_DRIVER_MOUNT_FAIL(not an ext2 filesystem (superblock magic
mismatch))
No need to ellaborate here; by definition, the magic number makes the
difference between a corrupt ext2 and something that is not ext2. So
you can just say it's not ext2.
+ /* Check the FS doesn't have feature bits enabled that we don't support. */
+ if (grub_le_to_cpu32 (data-sblock.feature_incompat)
+ ~(EXT2_DRIVER_SUPPORTED_INCOMPAT | EXT2_DRIVER_IGNORED_INCOMPAT))
+EXT2_DRIVER_MOUNT_FAIL(filesystem has unsupported incompatible
features)
Ok.
if (grub_errno)
-goto fail;
+EXT2_DRIVER_MOUNT_FAIL(could not read the root directory inode)
Ok.
--
Robert Millan
GPLv2 I know my rights; I want my phone call!
DRM What good is a phone call… if you are unable to speak?
(as seen on /.)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Drivemap module
Javier Martín [EMAIL PROTECTED] writes: Just an updated version of the patch that adds support for device-like names instead of raw BIOS disk numbers, i.e. this is now supported: grub drivemap (hd0) (hd1) In addition to the already supported: grub drivemap (hd0) 0x81 The effect is the same: the second BIOS hard drive will map to (hd0) through the installed int13h routine. The new syntax does not require the target device name to exist (hd1 need not exist in my example), and the parsing is very simple: it accepts names like (fdN) and (hdN) with and without parenthesis, and with N ranging from 0 to 127, thus allowing the full 0x00-0xFF range even though most BIOS-probing OSes don't bother going any further than fd7 and hd7 respectively. Great! Can you please send in a changelog entry? -- Marco ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] ext4 extent support
Hi Bean,
Bean [EMAIL PROTECTED] writes:
This patch add support for extents in ext4.
This is really great! :D
Can you also provide a changelog entry?
diff --git a/fs/ext2.c b/fs/ext2.c
index 22fd272..3518dcf 100644
--- a/fs/ext2.c
+++ b/fs/ext2.c
@@ -86,6 +86,8 @@
#define EXT3_JOURNAL_FLAG_DELETED4
#define EXT3_JOURNAL_FLAG_LAST_TAG 8
+#define EXT4_EXTENTS_FLAG0x8
+
/* The ext2 superblock. */
struct grub_ext2_sblock
{
@@ -226,6 +228,33 @@ struct grub_ext3_journal_sblock
grub_uint32_t start;
};
+#define EXT4_EXT_MAGIC 0xf30a
+
+struct grub_ext4_extent_header
+{
+ grub_uint16_t magic;
+ grub_uint16_t entries;
+ grub_uint16_t max;
+ grub_uint16_t depth;
+ grub_uint32_t generation;
+};
+
+struct grub_ext4_extent
+{
+ grub_uint32_t block;
+ grub_uint16_t len;
+ grub_uint16_t start_hi;
+ grub_uint32_t start;
+};
+
+struct grub_ext4_extent_idx
+{
+ grub_uint32_t block;
+ grub_uint32_t leaf;
+ grub_uint16_t leaf_hi;
+ grub_uint16_t unused;
+};
+
struct grub_fshelp_node
{
struct grub_ext2_data *data;
@@ -262,6 +291,46 @@ grub_ext2_blockgroup (struct grub_ext2_data *data, int
group,
sizeof (struct grub_ext2_block_group), (char *)
blkgrp);
}
+static struct grub_ext4_extent_header *
+grub_ext4_find_leaf (struct grub_ext2_data *data,
+ struct grub_ext4_extent_header *ext_block,
+ grub_uint32_t fileblock)
+{
+ char buf[EXT2_BLOCK_SIZE(data)];
+ struct grub_ext4_extent_idx *index;
+
+ while (1)
+{
+ int i;
+ grub_disk_addr_t block;
+
+ index = (struct grub_ext4_extent_idx *) (ext_block + 1);
+
+ if (grub_le_to_cpu16(ext_block-magic) != EXT4_EXT_MAGIC)
+return 0;
+
+ if (ext_block-depth == 0)
+return ext_block;
+
+ for (i = 0; i grub_le_to_cpu16 (ext_block-entries); i++)
+{
+ if (fileblock grub_le_to_cpu32(index[i].block))
+break;
+}
+
+ if (i == 0)
+return 0;
+
+ block = grub_le_to_cpu16 (index[i].leaf_hi);
+ block = (block 32) + grub_le_to_cpu32 (index[i].leaf);
+ if (grub_disk_read (data-disk,
+ block LOG2_EXT2_BLOCK_SIZE (data),
+ 0, sizeof (buf), buf))
+return 0;
+
+ ext_block = (struct grub_ext4_extent_header *) buf;
+}
+}
static grub_disk_addr_t
grub_ext2_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
@@ -272,6 +341,41 @@ grub_ext2_read_block (grub_fshelp_node_t node,
grub_disk_addr_t fileblock)
unsigned int blksz = EXT2_BLOCK_SIZE (data);
int log2_blksz = LOG2_EXT2_BLOCK_SIZE (data);
+ if (inode-flags EXT4_EXTENTS_FLAG)
+{
+ struct grub_ext4_extent_header *leaf;
+ struct grub_ext4_extent *ext;
+ int i;
+
+ leaf = grub_ext4_find_leaf (data,
+ (struct grub_ext4_extent_header *)
inode-blocks.dir_blocks,
+ fileblock);
+ if (! leaf)
+{
+ grub_error (GRUB_ERR_BAD_FS, invalid extent);
+ return -1;
+}
+
+ ext = (struct grub_ext4_extent *) (leaf + 1);
+ for (i = 0; i grub_le_to_cpu16 (leaf-entries); i++)
+if ((fileblock = grub_le_to_cpu32 (ext[i].block))
+(fileblock grub_le_to_cpu32 (ext[i].block) +
+ grub_le_to_cpu16 (ext[i].len))
+((grub_le_to_cpu16 (ext[i].len) 15) == 0))
+ {
+grub_disk_addr_t start;
+
+start = grub_le_to_cpu16 (ext[i].start_hi);
+start = (start 32) + grub_le_to_cpu32 (ext[i].start);
+return fileblock - grub_le_to_cpu32 (ext[i].block) + start;
+ }
+
+
+
+ grub_printf (HH\n);
Whoops? ;-)
+ return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+ ext2fs doesn't support extents);
Why the error? I thought you have added extent support? Doesn't this
mean the extent has another error of some kind?
--
Marco
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
Robert Millan wrote: About the duplicated commands, we can create a module minicmd to include the most basic command Then we can't have a rescue shell before heap is initialised and minicmd is loaded. Should we be concerned about this? What would one use that rescue shell for? -- coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br. Tel.: +49 761 7668825 • Fax: +49 761 7664613 Email: [EMAIL PROTECTED] • http://www.coresystems.de/ Registergericht: Amtsgericht Freiburg • HRB 7656 Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866 ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] ext4 extent support
On Sat, Jul 5, 2008 at 7:01 PM, Marco Gerards [EMAIL PROTECTED] wrote:
+ grub_printf (HH\n);
Whoops? ;-)
+ return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+ ext2fs doesn't support extents);
Why the error? I thought you have added extent support? Doesn't this
mean the extent has another error of some kind?
Hi,
Oh, I forget to cleanup the code, here is the new patch.
2008-07-06 Bean [EMAIL PROTECTED]
* fs/ext2.c (EXT4_EXTENTS_FLAG): New macro.
(grub_ext4_extent_header): New structure.
(grub_ext4_extent): Likewise.
(grub_ext4_extent_idx): Likewise.
(grub_ext4_find_leaf): New function.
(grub_ext2_read_block): Handle extents.
--
Bean
diff --git a/fs/ext2.c b/fs/ext2.c
index 22fd272..80e6576 100644
--- a/fs/ext2.c
+++ b/fs/ext2.c
@@ -86,6 +86,8 @@
#define EXT3_JOURNAL_FLAG_DELETED 4
#define EXT3_JOURNAL_FLAG_LAST_TAG 8
+#define EXT4_EXTENTS_FLAG 0x8
+
/* The ext2 superblock. */
struct grub_ext2_sblock
{
@@ -226,6 +228,33 @@ struct grub_ext3_journal_sblock
grub_uint32_t start;
};
+#define EXT4_EXT_MAGIC 0xf30a
+
+struct grub_ext4_extent_header
+{
+ grub_uint16_t magic;
+ grub_uint16_t entries;
+ grub_uint16_t max;
+ grub_uint16_t depth;
+ grub_uint32_t generation;
+};
+
+struct grub_ext4_extent
+{
+ grub_uint32_t block;
+ grub_uint16_t len;
+ grub_uint16_t start_hi;
+ grub_uint32_t start;
+};
+
+struct grub_ext4_extent_idx
+{
+ grub_uint32_t block;
+ grub_uint32_t leaf;
+ grub_uint16_t leaf_hi;
+ grub_uint16_t unused;
+};
+
struct grub_fshelp_node
{
struct grub_ext2_data *data;
@@ -262,6 +291,46 @@ grub_ext2_blockgroup (struct grub_ext2_data *data, int group,
sizeof (struct grub_ext2_block_group), (char *) blkgrp);
}
+static struct grub_ext4_extent_header *
+grub_ext4_find_leaf (struct grub_ext2_data *data,
+ struct grub_ext4_extent_header *ext_block,
+ grub_uint32_t fileblock)
+{
+ char buf[EXT2_BLOCK_SIZE(data)];
+ struct grub_ext4_extent_idx *index;
+
+ while (1)
+{
+ int i;
+ grub_disk_addr_t block;
+
+ index = (struct grub_ext4_extent_idx *) (ext_block + 1);
+
+ if (grub_le_to_cpu16(ext_block-magic) != EXT4_EXT_MAGIC)
+return 0;
+
+ if (ext_block-depth == 0)
+return ext_block;
+
+ for (i = 0; i grub_le_to_cpu16 (ext_block-entries); i++)
+{
+ if (fileblock grub_le_to_cpu32(index[i].block))
+break;
+}
+
+ if (i == 0)
+return 0;
+
+ block = grub_le_to_cpu16 (index[i].leaf_hi);
+ block = (block 32) + grub_le_to_cpu32 (index[i].leaf);
+ if (grub_disk_read (data-disk,
+ block LOG2_EXT2_BLOCK_SIZE (data),
+ 0, sizeof (buf), buf))
+return 0;
+
+ ext_block = (struct grub_ext4_extent_header *) buf;
+}
+}
static grub_disk_addr_t
grub_ext2_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
@@ -272,6 +341,38 @@ grub_ext2_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
unsigned int blksz = EXT2_BLOCK_SIZE (data);
int log2_blksz = LOG2_EXT2_BLOCK_SIZE (data);
+ if (inode-flags EXT4_EXTENTS_FLAG)
+{
+ struct grub_ext4_extent_header *leaf;
+ struct grub_ext4_extent *ext;
+ int i;
+
+ leaf = grub_ext4_find_leaf (data,
+ (struct grub_ext4_extent_header *) inode-blocks.dir_blocks,
+ fileblock);
+ if (! leaf)
+{
+ grub_error (GRUB_ERR_BAD_FS, invalid extent);
+ return -1;
+}
+
+ ext = (struct grub_ext4_extent *) (leaf + 1);
+ for (i = 0; i grub_le_to_cpu16 (leaf-entries); i++)
+if ((fileblock = grub_le_to_cpu32 (ext[i].block))
+(fileblock grub_le_to_cpu32 (ext[i].block) +
+ grub_le_to_cpu16 (ext[i].len))
+((grub_le_to_cpu16 (ext[i].len) 15) == 0))
+ {
+grub_disk_addr_t start;
+
+start = grub_le_to_cpu16 (ext[i].start_hi);
+start = (start 32) + grub_le_to_cpu32 (ext[i].start);
+return fileblock - grub_le_to_cpu32 (ext[i].block) + start;
+ }
+
+ grub_error (GRUB_ERR_BAD_FS, something wrong with extent);
+ return -1;
+}
/* Direct blocks. */
if (fileblock INDIRECT_BLOCKS)
blknr = grub_le_to_cpu32 (inode-blocks.dir_blocks[fileblock]);
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
Stefan Reinauer wrote: Robert Millan wrote: About the duplicated commands, we can create a module minicmd to include the most basic command Then we can't have a rescue shell before heap is initialised and minicmd is loaded. Should we be concerned about this? What would one use that rescue shell for? Idea of the rescue shell is load other modules in case grub itself cannot find them. It provides thin layer of tools so user is able to find them. Personally I would like to keep this functionality in core.img. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
On Sat, Jul 5, 2008 at 8:15 PM, Robert Millan [EMAIL PROTECTED] wrote: On Sat, Jul 05, 2008 at 10:46:56AM +0800, Bean wrote: If we move the option analyzer from normal.mod to kernel, then we can have one unified set of commands. How much space could this represent? It won't take much, the code is basically inside normal/arg.c. About the duplicated commands, we can create a module minicmd to include the most basic command Then we can't have a rescue shell before heap is initialised and minicmd is loaded. Should we be concerned about this? The rescue shell is still there. It's part of the line by line command scanner. -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
On Sun, Jul 6, 2008 at 1:32 AM, Bean [EMAIL PROTECTED] wrote: About the duplicated commands, we can create a module minicmd to include the most basic command Then we can't have a rescue shell before heap is initialised and minicmd is loaded. Should we be concerned about this? The rescue shell is still there. It's part of the line by line command scanner. Hi, Here is another way to look at rescue/normal mode. rescue mode: basic console interface + line scanner normal mode: advanced console interface (history, completion) + script parser text menu interface + script parser -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
Vesa Jääskeläinen wrote: Stefan Reinauer wrote: Robert Millan wrote: About the duplicated commands, we can create a module minicmd to include the most basic command Then we can't have a rescue shell before heap is initialised and minicmd is loaded. Should we be concerned about this? What would one use that rescue shell for? Idea of the rescue shell is load other modules in case grub itself cannot find them. It provides thin layer of tools so user is able to find them. Personally I would like to keep this functionality in core.img. So, how is the rescue shell different that grub itself. Why would it find modules that grub itself does not find? Stefan -- coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br. Tel.: +49 761 7668825 • Fax: +49 761 7664613 Email: [EMAIL PROTECTED] • http://www.coresystems.de/ Registergericht: Amtsgericht Freiburg • HRB 7656 Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866 ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
Stefan Reinauer wrote: Vesa Jääskeläinen wrote: Idea of the rescue shell is load other modules in case grub itself cannot find them. It provides thin layer of tools so user is able to find them. Personally I would like to keep this functionality in core.img. So, how is the rescue shell different that grub itself. Why would it find modules that grub itself does not find? User can use ls and insmod commands to load those modules from disk. Most common problem with GRUB legacy is that it just prints GRUB on screen. This will kinda remove that problem as user still has a way to boot his system with some keypresses. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
Vesa Jääskeläinen wrote: Stefan Reinauer wrote: Vesa Jääskeläinen wrote: Idea of the rescue shell is load other modules in case grub itself cannot find them. It provides thin layer of tools so user is able to find them. Personally I would like to keep this functionality in core.img. So, how is the rescue shell different that grub itself. Why would it find modules that grub itself does not find? User can use ls and insmod commands to load those modules from disk. Most common problem with GRUB legacy is that it just prints GRUB on screen. This will kinda remove that problem as user still has a way to boot his system with some keypresses. So the rescue shell has filesystems and a shell? Is the advanced console interface so huge that it can't live with the shell and the filesystems in the rescue shell? -- coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br. Tel.: +49 761 7668825 • Fax: +49 761 7664613 Email: [EMAIL PROTECTED] • http://www.coresystems.de/ Registergericht: Amtsgericht Freiburg • HRB 7656 Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866 ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] ext4 extent support
El sáb, 05-07-2008 a las 19:15 +0200, Felix Zielcke escribió: From: JavierMartín [EMAIL PROTECTED] Sent: Saturday, July 05, 2008 3:25 PM To: The development of GRUB 2 grub-devel@gnu.org Subject: Re: [PATCH] ext4 extent support I think flex_bg support is unimplemented right now (at least I didn't see it anywhere), but it worked because it's not being used? Just a guess I just checked. Kernel 2.6.26-rc8 has a bit code with flex_bg e2fsprogs 1.41WIP from Debian experimantel has a bit more code with flex (not much with flex_bg more flexbg or just _flex) But I don't know much C so I don't understand the code :) Anyway I think the most important ext4 support are extents, because you can enable them by just remounting to ext4 as long as you don't use noextents mount option flex_bg can only be enabled by mkfs.ext4(dev) not afterwards with tune2fs but it's default enabled in mke2fs.conf for ext4(dev) uninit_bg isn't enabled by default and can be enabled afterwards with tune2fs but resize2fs isn't currently working with it I mean unimplemented in GRUB right now. IIrc, flex_bg relaxes some of the rules governing the location of the metadata block groups or something like that, so that they can be placed in arbitrary locations. Uninit_bg allows yet-unused block groups and inode tables to be initalized on first use, and saves a lot of time in mkfs (because it doesn't have to write all the inode tables) and fsck (because there's no need to check unused block groups at all). signature.asc Description: Esta parte del mensaje está firmada digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
Stefan Reinauer wrote: Vesa Jääskeläinen wrote: Stefan Reinauer wrote: Vesa Jääskeläinen wrote: Idea of the rescue shell is load other modules in case grub itself cannot find them. It provides thin layer of tools so user is able to find them. Personally I would like to keep this functionality in core.img. So, how is the rescue shell different that grub itself. Why would it find modules that grub itself does not find? User can use ls and insmod commands to load those modules from disk. Most common problem with GRUB legacy is that it just prints GRUB on screen. This will kinda remove that problem as user still has a way to boot his system with some keypresses. So the rescue shell has filesystems and a shell? Is the advanced console interface so huge that it can't live with the shell and the filesystems in the rescue shell? It has anything what core provides. If by this you get core smaller then I am all for it. If it makes it larger then I would propose to find free space from somewhere else. Core.img just have to be standalone application so user can do recovery if something gets wrong in installation or something else. I do not know how well grub scripting is integrated to normal mode so check that out first. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: grub-probe detects ext4 wronly as ext2
El sáb, 05-07-2008 a las 14:07 +0200, Robert Millan escribió:
However, adding new strings is expensive, since they tend to take size more
easily than code. I would be careful about that.
I checked the space requirements, and seemingly there was a bit of space
available in the .rodata zone, since all those strings only added less
than 20 bytes o_O (at least in i386-pc). Wrapping it up, the pre-post
delta including code and strings is 120 bytes in i386-pc.
grub_ext2_mount (grub_disk_t disk)
{
struct grub_ext2_data *data;
+ const char *err_msg = 0;
Is this const right? You're modifiing its value.
Yeah, err_msg is a pointer to (const char), thus the characters are
unmodifiable but the pointer can be reassigned. You can also have char
* const, which is a const pointer to char (I don't see much utility
to it); and finally const char * const, a const pointer to (const
char), which would be the constant, unreassignable string pointer.
AFAIK, since GCC 4.0 string literals are const char * by default and
are stored in .rodata, so if the variable was termed as just char *
we'd have needed a cast. However, if the compiler considers string
literals as char *, no cast is needed to make it const char *.
grub_disk_read (disk, 1 * 2, 0, sizeof (struct grub_ext2_sblock),
(char *) data-sblock);
if (grub_errno)
-goto fail;
+EXT2_DRIVER_MOUNT_FAIL(could not read the superblock)
This overrides the grub_errno and grub_errmsg provided by grub_disk_read and
replaces them with values that hide the true problem. If there was a disk
read error, we really want to know about it from the lower layer.
Well, the old version did just the same (even worse, because the message
was generic). What would be the correct path of action here? I mean, how
can we propagate the error messages?
/* Make sure this is an ext2 filesystem. */
if (grub_le_to_cpu16 (data-sblock.magic) != EXT2_MAGIC)
-goto fail;
+EXT2_DRIVER_MOUNT_FAIL(not an ext2 filesystem (superblock magic
mismatch))
No need to ellaborate here; by definition, the magic number makes the
difference between a corrupt ext2 and something that is not ext2. So
you can just say it's not ext2.
Ok, I'm sending a new version of the thing with that part removed. I'm
trying to think of a ChangeLog entry for this. What about this?
fs/ext2.c: extN driver will reject filesystems with unimplemented
incompatible features enabled in the superblock
Index: fs/ext2.c
===
--- fs/ext2.c (revisión: 1691)
+++ fs/ext2.c (copia de trabajo)
@@ -71,8 +71,53 @@
? EXT2_GOOD_OLD_INODE_SIZE \
: grub_le_to_cpu16 (data-sblock.inode_size))
-#define EXT3_FEATURE_COMPAT_HAS_JOURNAL 0x0004
+/* Superblock filesystem feature flags (RW compatible)
+ * A filesystem with any of these enabled can be read and written by a driver
+ * that does not understand them without causing metadata/data corruption */
+#define EXT2_FEATURE_COMPAT_DIR_PREALLOC 0x0001
+#define EXT2_FEATURE_COMPAT_IMAGIC_INODES 0x0002
+#define EXT3_FEATURE_COMPAT_HAS_JOURNAL 0x0004
+#define EXT2_FEATURE_COMPAT_EXT_ATTR 0x0008
+#define EXT2_FEATURE_COMPAT_RESIZE_INODE 0x0010
+#define EXT2_FEATURE_COMPAT_DIR_INDEX 0x0020
+/* Superblock filesystem feature flags (RO compatible)
+ * A filesystem with any of these enabled can be safely read by a driver that
+ * does not understand them, but should not be written to, usually because
+ * additional metadata is required */
+#define EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER 0x0001
+#define EXT2_FEATURE_RO_COMPAT_LARGE_FILE 0x0002
+#define EXT2_FEATURE_RO_COMPAT_BTREE_DIR 0x0004
+#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM 0x0010
+#define EXT4_FEATURE_RO_COMPAT_DIR_NLINK 0x0020
+#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE 0x0040
+/* Superblock filesystem feature flags (back-incompatible)
+ * A filesystem with any of these enabled should not be attempted to be read
+ * by a driver that does not understand them, since they usually indicate
+ * metadata format changes that might confuse the reader. */
+#define EXT2_FEATURE_INCOMPAT_COMPRESSION 0x0001
+#define EXT2_FEATURE_INCOMPAT_FILETYPE 0x0002
+#define EXT3_FEATURE_INCOMPAT_RECOVER 0x0004 /* Needs recovery */
+#define EXT3_FEATURE_INCOMPAT_JOURNAL_DEV 0x0008 /* Volume is journal device */
+#define EXT2_FEATURE_INCOMPAT_META_BG 0x0010
+#define EXT4_FEATURE_INCOMPAT_EXTENTS 0x0040 /* Extents used */
+#define EXT4_FEATURE_INCOMPAT_64BIT 0x0080
+#define EXT4_FEATURE_INCOMPAT_FLEX_BG 0x0200
+/* The set of back-incompatible features this driver DOES support. Add (OR)
+ * flags here as the related features are implemented into the driver */
+#define EXT2_DRIVER_SUPPORTED_INCOMPAT ( EXT2_FEATURE_INCOMPAT_FILETYPE \
+ | EXT4_FEATURE_INCOMPAT_EXTENTS )
+/* List of rationales for the ignored incompatible features:
+ * needs_recovery: Not really
Re: Idea: elimination of the normal mode
On Sun, Jul 6, 2008 at 2:10 AM, Vesa Jääskeläinen [EMAIL PROTECTED] wrote: It has anything what core provides. If by this you get core smaller then I am all for it. If it makes it larger then I would propose to find free space from somewhere else. Core.img just have to be standalone application so user can do recovery if something gets wrong in installation or something else. I do not know how well grub scripting is integrated to normal mode so check that out first. Hi, With my suggestion, core.img would contain: minicmd + basic console interface + line scanner + fs modules It would be slightly bigger because we also add the option analyzer. But it may also reduce size due to better function separation. the line scanner would read a config file, which instruct extra modules to be loaded For example, to archive the same function of normal mode, we need: advanced console interface + text menu interface + script engine. Some advantage of this scheme: 1, One set of command. Now, if we want to use command like search, we must also include normal.mod, whose purpose is just to provide the option analyzer. 2, The separation of interface and script engine. Interface deal with interaction with user, like draw the menu, choose an menu item, edit a string, etc. Script engine is responsible for the interpretation of commands. These two parts are independent, we shouldn't bundle them into a single normal.mod.This would make modules like graphic interface more difficult to implement. -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode
Bean wrote: On Sun, Jul 6, 2008 at 2:10 AM, Vesa Jääskeläinen [EMAIL PROTECTED] wrote: It has anything what core provides. If by this you get core smaller then I am all for it. If it makes it larger then I would propose to find free space from somewhere else. Core.img just have to be standalone application so user can do recovery if something gets wrong in installation or something else. I do not know how well grub scripting is integrated to normal mode so check that out first. With my suggestion, core.img would contain: minicmd + basic console interface + line scanner + fs modules It would be slightly bigger because we also add the option analyzer. But it may also reduce size due to better function separation. It total size is smaller I do not care. If not, then I would pass it. I still like the idea to separate rescue mode from normal shell mode. This gives user an idea that grub has failed to load more of itself. I think Robert has from time to time complained that core.img gets too big sometimes to be embedded especially on raid configurations. So increasing size with this does not sound good on my ears. the line scanner would read a config file, which instruct extra modules to be loaded For example, to archive the same function of normal mode, we need: advanced console interface + text menu interface + script engine. Some advantage of this scheme: 1, One set of command. Now, if we want to use command like search, we must also include normal.mod, whose purpose is just to provide the option analyzer. Rescue is supposed to be minimal anyway. 2, The separation of interface and script engine. Interface deal with interaction with user, like draw the menu, choose an menu item, edit a string, etc. Script engine is responsible for the interpretation of commands. These two parts are independent, we shouldn't bundle them into a single normal.mod.This would make modules like graphic interface more difficult to implement. Well interaction between textual menu, graphical menu and shell interface will be studied anyway by Colin during his GSoC activities so lets wait what we get from there before jumping too far. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Endianness macros capitalization
On Sat, 2008-07-05 at 15:27 +0200, Javier Martín wrote: Just my two (euro) cents: why are the endianness macros written like functions? I'm talking about the grub_Xe_to_cpuNN family, which look like function calls instead of the macros they are. Shouldn't they be capitalized to GRUB_LE_TO_CPU32 and such? They probably should be functions. We may want to sparse annotate GRUB one day, and then inline functions in the only way to go. I prefer capitalized names only for macros that cannot be functions or have non-trivial size effects. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Endianness macros capitalization
El sáb, 05-07-2008 a las 17:30 -0400, Pavel Roskin escribió:
They probably should be functions. We may want to sparse annotate GRUB
one day, and then inline functions in the only way to go.
Hmm... you mean changing this
#define grub_swap_bytes16(x)\
({ \
grub_uint16_t _x = (x); \
(grub_uint16_t) ((_x 8) | (_x 8)); \
})
...for this
inline grub_uint16_t grub_swap_bytes16(uint16_t x)
{
return (x 8) | (x 8);
}
and such? The pro is that we get rid of the ugly hack in the macro
version that ensures single evaluation, but a con is that we cannot
_force_ any random compiler to inline anything, so we might end up with
gross numbers of function calls.
By the way, what is sparse annotate?
signature.asc
Description: Esta parte del mensaje está firmada digitalmente
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] New x86_64 EFI patch
Bean wrote: Hi, Perhaps you can also try the binary version at: http://grub4dos.sourceforge.net/grub2/grub.efi.1 A friend of mine have tested in in 32-bit EFI firmware, there is no problem for him. It confuses me! I could boot it from refit as EFI. Then it claimed to be GRUB 0.97. The help looked slightly different that what I was familiar with (but I've never used grub1 so I don't know...); and `reboot` worked. I didn't test more yet.. should I? -Isaac ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
GRUB Loading kernel... message
I thought I remembered somewhere a discussion how the message GRUB Loading kernel is confusing, because it doesn't say what kernel it's loading, and grub loads lots of kernels (this message means that the kernel is a core part of GRUB, and the subject GRUB the message mentions is different: it's some early loading code.) There is an argument that kernel does not mean the same thing as Linux kernel, which is quite right: that's why the message is not at all incorrect, just highly ambiguous. It seems like it should be Loading GRUB kernel . Although, looking at the files, boot/i386/pc/boot.S outputs GRUB and boot/i386/pc/diskboot.S outputs Loading kernel, so the parts actually mean different things: maybe it's important that it prints GRUB first in case it never prints anything else? perhaps there are other possible code-paths? In any case, then maybe it should say GRUB Loading GRUB kernel, or, GRUB Loading itself :-) to preserve the usefulness and increase the clarity. (Albeit, someone who doesn't know how GRUB works might be confused by how it can be loading itself: it's actually one part of GRUB loading a bigger part of GRUB. But we'll never be able to properly explain that to a nontechnical person :-) (Am I missing something, since this is my first time looking at GRUB code?) -Isaac ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: GRUB Loading kernel... message
On Sat, 2008-07-05 at 19:24 -0400, Isaac Dupree wrote: . Although, looking at the files, boot/i386/pc/boot.S outputs GRUB and boot/i386/pc/diskboot.S outputs Loading kernel, so the parts actually mean different things: maybe it's important that it prints GRUB first in case it never prints anything else? perhaps there are other possible code-paths? In any case, then maybe it should say GRUB Loading GRUB kernel, or, GRUB Loading itself :-) Considering that diskboot.img is limited to 512 bytes, I'd rather go with loading without kernel. The result would be GRUB loading, which would look better than your proposals. The saved bytes could be used for some good purpose eventually. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: GRUB Loading kernel... message
Pavel Roskin wrote: On Sat, 2008-07-05 at 19:24 -0400, Isaac Dupree wrote: . Although, looking at the files, boot/i386/pc/boot.S outputs GRUB and boot/i386/pc/diskboot.S outputs Loading kernel, so the parts actually mean different things: maybe it's important that it prints GRUB first in case it never prints anything else? perhaps there are other possible code-paths? In any case, then maybe it should say GRUB Loading GRUB kernel, or, GRUB Loading itself :-) Considering that diskboot.img is limited to 512 bytes, I'd rather go with loading without kernel. The result would be GRUB loading, which would look better than your proposals. The saved bytes could be used for some good purpose eventually. I like that wording: concise and clear (although maybe so concise that it gets hard for a user having a problem, to Google for it). By the way, currently the Loading seems to have the first letter capitalized (I don't care either way) -Isaac ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] New x86_64 EFI patch
On Sun, Jul 6, 2008 at 7:02 AM, Isaac Dupree [EMAIL PROTECTED] wrote: Bean wrote: Hi, Perhaps you can also try the binary version at: http://grub4dos.sourceforge.net/grub2/grub.efi.1 A friend of mine have tested in in 32-bit EFI firmware, there is no problem for him. It confuses me! I could boot it from refit as EFI. Then it claimed to be GRUB 0.97. The help looked slightly different that what I was familiar with (but I've never used grub1 so I don't know...); and `reboot` worked. I didn't test more yet.. should I? Hi, That's the fedora efi loader, have you used the right file ? BTW, you need to rename it as grub.efi, refit can't find files with .1 suffix. -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] ext4 extent support
On Sun, Jul 6, 2008 at 2:09 AM, Javier Martín [EMAIL PROTECTED] wrote: El sáb, 05-07-2008 a las 19:15 +0200, Felix Zielcke escribió: From: JavierMartín [EMAIL PROTECTED] Sent: Saturday, July 05, 2008 3:25 PM To: The development of GRUB 2 grub-devel@gnu.org Subject: Re: [PATCH] ext4 extent support I think flex_bg support is unimplemented right now (at least I didn't see it anywhere), but it worked because it's not being used? Just a guess I just checked. Kernel 2.6.26-rc8 has a bit code with flex_bg e2fsprogs 1.41WIP from Debian experimantel has a bit more code with flex (not much with flex_bg more flexbg or just _flex) But I don't know much C so I don't understand the code :) Anyway I think the most important ext4 support are extents, because you can enable them by just remounting to ext4 as long as you don't use noextents mount option flex_bg can only be enabled by mkfs.ext4(dev) not afterwards with tune2fs but it's default enabled in mke2fs.conf for ext4(dev) uninit_bg isn't enabled by default and can be enabled afterwards with tune2fs but resize2fs isn't currently working with it I mean unimplemented in GRUB right now. IIrc, flex_bg relaxes some of the rules governing the location of the metadata block groups or something like that, so that they can be placed in arbitrary locations. Hi, I found the description of flex_bg: This feature relaxes check restrictions on where each block groups meta data is located within the storage media. This allows for the allocation of bitmaps or inode tables outside the block group boundaries in cases where bad blocks forces us to look for new blocks which the owning block group can not satisfy. This will also allow for new meta-data allocation schemes to improve performance and scalability. The reallocation only occurs when bad blocks force us to use another block group, which is quite rare. So we should silently ignore this flag. -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
