[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: grub-probe detects ext4 wronly as ext2

From: Javier Martín
Subject: Re: grub-probe detects ext4 wronly as ext2
Date: Fri, 04 Jul 2008 22:41:35 +0200

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 */
> 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! */
> Instead of this can we have an explanation of what 
> 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".

Attachment: ext2_incompat.patch.3
Description: Text Data

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]