[Ocfs2-tools-devel] [PATCH 1/1] Ocfs2-tools: Ocfs2-tools: Let debugfs.ocfs2 correctly (r)dump the fast symlink v2.

[tristan.ye]

On Wed, 2009-06-03 at 10:38 -0700, Sunil Mushran wrote:
> Yes, Tao's suggestion was correct. But the code can be further
> optimized. See inlined comments.
> 
> Tristan Ye wrote:
> > Currently dump and rdump command in debugfs.ocfs2 didn't handle the
> > fast symlink correctly but a error encountered.
> >
> > With tao's suggestion, we directly dump the fast-symlink's destination from inode
> > to let (r)dump command in debugs.ocfs2 correctly handle fast symlink.
> >
> > Signed-off-by: Tristan Ye <tristan.ye at oracle.com>
> > ---
> >  debugfs.ocfs2/utils.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 45 insertions(+), 0 deletions(-)
> >
> > diff --git a/debugfs.ocfs2/utils.c b/debugfs.ocfs2/utils.c
> > index b1494fa..b1544d9 100644
> > --- a/debugfs.ocfs2/utils.c
> > +++ b/debugfs.ocfs2/utils.c
> > @@ -440,6 +440,25 @@ errcode_t dump_file(ocfs2_filesys *fs, uint64_t ino, int fd, char *out_file,
> >  		goto bail;
> >  	}
> >  
> > +	/* Dump the destination directly from inode if fast-symlinked*/
> > +	if (S_ISLNK(ci->ci_inode->i_mode) && !ci->ci_inode->i_clusters) {
> > +
> > +		buf = strdup((char *)ci->ci_inode->id2.i_symlink);
> > +
> > +		wrote = write(fd, buf, strlen(buf));
> > +
> > +		if (wrote != strlen(buf)) {
> > +			com_err(gbls.cmd, errno, "while writing file");
> > +			ret = errno;
> > +			goto bail;
> > +		}
> > +
> > +		if (preserve)
> > +			ret = fix_perms(ci->ci_inode, &fd, out_file);
> > +		
> > +		goto bail;
> > +	}
> > +
> >  	buflen = 1024 * 1024;
> >  
> >  	ret = ocfs2_malloc_blocks(fs->fs_io,
> > @@ -631,6 +650,30 @@ static errcode_t rdump_symlink(ocfs2_filesys *fs, uint64_t blkno, char *name)
> >  	uint32_t len = 0;
> >  	errcode_t ret;
> >  
> > +	char *ino_buf = NULL;
> > +	struct ocfs2_dinode *inode;
> > +
> > +	ret = ocfs2_malloc_block(fs->fs_io, &ino_buf);
> > +	if (ret) {
> > +		com_err(gbls.cmd, ret, "while allocating a block");
> > +		goto bail;
> > +	}
> > +
> > +	/* Get symlink's destination directly if fast-symlinked*/
> > +	ret = ocfs2_read_inode(fs, blkno, ino_buf);
> > +	if (ret) {
> > +		com_err(gbls.cmd, ret, "while reading inode %"PRIu64"", blkno);
> > +		goto bail;
> > +	}
> > +
> > +	inode = (struct ocfs2_dinode *)ino_buf;
> > +
> > +	if (S_ISLNK(inode->i_mode) && !inode->i_clusters) {
> > +		if (symlink((char *)inode->id2.i_symlink, name) == -1)
> > +			ret = errno;
> > +		goto bail;
> > +	}
> > +
> >   
> 
> How about we add struct ocfs2_dinode *di to the args. We won't
> have to read the inode twice. The code flow could go as:
> 
> char *link;
> 
> if (!di->i_clusters)
>   link = (char *)di->id2.i_symlink;
> else {
>   read_whole_file();
>   link = buf;

Yes, it would be better to avoid the second inode reading, thanks.

> }
> 
> symlink(link, name);
> 
> In dump_file(), just call rdump_symlink()... that is after 
> read_cached_inode().
> The current code does not look right. Hint: Use ci_inode.

Just a little bit confused,  why we have to call rdump_symlink() here,
the existing-code's logic is to dump the symlink's(for none-fast
symlinks) real content. so you mean the currrent logic of dump cmd for
none-fast symlinks was not correct?



> 
> >  	ret = read_whole_file(fs, blkno, &buf, &len);
> >  	if (ret)
> >  		goto bail;
> > @@ -641,6 +684,8 @@ static errcode_t rdump_symlink(ocfs2_filesys *fs, uint64_t blkno, char *name)
> >  bail:
> >  	if (buf)
> >  		ocfs2_free(&buf);
> > +	if (ino_buf)
> > +		ocfs2_free(&ino_buf);
> >  
> >  	return ret;
> >  }
> >   
>