[Ocfs2-tools-devel] [PATCH 1/1] Add corrupt code for inline data in fswreck

tristan tristan.ye at oracle.com
Mon Aug 11 21:25:16 PDT 2008


On Mon, 2008-08-11 at 22:10 +0800, Tao Ma wrote:
> Hi tristan,
> 	Thanks for your work on this.
> 
> First, please read http://lxr.linux.no/linux/Documentation/CodingStyle. 
> It explains the policy for writing Linux kernel source code. Although 
> ocfs2-tools isn't a kernel project, I think we should follow the same 
> rule. Actually there is a good tool which will help you find the coding 
> style problem, and you can find in linux-kernel-src/scripts/checkpatch.pl.

Thanks,tao.

As you said,there were many problems with my original patch about coding
style,will pay more attention to comply with such policies to keep the
unification of srcs.

I'll email the new patches soon for your review.
> 
> Some other comments are below.
> tristan wrote:
> >  Add corrupt code for inline data in fswreck,include both regular file
> > and dir.
> >  Mainly aims at corrupting the newly added checking codes in
> > fsck.ocfs2.checks for inline-data.
> >  Signed-off-by: Tristan Ye <tristan.ye at oracle.com>
> > 
> > ---
> >  fswreck/corrupt.c           |   13 +++++
> >  fswreck/dir.c               |  110
> > +++++++++++++++++++++++++++++++++++++++++++
> >  fswreck/include/dir.h       |    2 +
> >  fswreck/include/fsck_type.h |   12 ++++-
> >  fswreck/include/inode.h     |    2 +
> >  fswreck/include/main.h      |    5 ++-
> >  fswreck/inode.c             |  105
> > +++++++++++++++++++++++++++++++++++++++++
> >  fswreck/main.c              |    4 ++
> >  8 files changed, 251 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fswreck/corrupt.c b/fswreck/corrupt.c
> > index f698cb0..188b767 100644
> > --- a/fswreck/corrupt.c
> > +++ b/fswreck/corrupt.c
> > @@ -147,6 +147,19 @@ void corrupt_file(ocfs2_filesys *fs, int code,
> > uint16_t slotnum)
> >  	case CORRUPT_DIR_NOT_CONNECTED:
> >  		func = mess_up_dir_not_connected;
> >  		break;
> > +	case CORRUPT_REGULAR_INLINE_DATA_FLAG:
> > +		func = mess_up_regular_inline_flag;
> > +		break;
> > +	case CORRUPT_REGULAR_INLINE_DATA_SIZE:
> > +		func = mess_up_regular_inline_size;
> > +		break;
> > +	case CORRUPT_DIR_INLINE_DATA_FLAG:
> > +		func = mess_up_dir_inline_flag;
> > +		break;
> > +	case CORRUPT_DIR_INLINE_DATA_SIZE:
> > +		func = mess_up_dir_inline_size;
> > +		break;
> > +
> >  	default:
> >  		FSWRK_FATAL("Invalid code=%d", code);
> >  	}
> > diff --git a/fswreck/dir.c b/fswreck/dir.c
> > index 3338a17..34fdae9 100644
> > --- a/fswreck/dir.c
> > +++ b/fswreck/dir.c
> > @@ -452,3 +452,113 @@ void mess_up_dir_not_connected(ocfs2_filesys *fs,
> > uint64_t blkno)
> >  
> >  	return;
> >  }
> > +
> > +void create_inline_dir(ocfs2_filesys *fs,const char *dirname,
> > +		       uint64_t blkno, uint64_t *retblkno)
> > +{
> > +	errcode_t ret;
> > +	uint64_t tmp_blkno;
> > +
> > +	ret = ocfs2_check_directory(fs,blkno);
> > +	if (ret)
> > +                FSWRK_COM_FATAL(progname, ret);
> > +	
> > +	ret  = ocfs2_new_inode(fs, &tmp_blkno, S_IFDIR | 0755);
> > +	if (ret)
> > +		FSWRK_COM_FATAL(progname, ret);
> > +
> > +	ret = ocfs2_init_dir(fs, tmp_blkno, blkno);
> > +        if (ret)
> > +                FSWRK_COM_FATAL(progname, ret);
> > +
> > +	ret = ocfs2_link(fs, blkno, dirname, tmp_blkno, OCFS2_FT_DIR);
> > +        if (ret)
> > +                FSWRK_COM_FATAL(progname, ret);
> > +
> > +	*retblkno = tmp_blkno;
> > +	
> > +}
> > +void mess_up_dir_inline_flag(ocfs2_filesys *fs, uint64_t blkno)
> > +{
> > +	char inline_dirname[OCFS2_MAX_FILENAME_LEN];
> > +        char *buf = NULL;
> > +        uint64_t inline_blkno;
> > +        errcode_t ret;
> > +        struct ocfs2_dinode *sb_di = fs->fs_super;
> > +        struct ocfs2_dinode *di;
> > +
> > +        /*mess up inline flag in inode*/
> > +        snprintf(inline_dirname,OCFS2_MAX_FILENAME_LEN,
> > +                 "inline_dir_mess_flag_%d", getpid());
> Are you sure getpid() is ok? normally we corrupt a file with only one 
> corruption type, but you use getpid() in mess_up_dir_inline_flag, 
> mess_up_dir_inline_size, etc. So you'd better choose another file name.
Yes,i'll be fine to use a random filename if the policy of fswreck
asked us to do like that.
> > +        create_inline_dir(fs,inline_dirname,blkno,&inline_blkno);
> > +
> > +        ret = ocfs2_malloc_block(fs->fs_io, &buf);
> > +        if (ret)
> > +                FSWRK_COM_FATAL(progname, ret);
> > +
> > +        ret = ocfs2_read_inode(fs, inline_blkno, buf);
> > +        if (ret)
> > +                FSWRK_COM_FATAL(progname, ret);
> > +
> > +        di = (struct ocfs2_dinode *)buf;
> > +
> > +        if (di->i_dyn_features & OCFS2_INLINE_DATA_FL) {
> > +                goto bail;
> > +        }
> > +        else {
> > +                di->i_dyn_features |= OCFS2_INLINE_DATA_FL;
> > +                ret = ocfs2_write_inode(fs,inline_blkno,buf);
> > +                if (ret)
> > +                        FSWRK_COM_FATAL(progname, ret);
> > +        }
> What do you want to do here? I suppose that you are now trying to 
> corrupt a volume which don't have INLINE_DATA support? If the inode 
> already has inline-data enabled, you just go out with no corruption.
> btw, you don't need to "goto bail" since "bail" is right after your "if".
> > +
> > +bail:
> > +        if (buf)
> > +                ocfs2_free(&buf);
> > +
> > +        return;
> > +
> > +}
> > +void mess_up_dir_inline_size(ocfs2_filesys *fs, uint64_t blkno)
> > +{
> > +	char inline_dirname[OCFS2_MAX_FILENAME_LEN];
> > +        char *buf = NULL;
> > +        uint64_t inline_blkno;
> > +        errcode_t ret;
> > +        struct ocfs2_dinode *di;
> > +        uint16_t max_inline_sz;
> > +
> > +        /*mess up inline flag in inode*/
> > +        snprintf(inline_dirname,OCFS2_MAX_FILENAME_LEN,
> > +                 "inline_dir_mess_size_%d", getpid());
> > +        create_inline_dir(fs,inline_dirname,blkno,&inline_blkno);
> > +
> > +        ret = ocfs2_malloc_block(fs->fs_io, &buf);
> > +        if (ret)
> > +                FSWRK_COM_FATAL(progname, ret);
> > +
> > +        ret = ocfs2_read_inode(fs, inline_blkno, buf);
> > +        if (ret)
> > +                FSWRK_COM_FATAL(progname, ret);
> > +
> > +        di = (struct ocfs2_dinode *)buf;
> > +        max_inline_sz = ocfs2_max_inline_data(fs->fs_blocksize);
> > +
> > +        if (!(di->i_dyn_features & OCFS2_INLINE_DATA_FL)) {
> > +                di->i_dyn_features |= OCFS2_INLINE_DATA_FL;
> > +        }
> > +        /*mess up id_count,i_size and i_cluster in a inline inode*/
> > +        di->id2.i_data.id_count = 0;
> > +        di->i_size = max_inline_sz + 1;
> > +        di->i_clusters = 1;
> > +
> > +        ret = ocfs2_write_inode(fs,inline_blkno,buf);
> > +        if (ret)
> > +                FSWRK_COM_FATAL(progname, ret);
> > +
> > +        if (buf)
> > +                ocfs2_free(&buf);
> > +
> > +        return;
> > +
> > +}
> > diff --git a/fswreck/include/dir.h b/fswreck/include/dir.h
> > index 57afcfb..77e865c 100644
> > --- a/fswreck/include/dir.h
> > +++ b/fswreck/include/dir.h
> > @@ -30,5 +30,7 @@ void mess_up_dir_dot(ocfs2_filesys *fs, uint64_t
> > blkno);
> >  void mess_up_dir_ent(ocfs2_filesys *fs, uint64_t blkno);
> >  void mess_up_dir_parent_dup(ocfs2_filesys *fs, uint64_t blkno);
> >  void mess_up_dir_not_connected(ocfs2_filesys *fs, uint64_t blkno);
> > +void mess_up_dir_inline_flag(ocfs2_filesys *fs, uint64_t blkno);
> > +void mess_up_dir_inline_size(ocfs2_filesys *fs, uint64_t blkno);
> >  
> >  #endif		/* __DIR_H */
> > diff --git a/fswreck/include/fsck_type.h b/fswreck/include/fsck_type.h
> > index a6a4873..c443a92 100644
> > --- a/fswreck/include/fsck_type.h
> > +++ b/fswreck/include/fsck_type.h
> > @@ -112,7 +112,15 @@ enum fsck_type
> >  	INODE_NOT_CONNECTED,
> >  	INODE_COUNT,
> >  	INODE_ORPHANED,
> > -	NUM_FSCK_TYPE
> > +	NUM_FSCK_TYPE,
> > +	REGULAR_INLINE_DATA_FLAG,
> > +	REGULAR_INLINE_DATA_ID_COUNT,
> > +	REGULAR_INLINE_DATA_I_CLUSTER,
> > +	REGULAR_INLINE_DATA_I_SIZE,
> > +	DIR_INLINE_DATA_FLAG,
> > +	DIR_INLINE_DATA_ID_COUNT,
> > +	DIR_INLINE_DATA_I_CLUSTER,
> > +	DIR_INLINE_DATA_I_SIZE
> NUM_FSCK_TYPE, as its name shows, mean the fsck types number, so the 
> type should be added before it. And all the fsck types are defined in 
> fsck.ocfs2/fsck.ocfs2.checks.8.in, so you should not add others here.
That's really a negligence of my duty,thanks for correcting.
> 
> Your modification for normal file corruption is almost the same as dir 
> corruption, so it has the same problem.
> 
> Regards,
> Tao




More information about the Ocfs2-tools-devel mailing list