[Ocfs2-tools-devel] [PATCH 3/3] mkfs.ocfs2: Add 64bit journal support.

[Mark Fasheh]

On Sun, Sep 07, 2008 at 02:21:40PM -0700, Joel Becker wrote:
> On Sun, Sep 07, 2008 at 01:53:02PM -0700, Mark Fasheh wrote:
> > On Fri, Sep 05, 2008 at 11:36:42PM -0700, Joel Becker wrote:
> > > The [no]block32 and [no]block64 journal options are added to mkfs -J.
> > > noblock32 and block64 will create 64bit-capable journals.
> > 
> > Obviously, this patch will have to be updated with any incompat bit we add
> > to the scheme.
> 
> 	Yup.  Any thoughts?  I was just thinking a INCOMPAT_JBD2.  The
> kernel only needs to know about the bit (jbd and jbd2 handle the journal
> superblock compat checks properly).  Our tools will start supporting
> INCOMPAT_JBD2 when they start rejecting unknown journal superblock bits.
> Sound good?

Hmm... Well actually, how about a compat bit instead? We're only worried
about stopping fsck, and since it's more strict than the kernel about
bits...


> > > +/* Returns true if we support these journal features */
> > > +static int ocfs2_journal_check_available_features(journal_superblock_t *jsb,
> > > +						  ocfs2_fs_options *features)
> > > +{
> > > +	if (!features->opt_compat && !features->opt_ro_compat &&
> > > +	    !features->opt_incompat)
> > > +		return 1;
> > > +
> > > +	if (jsb->s_header.h_blocktype != JBD2_SUPERBLOCK_V2)
> > > +		return 0;
> > 
> > Ahh, this is clearly JFS_SUPERBLOCK_V2 renamed for JBD2.
> 
> 	Yes,  _V1 didn't have compat bits, _V2 does.  I'm using the JBD2
> defines for consistency with the kernel (they're fully compatible).
>
> > So - I take it we've never written a V1 superblock on an Ocfs2 volume before?
> 
> 	Not that I know of.  If we have, this check fails as it should
> (V1 superblocks don't know any features).


Sounds good.



> 
> > > diff --git a/mkfs.ocfs2/mkfs.h b/mkfs.ocfs2/mkfs.h
> > > index b98b705..bd8ac45 100644
> > > --- a/mkfs.ocfs2/mkfs.h
> > > +++ b/mkfs.ocfs2/mkfs.h
> > > @@ -212,6 +212,7 @@ struct _State {
> > >  	unsigned int initial_slots;
> > >  
> > >  	uint64_t journal_size_in_bytes;
> > > +	int journal64;
> > >  
> > >  	char *vol_label;
> > >  	char *device_name;
> > 
> > Don't we need to update check_32bit_blocks()? The function altogether should be skipped
> > if we have 64 bit blocks turned on. Also, in case we need to check, and the
> > check fails, we can try updating the error text:
> 
> 	You're right.  I worked on setting the bit without checking
> where mkfs might validate the limit.  I was rushing out the quick first
> pass before the weekend. :-)

Heh, no problem. Anyway, it's a minor one-line update :)

	--Mark

--
Mark Fasheh