[Ocfs2-tools-devel] [PATCH 05/12] debugfs.ocfs2: Dump directory block trailers.

[Sunil Mushran]

Comments inlined.

Joel Becker wrote:
> Teach debugfs.ocfs2 to notice and dump directory block trailers via the
> new "dir_trailers" command.
>
> Signed-off-by: Joel Becker <joel.becker at oracle.com>
> ---
>  debugfs.ocfs2/commands.c     |   54 +++++++++++++++++++++++++++++++++++++++++-
>  debugfs.ocfs2/dump.c         |   21 ++++++++++++++++
>  debugfs.ocfs2/include/dump.h |    7 +++++
>  3 files changed, 81 insertions(+), 1 deletions(-)
>
> diff --git a/debugfs.ocfs2/commands.c b/debugfs.ocfs2/commands.c
> index 06b6c98..7aaa840 100644
> --- a/debugfs.ocfs2/commands.c
> +++ b/debugfs.ocfs2/commands.c
> @@ -73,6 +73,7 @@ static void do_bmap (char **args);
>  static void do_icheck (char **args);
>  static void do_dlm_locks (char **args);
>  static void do_controld(char **args);
> +static void do_dir_trailers(char **args);
>  
>  dbgfs_gbls gbls;
>  
> @@ -107,7 +108,8 @@ static Command commands[] = {
>  	{ "stat",	do_stat },
>  	{ "stats",	do_stats },
>  	{ "encode",	do_encode_lockres },
> -	{ "decode",	do_decode_lockres }
> +	{ "decode",	do_decode_lockres },
> +	{ "dir_trailers",	do_dir_trailers },
>   

Not a big issue but dir_trailers is too implementation specific.
How about "statd"? It is like "stats" for stat-ing the sb.

>  };
>  
>  /*
> @@ -833,6 +835,7 @@ static void do_help (char **args)
>  	printf ("decode <lockname#> ...\t\t\tDecode block#(s) from the lockname(s)\n");
>  	printf ("dlm_locks [-f <file>] [-l] lockname\t\t\tShow live dlm locking state\n");
>  	printf ("dump [-p] <filespec> <outfile>\t\tDumps file to outfile on a mounted fs\n");
> +	printf ("dir_trailers <filespec>\t\t\tDump directory block trailers\n");
>  	printf ("encode <filespec>\t\t\tShow lock name\n");
>  	printf ("extent <block#>\t\t\t\tShow extent block\n");
>  	printf ("findpath <block#>\t\t\tList one pathname of the inode/lockname\n");
> @@ -1237,6 +1240,55 @@ static void do_group (char **args)
>  }
>  
>  /*
> + * do_dir_trailers()
> + *
> + */
> +static void do_dir_trailers (char **args)
> +{
> +	struct ocfs2_dinode *inode;
> +	uint64_t ino_blkno;
> +	char *buf = NULL;
> +	FILE *out;
> +	errcode_t ret = 0;
> +	struct dir_trailer_walk ctxt;
> +
> +	if (process_inode_args(args, &ino_blkno))
> +		return;
> +
> +	out = open_pager(gbls.interactive);
> +
> +	ctxt.fs = gbls.fs;
> +	ctxt.out = out;
> +
> +	buf = gbls.blockbuf;
> +	ret = ocfs2_read_inode(gbls.fs, ino_blkno, buf);
> +	if (ret) {
> +		com_err(args[0], ret, "while reading inode %"PRIu64"",
> +			ino_blkno);
> +		close_pager (out);
> +		return ;
> +	}
> +
> +	inode = (struct ocfs2_dinode *)buf;
> +	if (!ocfs2_dir_has_trailer(gbls.fs, inode)) {
> +		fprintf(out, "Inode %"PRIu64" has no trailers\n", ino_blkno);
> +		close_pager (out);
> +		return;
> +	}
> +
> +	ret = ocfs2_dir_iterate(gbls.fs, ino_blkno,
> +				OCFS2_DIRENT_FLAG_INCLUDE_TRAILER, NULL,
> +				dump_dir_trailers, (void *)&ctxt);
> +	if (ret)
> +		com_err(args[0], ret, "while iterating directory at "
> +			"block %"PRIu64"", ino_blkno);
> +
> +	close_pager(out);
> +
> +	return;
> +}
>   

Why not open_pager() just before ocfs2_dir_iterate?

> +
> +/*
>   * do_extent()
>   *
>   */
> diff --git a/debugfs.ocfs2/dump.c b/debugfs.ocfs2/dump.c
> index f68952d..2bd2032 100644
> --- a/debugfs.ocfs2/dump.c
> +++ b/debugfs.ocfs2/dump.c
> @@ -449,6 +449,27 @@ int  dump_dir_entry (struct ocfs2_dir_entry *rec, int offset, int blocksize,
>  }
>  
>  /*
> + * dump_dir_entry()
> + *
> + */
> +int dump_dir_trailers(struct ocfs2_dir_entry *rec, int offset, int blocksize,
> +		      char *buf, void *priv_data)
> +{
> +	struct dir_trailer_walk *ctxt = priv_data;
> +	struct ocfs2_dir_block_trailer *trailer =
> +		(struct ocfs2_dir_block_trailer *)rec;
> +
> +	if (ocfs2_dir_trailer_blk_off(ctxt->fs) != offset)
> +		return 0;
> +
> +	fprintf(ctxt->out,
> +                "\tDirblock: %-15"PRIu64" Inode: %-15"PRIu64" rec_len: %-4u\n",
> +                trailer->db_blkno, trailer->db_parent_dinode,
> +                trailer->db_compat_rec_len);
>   

Bike shedding maybe but "Reclen" is probably more in debugfs style.

> +	return 0;
> +}
> +
> +/*
>   * dump_jbd_header()
>   *
>   */
> diff --git a/debugfs.ocfs2/include/dump.h b/debugfs.ocfs2/include/dump.h
> index 525f6b9..f300559 100644
> --- a/debugfs.ocfs2/include/dump.h
> +++ b/debugfs.ocfs2/include/dump.h
> @@ -33,6 +33,11 @@ typedef struct _list_dir_opts {
>  	char *buf;
>  } list_dir_opts;
>  
> +struct dir_trailer_walk {
> +	ocfs2_filesys *fs;
> +	FILE *out;
> +};
> +
>  void dump_super_block (FILE *out, struct ocfs2_super_block *sb);
>  void dump_local_alloc (FILE *out, struct ocfs2_local_alloc *loc);
>  void dump_truncate_log (FILE *out, struct ocfs2_truncate_log *tl);
> @@ -43,6 +48,8 @@ void dump_extent_block (FILE *out, struct ocfs2_extent_block *blk);
>  void dump_group_descriptor (FILE *out, struct ocfs2_group_desc *grp, int index);
>  int  dump_dir_entry (struct ocfs2_dir_entry *rec, int offset, int blocksize,
>  		     char *buf, void *priv_data);
> +int dump_dir_trailers(struct ocfs2_dir_entry *rec, int offset, int blocksize,
> +		      char *buf, void *priv_data);
>  void dump_jbd_header (FILE *out, journal_header_t *header);
>  void dump_jbd_superblock (FILE *out, journal_superblock_t *jsb);
>  void dump_jbd_block (FILE *out, journal_superblock_t *jsb,
>