Re: grub-probe detects ext4 wronly as ext2

[Javier Martín]

OK, I've addressed all your concerns and here is a new version of the
patch. With it, the delta-size of the compiled ext2.mod against a
completely unpatched one is just 148 bytes.

El vie, 04-07-2008 a las 20:57 +0200, Robert Millan escribió:
> 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.
Wonderful! I was sick of jumping through hoops with cvs diff.

> 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

> > +/* 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).
Done too and checked that ext4 filesystems w/o other incompatible
features like 64BIT are now recognized (though I did not check reading
since I haven't yet applied Bean's patch to my tree).

> > +/* 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.
Done - explained why RECOVER is ignored even though it's "incompatible"

> 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".

ext2_incompat.patch.3
Description: Text Data

signature.asc
Description: Esta parte del mensaje está firmada digitalmente