On 9/14/15 2:18 PM, Brian Foster wrote:
> On Wed, Sep 09, 2015 at 02:34:02PM -0500, Eric Sandeen wrote:
>> verify_da_path and traverse_int_dablock are similar to
>> verify_dir2_path and traverse_int_dir2block, but one
>> difference is that the dir2 code reads using the
>> multibuffer capable da_read_buf() routine, whereas
>> the attr code doesn't need to, and just calls
>> libxfs_readbuf.
>>
>> The multibuffer code falls back just fine when the
>> geometry indicates that it's not needed, so use that
>> same code in the attribute routines, and remove
>> another dir2 / da difference. We make da_read_buf()
>> non-static to facilitate this.
>>
>> Finally, add a local *geo to these routines,
>> to make the code even more similar at this point.
>> The geometry will get passed in later in the series.
>>
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
>> ---
>> repair/attr_repair.c | 49 +++++++++++++++++++++++++++++++++----------------
>> repair/dir2.c | 22 +++++++++++++---------
>> repair/dir2.h | 8 ++++++++
>> 3 files changed, 54 insertions(+), 25 deletions(-)
>>
>> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
>> index aba0782..fe81df4 100644
>> --- a/repair/attr_repair.c
>> +++ b/repair/attr_repair.c
>> @@ -138,14 +138,20 @@ traverse_int_dablock(xfs_mount_t *mp,
>> xfs_dablk_t *rbno,
>> int whichfork)
>> {
>> + bmap_ext_t *bmp;
Well, yeah - I thought about that. The goal was to make it as close
as possible to the other code, so I decided against the struct foo's.
I could do a "get rid of typedefs" as the last patch to the new util
code if you like.
> struct bmap_ext
>
> (and several other places below)
>
>> xfs_dablk_t bno;
>> int i;
>> + int nex;
>> xfs_da_intnode_t *node;
>> + bmap_ext_t lbmp;
>> xfs_fsblock_t fsbno;
>> xfs_buf_t *bp;
>> + struct xfs_da_geometry *geo;
>> struct xfs_da_node_entry *btree;
>> struct xfs_da3_icnode_hdr nodehdr;
>>
>> + geo = mp->m_attr_geo;
>> +
>> /*
>> * traverse down left-side of tree until we hit the
>> * left-most leaf block setting up the btree cursor along
>> @@ -160,13 +166,16 @@ traverse_int_dablock(xfs_mount_t *mp,
>> /*
>> * read in each block along the way and set up cursor
>> */
>> - fsbno = blkmap_get(da_cursor->blkmap, bno);
>> + nex = blkmap_getn(da_cursor->blkmap, bno,
>> + geo->fsbcount, &bmp, &lbmp);
>>
>
> ...
> attr_repair.c: In function ‘process_attributes’:
> attr_repair.c:185:5: warning: ‘fsbno’ may be used uninitialized in this function [-Wmaybe-uninitialized]
hm, whoops. Well, it goes away in the long run, but I can still fix
that up.
Thanks,
-Eric
> do_warn(
> ...
>
> Brian
>
>> - if (fsbno == NULLFSBLOCK)
>> + if (nex == 0)
>> goto error_out;
>>
>> - bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
>> - XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
>> + bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
>> + if (bmp != &lbmp)
>> + free(bmp);
>> +
>> if (!bp) {
>> if (whichfork == XFS_DATA_FORK)
>> do_warn(
>> @@ -192,12 +201,10 @@ traverse_int_dablock(xfs_mount_t *mp,
>> goto error_out;
>> }
>>
>> - if (nodehdr.count > mp->m_attr_geo->node_ents) {
>> + if (nodehdr.count > geo->node_ents) {
>> do_warn(_("bad record count in inode %" PRIu64 ", "
>> "count = %d, max = %d\n"),
>> - da_cursor->ino,
>> - nodehdr.count,
>> - mp->m_attr_geo->node_ents);
>> + da_cursor->ino, nodehdr.count, geo->node_ents);
>> libxfs_putbuf(bp);
>> goto error_out;
>> }
>> @@ -492,9 +499,15 @@ verify_da_path(xfs_mount_t *mp,
>> int bad;
>> int entry;
>> int this_level = p_level + 1;
>> + bmap_ext_t *bmp;
>> + int nex;
>> + bmap_ext_t lbmp;
>> + struct xfs_da_geometry *geo;
>> struct xfs_da_node_entry *btree;
>> struct xfs_da3_icnode_hdr nodehdr;
>>
>> + geo = mp->m_attr_geo;
>> +
>> /*
>> * index is currently set to point to the entry that
>> * should be processed now in this level.
>> @@ -536,17 +549,21 @@ verify_da_path(xfs_mount_t *mp,
>> */
>> dabno = nodehdr.forw;
>> ASSERT(dabno != 0);
>> - fsbno = blkmap_get(cursor->blkmap, dabno);
>> -
>> - if (fsbno == NULLFSBLOCK) {
>> - do_warn(_("can't get map info for block %u "
>> - "of directory inode %" PRIu64 "\n"),
>> + nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
>> + &bmp, &lbmp);
>> + if (nex == 0) {
>> + do_warn(
>> +_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>> dabno, cursor->ino);
>> return(1);
>> }
>>
>> - bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
>> - XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
>> + fsbno = bmp[0].startblock;
>> +
>> + bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
>> + if (bmp != &lbmp)
>> + free(bmp);
>> +
>> if (!bp) {
>> do_warn(
>> _("can't read block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
>> @@ -577,7 +594,7 @@ verify_da_path(xfs_mount_t *mp,
>> dabno, fsbno, cursor->ino);
>> bad++;
>> }
>> - if (nodehdr.count > mp->m_attr_geo->node_ents) {
>> + if (nodehdr.count > geo->node_ents) {
>> do_warn(
>> _("entry count %d too large in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
>> nodehdr.count,
>> diff --git a/repair/dir2.c b/repair/dir2.c
>> index 54c49eb..44367c6 100644
>> --- a/repair/dir2.c
>> +++ b/repair/dir2.c
>> @@ -92,7 +92,7 @@ namecheck(char *name, int length)
>> * Multibuffer handling.
>> * V2 directory blocks can be noncontiguous, needing multiple buffers.
>> */
>> -static struct xfs_buf *
>> +struct xfs_buf *
>> da_read_buf(
>> xfs_mount_t *mp,
>> int nex,
>> @@ -143,9 +143,12 @@ traverse_int_dir2block(xfs_mount_t *mp,
>> int nex;
>> xfs_da_intnode_t *node;
>> bmap_ext_t lbmp;
>> + struct xfs_da_geometry *geo;
>> struct xfs_da_node_entry *btree;
>> struct xfs_da3_icnode_hdr nodehdr;
>>
>> + geo = mp->m_dir_geo;
>> +
>> /*
>> * traverse down left-side of tree until we hit the
>> * left-most leaf block setting up the btree cursor along
>> @@ -161,7 +164,7 @@ traverse_int_dir2block(xfs_mount_t *mp,
>> * read in each block along the way and set up cursor
>> */
>> nex = blkmap_getn(da_cursor->blkmap, bno,
>> - mp->m_dir_geo->fsbcount, &bmp, &lbmp);
>> + geo->fsbcount, &bmp, &lbmp);
>>
>> if (nex == 0)
>> goto error_out;
>> @@ -207,13 +210,11 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
>> goto error_out;
>> }
>> btree = M_DIROPS(mp)->node_tree_p(node);
>> - if (nodehdr.count > mp->m_dir_geo->node_ents) {
>> - libxfs_putbuf(bp);
>> + if (nodehdr.count > geo->node_ents) {
>> do_warn(
>> _("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
>> - da_cursor->ino,
>> - nodehdr.count,
>> - mp->m_dir_geo->node_ents);
>> + da_cursor->ino, nodehdr.count, geo->node_ents);
>> + libxfs_putbuf(bp);
>> goto error_out;
>> }
>> /*
>> @@ -488,9 +489,12 @@ verify_dir2_path(xfs_mount_t *mp,
>> bmap_ext_t *bmp;
>> int nex;
>> bmap_ext_t lbmp;
>> + struct xfs_da_geometry *geo;
>> struct xfs_da_node_entry *btree;
>> struct xfs_da3_icnode_hdr nodehdr;
>>
>> + geo = mp->m_dir_geo;
>> +
>> /*
>> * index is currently set to point to the entry that
>> * should be processed now in this level.
>> @@ -532,7 +536,7 @@ verify_dir2_path(xfs_mount_t *mp,
>> */
>> dabno = nodehdr.forw;
>> ASSERT(dabno != 0);
>> - nex = blkmap_getn(cursor->blkmap, dabno, mp->m_dir_geo->fsbcount,
>> + nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
>> &bmp, &lbmp);
>> if (nex == 0) {
>> do_warn(
>> @@ -574,7 +578,7 @@ _("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
>> dabno, cursor->ino);
>> bad++;
>> }
>> - if (nodehdr.count > mp->m_dir_geo->node_ents) {
>> + if (nodehdr.count > geo->node_ents) {
>> do_warn(
>> _("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
>> nodehdr.count,
>> diff --git a/repair/dir2.h b/repair/dir2.h
>> index 3cc1941..186633f 100644
>> --- a/repair/dir2.h
>> +++ b/repair/dir2.h
>> @@ -58,6 +58,14 @@ typedef struct dir2_bt_cursor {
>> struct blkmap *blkmap;
>> } dir2_bt_cursor_t;
>>
>> +#include "bmap.h" /* Goes away in later refactoring */
>> +struct xfs_buf *
>> +da_read_buf(
>> + xfs_mount_t *mp,
>> + int nex,
>> + bmap_ext_t *bmp,
>> + const struct xfs_buf_ops *ops);
>> +
>> int
>> process_dir2(
>> xfs_mount_t *mp,
>> --
>> 1.7.1
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@xxxxxxxxxxx
>> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
