Hi Ted,
The attached patch adds a command to debugfs to display all the
extents/indices contained in an inode or the block specified.
The options to the command includes tranversing and dumping the entire
extent tree (if inode specified) or the branch (in case of block) and
can display only the headers if required.
Would you please let me know your comments/suggestions ?
Thanks,
Girish
On Wed, Jun 11, 2008 at 03:41:28PM +0530, Girish Shilamkar wrote:
> Hi Ted,
> The attached patch adds a command to debugfs to display all the
> extents/indices contained in an inode or the block specified.
> The options to the command includes tranversing and dumping the entire
> extent tree (if inode specified) or the branch (in case of block) and
> can display only the headers if required.
>
> Would you please let me know your comments/suggestions ?
lib/ext2fs/tst_extents (I don't know why we don't install this by
default) have inode, root and ns/n command which help to iterate the
extent format inode
-aneesh
On Jun 11, 2008 16:13 +0530, Aneesh Kumar K.V wrote:
> On Wed, Jun 11, 2008 at 03:41:28PM +0530, Girish Shilamkar wrote:
> > The attached patch adds a command to debugfs to display all the
> > extents/indices contained in an inode or the block specified.
> > The options to the command includes tranversing and dumping the entire
> > extent tree (if inode specified) or the branch (in case of block) and
> > can display only the headers if required.
> >
> > Would you please let me know your comments/suggestions ?
>
> lib/ext2fs/tst_extents (I don't know why we don't install this by
> default) have inode, root and ns/n command which help to iterate the
> extent format inode
The "tst_*" programs are purely for regression testing. Having this
functionality built into a proper tool like debugfs is needed.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
On Thu, Jun 12, 2008 at 02:32:04AM -0600, Andreas Dilger wrote:
> >
> > lib/ext2fs/tst_extents (I don't know why we don't install this by
> > default) have inode, root and ns/n command which help to iterate the
> > extent format inode
Some of the reasons why tst_extents was created to extend debugfs,
instead of simply adding these commands to debugfs are:
1) To demonstrate the technique for creating programs for doing
per-module testing various new bits of the libext2 library, without
having to replicate a lot of infrastructure already provided by
debugfs. (Code reuse is good; lots of testing of *any* code before
you submit it, in either e2fsprogs or the ext4 kernel code is a very
good thing; one of the thing that disturbs me a little is when I trip
over bugs that indicate that obviously the code was obviously NOT
tested before submission, such as the on-line resizing code or patches
to the on-line resizing code, when online resizing was clearly and
obviously busted.)
2) The command names, "inode", "root", "next_sibling", etc. are
clearly inappropriate for debugfs.
> The "tst_*" programs are purely for regression testing. Having this
> functionality built into a proper tool like debugfs is needed.
Agreed. That being siad, Girish's patch doesn't apply against the
latest e2fsprogs git repository. It uses ext2fs_read_ext_block(), and
one of the reasons why I objected to Clusterfs's extent support in
e2fsprogs is that it exposed the low-level extent format to userspace
(and meant that the swapfs.c also needed extreme intimate knowledge of
the extent tree format), and that's a bad idea if we ever want to
change the extent format in the future. So the extents abstraction in
the latest e2fsprogs git tree does not have ext2fs_read_ext_block().
So to properly add the desired features to debugfs would require
taking some of the code from tst_extents (really, lib/ext2fs/extents.c
in the #ifdef DEBUG section), and moving it into debugfs.
I doubt we would want to move all of the tst_extents functionality
into debugfs, though. Probably just enough to dump the extent tree
and the set_bmap functionality --- and the latter should be done by
extending debugfs.c:do_bmap() so that it can take an optional 3rd
argument which utilizes ext2fs_bmap() to set a logical block mapping;
that way do_bmap() will work with normal and extent-based files, since
ext2fs_bmap/ext2fs_bmap2 have been wired to use Eric's
ext2fs_extent_set_bmap() function automatically for extents-based
files.
Regards,
- Ted