On Fri, Jul 04, 2008 at 04:45:02PM +0200, Javier Martín wrote: > > By the way, I'm already using SVN (and thus svn diff) for this patch. Is > that right? Was the migration completed already?
Yep. > +/* Superblock filesystem feature flags (RW compatible) */ > +#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) */ > +#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) */ > +#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 /* Journal device */ > +#define EXT2_FEATURE_INCOMPAT_META_BG 0x0010 > +#define EXT4_FEATURE_INCOMPAT_EXTENTS 0x0040 /* extents > support */ > +#define EXT4_FEATURE_INCOMPAT_64BIT 0x0080 > +#define EXT4_FEATURE_INCOMPAT_FLEX_BG 0x0200 I'd suggest making the "RW compatible" etc notes a bit more ellaborate to make it clear what they mean (I'm confused myself). > +/* 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 ) I suppose we'll want to have EXT4_FEATURE_INCOMPAT_EXTENTS here, now that it's been implemented (Bean just sent a patch, which will probably be merged first). > +/* The set of back-incompatible features this driver DOES NOT support but are > + * ignored for some hackish reason. Flags here should be here _temporarily_! > + * Remember that INCOMPAT_* features are so for a reason! */ > +#define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER ) Instead of this can we have an explanation of what EXT3_FEATURE_INCOMPAT_RECOVER is doing here? I think the reason was that our code for this feature wasn't as mature as it should be, and it appears that not handling it brings better results in the short term. Maybe Pavel can ellaborate more, I think it was him who brought up this point. > + /* 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)) > + goto fail; > [...] > fail: > - grub_error (GRUB_ERR_BAD_FS, "not an ext2 filesystem"); > + grub_error (GRUB_ERR_BAD_FS, "not an ext2 filesystem, or incompatible" > + "features enabled"); 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. -- 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
