[Ocfs2-tools-devel] [PATCH 2/3] Ocfs2-tools: Implement new prompt codes of sparse file for fswreck.

[tristan.ye]

On Wed, 2009-07-01 at 15:22 +0800, Tao Ma wrote:
> Hi tristan,
> 
> Tristan Ye wrote:
> > Adding new corruptions of sparse file for fswreck after fsck.ocfs2
> > supported new separated prompt codes.
> > 
> > Signed-off-by: Tristan Ye <tristan.ye at oracle.com>
> > ---
> >  fswreck/corrupt.c           |    6 ++++++
> >  fswreck/include/fsck_type.h |    2 ++
> >  fswreck/inode.c             |   35 ++++++++++++++++++++++++++++++++---
> >  fswreck/main.c              |    4 ++++
> >  4 files changed, 44 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fswreck/corrupt.c b/fswreck/corrupt.c
> > index 151d308..a9e5188 100644
> > --- a/fswreck/corrupt.c
> > +++ b/fswreck/corrupt.c
> > @@ -162,9 +162,15 @@ void corrupt_file(ocfs2_filesys *fs, enum fsck_type type, uint16_t slotnum)
> >  	case INODE_SIZE:
> >  		func = mess_up_inode_field;
> >  		break;
> > +	case INODE_SPARSE_SIZE:
> > +		func = mess_up_inode_field;
> > +		break;
> >  	case INODE_CLUSTERS:
> >  		func = mess_up_inode_field;
> >  		break;
> > +	case INODE_SPARSE_CLUSTERS:
> > +		func = mess_up_inode_field;
> > +		break;
> >  	case INODE_COUNT:
> >  		func = mess_up_inode_field;
> >  		break;
> > diff --git a/fswreck/include/fsck_type.h b/fswreck/include/fsck_type.h
> > index 968b38a..c7b9170 100644
> > --- a/fswreck/include/fsck_type.h
> > +++ b/fswreck/include/fsck_type.h
> > @@ -91,7 +91,9 @@ enum fsck_type
> >  	LINK_BLOCKS,
> >  	DIR_ZERO,
> >  	INODE_SIZE,
> > +	INODE_SPARSE_SIZE,
> >  	INODE_CLUSTERS,
> > +	INODE_SPARSE_CLUSTERS,
> >  	LALLOC_REPAIR,
> >  	LALLOC_USED,
> >  	CLUSTER_ALLOC_BIT,
> > diff --git a/fswreck/inode.c b/fswreck/inode.c
> > index a66ff3f..4c28e4d 100644
> > --- a/fswreck/inode.c
> > +++ b/fswreck/inode.c
> > @@ -47,6 +47,23 @@ static void damage_inode(ocfs2_filesys *fs, uint64_t blkno,
> >  	errcode_t ret;
> >  	char *buf = NULL;
> >  	struct ocfs2_dinode *di;
> > +	struct ocfs2_dinode *sb_di;
> > +	char *sb_blk;
> > +
> > +	if ((type == INODE_SPARSE_SIZE) || (type == INODE_SPARSE_CLUSTERS)) {
> > +
> > +		sb_blk = (char *)fs->fs_super;
> > +		sb_di = (struct ocfs2_dinode *)sb_blk;
> > +		if (!(sb_di->id2.i_super.s_feature_incompat &
> > +			OCFS2_FEATURE_INCOMPAT_SPARSE_ALLOC)) {
> > +
> > +			sb_di->id2.i_super.s_feature_incompat |= OCFS2_FEATURE_INCOMPAT_SPARSE_ALLOC;
> > +			ret = ocfs2_write_inode(fs, OCFS2_SUPER_BLOCK_BLKNO,
> > +						sb_blk);
> > +			if (ret)
> > +				FSWRK_COM_FATAL(progname, ret);
> > +		}
> > +	}
> Here you change the feature bit in the super block. But this will affect 
> our fswreck codes which run the test with the same volume, right? I am 
> not sure whether it is OK for other codes.

Yes, I remembered the rule in fswreck when sb's feature flag didn't
support the operation, we simply let programe quit abnormally.

I'll change it accordingly.

> 
> And you also need to handle endian problem here.

What did you mean i should handle the endian problem here? I didn't find
any endian risk here. we expected to handle all endian problems in
libocfs2, right?

> >  
> >  	ret = ocfs2_malloc_block(fs->fs_io, &buf);
> >  	if (ret)
> > @@ -81,7 +98,7 @@ static void damage_inode(ocfs2_filesys *fs, uint64_t blkno,
> >  			"Corrupt inode#%"PRIu64", change i_blkno from %"PRIu64
> >  			" to %"PRIu64"\n",
> >  			blkno, di->i_blkno, (di->i_blkno + 10));
> > -		di->i_blkno += 100;
> > +		di->i_blkno += 10;
> >  		break;
> any reason why you chagne from 100 to 10?

Cause I saw fprintf() using '10', maybe I should change the 10 to 100 in
fprintf() :).

> >  	case INODE_NZ_DTIME:
> >  		fprintf(stdout, "INODE_NZ_DTIME: "
> > @@ -103,12 +120,25 @@ static void damage_inode(ocfs2_filesys *fs, uint64_t blkno,
> >  			 blkno, di->i_size, (di->i_size + 100));
> >  		di->i_size += 100;
> >  		break;
> > +	case INODE_SPARSE_SIZE:
> > +		fprintf(stdout, "INODE_SPARSE_SIZE: "
> > +			"Corrupt inode#%"PRIu64", change i_size"
> > +			" from %"PRIu64" to %"PRIu64"\n",
> > +			 blkno, di->i_size, (di->i_size + 100));
> > +		di->i_size += 100;
> > +		break;
> 100 may not be enough. in fsck, we check whether i_size is within the 
> last cluster for a sparse file. So say we have a file size = 3000 and 
> cs=4k, with 3100, we are still good. So I guess you'd better and 
> cluster_size here.

Cool, good catch.



> 
> Regards,
> Tao