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

[Sunil Mushran]

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;
}

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.

>  	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;
>  }
>