[Ocfs2-tools-devel] [PATCH] debugfs.ocfs2: Add 'frag' command

[Mark Fasheh]

On Thu, Aug 06, 2009 at 02:10:41PM -0700, Sunil Mushran wrote:
> Signed-off-by: Sunil Mushran <sunil.mushran at oracle.com>
> 
> Two small comments below.

Sweet, thanks for the review.


> >+static void do_frag(char **args)
> >+{
> >+	struct ocfs2_dinode *inode;
> >+	uint64_t blkno;
> >+	char *buf = NULL;
> >+	FILE *out;
> >+	errcode_t ret = 0;
> >+	uint32_t clusters;
> >+	uint32_t extents = 0;
> >+
> >+	if (process_inode_args(args, &blkno))
> >+		return;
> >+
> >+	buf = gbls.blockbuf;
> >+	ret = ocfs2_read_inode(gbls.fs, blkno, buf);
> >+	if (ret) {
> >+		com_err(args[0], ret, "while reading inode %"PRIu64"", 
> >blkno);
> >+		return ;
> >+	}
> >+
> >+	inode = (struct ocfs2_dinode *)buf;
> >+
> >+	out = open_pager(gbls.interactive);
> >+
> >+	clusters = inode->i_clusters;
> >+	if (!(inode->i_dyn_features & OCFS2_INLINE_DATA_FL))
> >+		ret = calc_num_extents(gbls.fs, &(inode->id2.i_list), 
> >&extents);
> >+
> >+	dump_frag(out, inode->i_blkno, clusters, extents);
> >+
> >+	if (ret)
> >+		com_err(args[0], ret, "while traversing inode at block "
> >+			"%"PRIu64, blkno);
> >  
> 
> Shouldn't it be:
> if (ret)
> com_err();
> else
> dump_frag();

Yeah, good catch.


> 
> >+
> >+	close_pager(out);
> >+
> >+	return ;
> >+}
> >diff --git a/debugfs.ocfs2/dump.c b/debugfs.ocfs2/dump.c
> >index 17f58e9..f69f8bd 100644
> >--- a/debugfs.ocfs2/dump.c
> >+++ b/debugfs.ocfs2/dump.c
> >@@ -1011,3 +1011,17 @@ uint32_t dump_xattr_ibody(FILE *out, ocfs2_filesys 
> >*fs,
> > 		return 0;
> > 	}
> > }
> >+
> >+void dump_frag(FILE *out, uint64_t ino, uint32_t clusters,
> >+	       uint32_t extents)
> >+{
> >+	float frag_level = 0;
> >+
> >+	if (clusters > 1 && extents) {
> >+		float e = extents, c = clusters;
> >+		frag_level = 100 * (e / c);
> >+	}
> >+
> >+	fprintf(out, "Inode: %"PRIu64"\t%% fragmented: %.2f\tclusters:"
> >+		" %u\textents: %u\n", ino, frag_level, clusters, extents);
> >  
> 
> How about adding "%" to the fragmented value?

That's what the double "%%"  is supposed to do. So the output looks like
this:

debugfs: Inode: 8919058  % fragmented: 83.02     clusters: 1431  extents: 1188
	--Mark

--
Mark Fasheh