* [PATCH 0/5] xfsprogs: rollup of various fixes for 5.6
@ 2020-04-06 18:52 Darrick J. Wong
2020-04-06 18:52 ` [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name Darrick J. Wong
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Darrick J. Wong @ 2020-04-06 18:52 UTC (permalink / raw)
To: sandeen, darrick.wong; +Cc: linux-xfs
Hi all,
This series contains fixes for a handful of errors that Coverity found,
minor reworks for API shifts, and fix for a typecasting problem in scrub.
If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.
This is an extraordinary way to destroy everything. Enjoy!
Comments and questions are, as always, welcome.
--D
xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=xfsprogs-5.6-fixes
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name 2020-04-06 18:52 [PATCH 0/5] xfsprogs: rollup of various fixes for 5.6 Darrick J. Wong @ 2020-04-06 18:52 ` Darrick J. Wong 2020-04-06 22:06 ` Allison Collins 2020-04-09 7:43 ` Christoph Hellwig 2020-04-06 18:52 ` [PATCH 2/5] libxfs: check return value of device flush when closing device Darrick J. Wong ` (3 subsequent siblings) 4 siblings, 2 replies; 16+ messages in thread From: Darrick J. Wong @ 2020-04-06 18:52 UTC (permalink / raw) To: sandeen, darrick.wong; +Cc: linux-xfs From: Darrick J. Wong <darrick.wong@oracle.com> Don't crash if we failed to write a buffer that had no buffer verifier. This should be rare in practice, but coverity found a valid bug. Coverity-id: 1460462 Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- libxfs/rdwr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c index 3c43a4d0..8a690269 100644 --- a/libxfs/rdwr.c +++ b/libxfs/rdwr.c @@ -1028,7 +1028,7 @@ libxfs_bwrite( if (bp->b_error) { fprintf(stderr, _("%s: write failed on %s bno 0x%llx/0x%x, err=%d\n"), - __func__, bp->b_ops->name, + __func__, bp->b_ops ? bp->b_ops->name : "(unknown)", (long long)bp->b_bn, bp->b_bcount, -bp->b_error); } else { bp->b_flags |= LIBXFS_B_UPTODATE; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name 2020-04-06 18:52 ` [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name Darrick J. Wong @ 2020-04-06 22:06 ` Allison Collins 2020-04-09 7:43 ` Christoph Hellwig 1 sibling, 0 replies; 16+ messages in thread From: Allison Collins @ 2020-04-06 22:06 UTC (permalink / raw) To: Darrick J. Wong, sandeen; +Cc: linux-xfs On 4/6/20 11:52 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Don't crash if we failed to write a buffer that had no buffer verifier. > This should be rare in practice, but coverity found a valid bug. > > Coverity-id: 1460462 > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Looks ok: Reviewed-by: Allison Collins <allison.henderson@oracle.com> > --- > libxfs/rdwr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c > index 3c43a4d0..8a690269 100644 > --- a/libxfs/rdwr.c > +++ b/libxfs/rdwr.c > @@ -1028,7 +1028,7 @@ libxfs_bwrite( > if (bp->b_error) { > fprintf(stderr, > _("%s: write failed on %s bno 0x%llx/0x%x, err=%d\n"), > - __func__, bp->b_ops->name, > + __func__, bp->b_ops ? bp->b_ops->name : "(unknown)", > (long long)bp->b_bn, bp->b_bcount, -bp->b_error); > } else { > bp->b_flags |= LIBXFS_B_UPTODATE; > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name 2020-04-06 18:52 ` [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name Darrick J. Wong 2020-04-06 22:06 ` Allison Collins @ 2020-04-09 7:43 ` Christoph Hellwig 1 sibling, 0 replies; 16+ messages in thread From: Christoph Hellwig @ 2020-04-09 7:43 UTC (permalink / raw) To: Darrick J. Wong; +Cc: sandeen, linux-xfs On Mon, Apr 06, 2020 at 11:52:30AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Don't crash if we failed to write a buffer that had no buffer verifier. > This should be rare in practice, but coverity found a valid bug. > > Coverity-id: 1460462 > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/5] libxfs: check return value of device flush when closing device 2020-04-06 18:52 [PATCH 0/5] xfsprogs: rollup of various fixes for 5.6 Darrick J. Wong 2020-04-06 18:52 ` [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name Darrick J. Wong @ 2020-04-06 18:52 ` Darrick J. Wong 2020-04-06 22:06 ` Allison Collins 2020-04-09 7:43 ` Christoph Hellwig 2020-04-06 18:52 ` [PATCH 3/5] xfs_db: clean up the salvage read callsites in set_cur() Darrick J. Wong ` (2 subsequent siblings) 4 siblings, 2 replies; 16+ messages in thread From: Darrick J. Wong @ 2020-04-06 18:52 UTC (permalink / raw) To: sandeen, darrick.wong; +Cc: linux-xfs From: Darrick J. Wong <darrick.wong@oracle.com> Although the libxfs_umount function flushes all devices when unmounting the incore filesystem, the libxfs io code will flush the device again when the application close them. Check and report any errors that might happen, though this is unlikely. Coverity-id: 1460464 Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- libxfs/init.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/libxfs/init.c b/libxfs/init.c index 3e6436c1..cb8967bc 100644 --- a/libxfs/init.c +++ b/libxfs/init.c @@ -166,13 +166,18 @@ libxfs_device_close(dev_t dev) for (d = 0; d < MAX_DEVS; d++) if (dev_map[d].dev == dev) { - int fd; + int fd, ret; fd = dev_map[d].fd; dev_map[d].dev = dev_map[d].fd = 0; - fsync(fd); - platform_flush_device(fd, dev); + ret = platform_flush_device(fd, dev); + if (ret) { + ret = -errno; + fprintf(stderr, + _("%s: flush of device %lld failed, err=%d"), + progname, (long long)dev, ret); + } close(fd); return; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] libxfs: check return value of device flush when closing device 2020-04-06 18:52 ` [PATCH 2/5] libxfs: check return value of device flush when closing device Darrick J. Wong @ 2020-04-06 22:06 ` Allison Collins 2020-04-09 7:43 ` Christoph Hellwig 1 sibling, 0 replies; 16+ messages in thread From: Allison Collins @ 2020-04-06 22:06 UTC (permalink / raw) To: Darrick J. Wong, sandeen; +Cc: linux-xfs On 4/6/20 11:52 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Although the libxfs_umount function flushes all devices when unmounting > the incore filesystem, the libxfs io code will flush the device again > when the application close them. Check and report any errors that might > happen, though this is unlikely. > > Coverity-id: 1460464 > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Ok, makes sense Reviewed-by: Allison Collins <allison.henderson@oracle.com> > --- > libxfs/init.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > > diff --git a/libxfs/init.c b/libxfs/init.c > index 3e6436c1..cb8967bc 100644 > --- a/libxfs/init.c > +++ b/libxfs/init.c > @@ -166,13 +166,18 @@ libxfs_device_close(dev_t dev) > > for (d = 0; d < MAX_DEVS; d++) > if (dev_map[d].dev == dev) { > - int fd; > + int fd, ret; > > fd = dev_map[d].fd; > dev_map[d].dev = dev_map[d].fd = 0; > > - fsync(fd); > - platform_flush_device(fd, dev); > + ret = platform_flush_device(fd, dev); > + if (ret) { > + ret = -errno; > + fprintf(stderr, > + _("%s: flush of device %lld failed, err=%d"), > + progname, (long long)dev, ret); > + } > close(fd); > > return; > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] libxfs: check return value of device flush when closing device 2020-04-06 18:52 ` [PATCH 2/5] libxfs: check return value of device flush when closing device Darrick J. Wong 2020-04-06 22:06 ` Allison Collins @ 2020-04-09 7:43 ` Christoph Hellwig 1 sibling, 0 replies; 16+ messages in thread From: Christoph Hellwig @ 2020-04-09 7:43 UTC (permalink / raw) To: Darrick J. Wong; +Cc: sandeen, linux-xfs On Mon, Apr 06, 2020 at 11:52:36AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Although the libxfs_umount function flushes all devices when unmounting > the incore filesystem, the libxfs io code will flush the device again > when the application close them. Check and report any errors that might > happen, though this is unlikely. > > Coverity-id: 1460464 > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/5] xfs_db: clean up the salvage read callsites in set_cur() 2020-04-06 18:52 [PATCH 0/5] xfsprogs: rollup of various fixes for 5.6 Darrick J. Wong 2020-04-06 18:52 ` [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name Darrick J. Wong 2020-04-06 18:52 ` [PATCH 2/5] libxfs: check return value of device flush when closing device Darrick J. Wong @ 2020-04-06 18:52 ` Darrick J. Wong 2020-04-06 22:07 ` Allison Collins 2020-04-06 18:52 ` [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf Darrick J. Wong 2020-04-06 18:52 ` [PATCH 5/5] xfs_scrub: fix type error in render_ino_from_handle Darrick J. Wong 4 siblings, 1 reply; 16+ messages in thread From: Darrick J. Wong @ 2020-04-06 18:52 UTC (permalink / raw) To: sandeen, darrick.wong; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner From: Darrick J. Wong <darrick.wong@oracle.com> Clean up the LIBXFS_READBUF_SALVAGE call sites in set_cur so that we use the return value directly instead of scraping it out later. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> --- db/io.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/db/io.c b/db/io.c index 384e4c0f..6628d061 100644 --- a/db/io.c +++ b/db/io.c @@ -516,6 +516,7 @@ set_cur( xfs_ino_t ino; uint16_t mode; const struct xfs_buf_ops *ops = type ? type->bops : NULL; + int error; if (iocur_sp < 0) { dbprintf(_("set_cur no stack element to set\n")); @@ -542,20 +543,21 @@ set_cur( if (!iocur_top->bbmap) return; memcpy(iocur_top->bbmap, bbmap, sizeof(struct bbmap)); - libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, bbmap->nmaps, - LIBXFS_READBUF_SALVAGE, &bp, ops); + error = -libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, + bbmap->nmaps, LIBXFS_READBUF_SALVAGE, &bp, + ops); } else { - libxfs_buf_read(mp->m_ddev_targp, blknum, len, + error = -libxfs_buf_read(mp->m_ddev_targp, blknum, len, LIBXFS_READBUF_SALVAGE, &bp, ops); iocur_top->bbmap = NULL; } /* - * Keep the buffer even if the verifier says it is corrupted. - * We're a diagnostic tool, after all. + * Salvage mode means that we still get a buffer even if the verifier + * says the metadata is corrupt. Therefore, the only errors we should + * get are for IO errors or runtime errors. */ - if (!bp || (bp->b_error && bp->b_error != -EFSCORRUPTED && - bp->b_error != -EFSBADCRC)) + if (error) return; iocur_top->buf = bp->b_addr; iocur_top->bp = bp; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/5] xfs_db: clean up the salvage read callsites in set_cur() 2020-04-06 18:52 ` [PATCH 3/5] xfs_db: clean up the salvage read callsites in set_cur() Darrick J. Wong @ 2020-04-06 22:07 ` Allison Collins 0 siblings, 0 replies; 16+ messages in thread From: Allison Collins @ 2020-04-06 22:07 UTC (permalink / raw) To: Darrick J. Wong, sandeen; +Cc: linux-xfs, Christoph Hellwig, Dave Chinner On 4/6/20 11:52 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Clean up the LIBXFS_READBUF_SALVAGE call sites in set_cur so that we > use the return value directly instead of scraping it out later. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Dave Chinner <dchinner@redhat.com> Looks ok: Reviewed-by: Allison Collins <allison.henderson@oracle.com> > --- > db/io.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > > diff --git a/db/io.c b/db/io.c > index 384e4c0f..6628d061 100644 > --- a/db/io.c > +++ b/db/io.c > @@ -516,6 +516,7 @@ set_cur( > xfs_ino_t ino; > uint16_t mode; > const struct xfs_buf_ops *ops = type ? type->bops : NULL; > + int error; > > if (iocur_sp < 0) { > dbprintf(_("set_cur no stack element to set\n")); > @@ -542,20 +543,21 @@ set_cur( > if (!iocur_top->bbmap) > return; > memcpy(iocur_top->bbmap, bbmap, sizeof(struct bbmap)); > - libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, bbmap->nmaps, > - LIBXFS_READBUF_SALVAGE, &bp, ops); > + error = -libxfs_buf_read_map(mp->m_ddev_targp, bbmap->b, > + bbmap->nmaps, LIBXFS_READBUF_SALVAGE, &bp, > + ops); > } else { > - libxfs_buf_read(mp->m_ddev_targp, blknum, len, > + error = -libxfs_buf_read(mp->m_ddev_targp, blknum, len, > LIBXFS_READBUF_SALVAGE, &bp, ops); > iocur_top->bbmap = NULL; > } > > /* > - * Keep the buffer even if the verifier says it is corrupted. > - * We're a diagnostic tool, after all. > + * Salvage mode means that we still get a buffer even if the verifier > + * says the metadata is corrupt. Therefore, the only errors we should > + * get are for IO errors or runtime errors. > */ > - if (!bp || (bp->b_error && bp->b_error != -EFSCORRUPTED && > - bp->b_error != -EFSBADCRC)) > + if (error) > return; > iocur_top->buf = bp->b_addr; > iocur_top->bp = bp; > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf 2020-04-06 18:52 [PATCH 0/5] xfsprogs: rollup of various fixes for 5.6 Darrick J. Wong ` (2 preceding siblings ...) 2020-04-06 18:52 ` [PATCH 3/5] xfs_db: clean up the salvage read callsites in set_cur() Darrick J. Wong @ 2020-04-06 18:52 ` Darrick J. Wong 2020-04-06 22:09 ` Allison Collins ` (2 more replies) 2020-04-06 18:52 ` [PATCH 5/5] xfs_scrub: fix type error in render_ino_from_handle Darrick J. Wong 4 siblings, 3 replies; 16+ messages in thread From: Darrick J. Wong @ 2020-04-06 18:52 UTC (permalink / raw) To: sandeen, darrick.wong; +Cc: linux-xfs From: Darrick J. Wong <darrick.wong@oracle.com> xfs_da_read_buf dropped the 'mappedbno' argument in favor of a flags argument. Foolishly, we're passing that parameter (which is -1 in all callers) to xfs_da_read_buf, which gets us the wrong behavior. Since mappedbno == -1 meant "complain if we fall into a hole" (which is the default behavior of xfs_da_read_buf) we can fix this by passing a zero flags argument and getting rid of mappedbno entirely. Coverity-id: 1457898 Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- repair/phase6.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/repair/phase6.c b/repair/phase6.c index 3fb1af24..beceea9a 100644 --- a/repair/phase6.c +++ b/repair/phase6.c @@ -179,7 +179,6 @@ static int dir_read_buf( struct xfs_inode *ip, xfs_dablk_t bno, - xfs_daddr_t mappedbno, struct xfs_buf **bpp, const struct xfs_buf_ops *ops, int *crc_error) @@ -187,14 +186,13 @@ dir_read_buf( int error; int error2; - error = -libxfs_da_read_buf(NULL, ip, bno, mappedbno, bpp, - XFS_DATA_FORK, ops); + error = -libxfs_da_read_buf(NULL, ip, bno, 0, bpp, XFS_DATA_FORK, ops); if (error != EFSBADCRC && error != EFSCORRUPTED) return error; - error2 = -libxfs_da_read_buf(NULL, ip, bno, mappedbno, bpp, - XFS_DATA_FORK, NULL); + error2 = -libxfs_da_read_buf(NULL, ip, bno, 0, bpp, XFS_DATA_FORK, + NULL); if (error2) return error2; @@ -2035,8 +2033,7 @@ longform_dir2_check_leaf( int fixit = 0; da_bno = mp->m_dir_geo->leafblk; - error = dir_read_buf(ip, da_bno, -1, &bp, &xfs_dir3_leaf1_buf_ops, - &fixit); + error = dir_read_buf(ip, da_bno, &bp, &xfs_dir3_leaf1_buf_ops, &fixit); if (error == EFSBADCRC || error == EFSCORRUPTED || fixit) { do_warn( _("leaf block %u for directory inode %" PRIu64 " bad CRC\n"), @@ -2137,8 +2134,8 @@ longform_dir2_check_node( * a node block, then we'll skip it below based on a magic * number check. */ - error = dir_read_buf(ip, da_bno, -1, &bp, - &xfs_da3_node_buf_ops, &fixit); + error = dir_read_buf(ip, da_bno, &bp, &xfs_da3_node_buf_ops, + &fixit); if (error) { do_warn( _("can't read leaf block %u for directory inode %" PRIu64 ", error %d\n"), @@ -2205,8 +2202,8 @@ longform_dir2_check_node( if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK)) break; - error = dir_read_buf(ip, da_bno, -1, &bp, - &xfs_dir3_free_buf_ops, &fixit); + error = dir_read_buf(ip, da_bno, &bp, &xfs_dir3_free_buf_ops, + &fixit); if (error) { do_warn( _("can't read freespace block %u for directory inode %" PRIu64 ", error %d\n"), @@ -2367,7 +2364,7 @@ longform_dir2_entry_check(xfs_mount_t *mp, else ops = &xfs_dir3_data_buf_ops; - error = dir_read_buf(ip, da_bno, -1, &bplist[db], ops, &fixit); + error = dir_read_buf(ip, da_bno, &bplist[db], ops, &fixit); if (error) { do_warn( _("can't read data block %u for directory inode %" PRIu64 " error %d\n"), ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf 2020-04-06 18:52 ` [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf Darrick J. Wong @ 2020-04-06 22:09 ` Allison Collins 2020-04-07 19:05 ` Eric Sandeen 2020-04-09 7:44 ` Christoph Hellwig 2 siblings, 0 replies; 16+ messages in thread From: Allison Collins @ 2020-04-06 22:09 UTC (permalink / raw) To: Darrick J. Wong, sandeen; +Cc: linux-xfs On 4/6/20 11:52 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > xfs_da_read_buf dropped the 'mappedbno' argument in favor of a flags > argument. Foolishly, we're passing that parameter (which is -1 in all > callers) to xfs_da_read_buf, which gets us the wrong behavior. > > Since mappedbno == -1 meant "complain if we fall into a hole" (which is > the default behavior of xfs_da_read_buf) we can fix this by passing a > zero flags argument and getting rid of mappedbno entirely. > > Coverity-id: 1457898 > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> I dont see any logical errors Reviewed-by: Allison Collins <allison.henderson@oracle.com> > --- > repair/phase6.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > > diff --git a/repair/phase6.c b/repair/phase6.c > index 3fb1af24..beceea9a 100644 > --- a/repair/phase6.c > +++ b/repair/phase6.c > @@ -179,7 +179,6 @@ static int > dir_read_buf( > struct xfs_inode *ip, > xfs_dablk_t bno, > - xfs_daddr_t mappedbno, > struct xfs_buf **bpp, > const struct xfs_buf_ops *ops, > int *crc_error) > @@ -187,14 +186,13 @@ dir_read_buf( > int error; > int error2; > > - error = -libxfs_da_read_buf(NULL, ip, bno, mappedbno, bpp, > - XFS_DATA_FORK, ops); > + error = -libxfs_da_read_buf(NULL, ip, bno, 0, bpp, XFS_DATA_FORK, ops); > > if (error != EFSBADCRC && error != EFSCORRUPTED) > return error; > > - error2 = -libxfs_da_read_buf(NULL, ip, bno, mappedbno, bpp, > - XFS_DATA_FORK, NULL); > + error2 = -libxfs_da_read_buf(NULL, ip, bno, 0, bpp, XFS_DATA_FORK, > + NULL); > if (error2) > return error2; > > @@ -2035,8 +2033,7 @@ longform_dir2_check_leaf( > int fixit = 0; > > da_bno = mp->m_dir_geo->leafblk; > - error = dir_read_buf(ip, da_bno, -1, &bp, &xfs_dir3_leaf1_buf_ops, > - &fixit); > + error = dir_read_buf(ip, da_bno, &bp, &xfs_dir3_leaf1_buf_ops, &fixit); > if (error == EFSBADCRC || error == EFSCORRUPTED || fixit) { > do_warn( > _("leaf block %u for directory inode %" PRIu64 " bad CRC\n"), > @@ -2137,8 +2134,8 @@ longform_dir2_check_node( > * a node block, then we'll skip it below based on a magic > * number check. > */ > - error = dir_read_buf(ip, da_bno, -1, &bp, > - &xfs_da3_node_buf_ops, &fixit); > + error = dir_read_buf(ip, da_bno, &bp, &xfs_da3_node_buf_ops, > + &fixit); > if (error) { > do_warn( > _("can't read leaf block %u for directory inode %" PRIu64 ", error %d\n"), > @@ -2205,8 +2202,8 @@ longform_dir2_check_node( > if (bmap_next_offset(NULL, ip, &next_da_bno, XFS_DATA_FORK)) > break; > > - error = dir_read_buf(ip, da_bno, -1, &bp, > - &xfs_dir3_free_buf_ops, &fixit); > + error = dir_read_buf(ip, da_bno, &bp, &xfs_dir3_free_buf_ops, > + &fixit); > if (error) { > do_warn( > _("can't read freespace block %u for directory inode %" PRIu64 ", error %d\n"), > @@ -2367,7 +2364,7 @@ longform_dir2_entry_check(xfs_mount_t *mp, > else > ops = &xfs_dir3_data_buf_ops; > > - error = dir_read_buf(ip, da_bno, -1, &bplist[db], ops, &fixit); > + error = dir_read_buf(ip, da_bno, &bplist[db], ops, &fixit); > if (error) { > do_warn( > _("can't read data block %u for directory inode %" PRIu64 " error %d\n"), > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf 2020-04-06 18:52 ` [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf Darrick J. Wong 2020-04-06 22:09 ` Allison Collins @ 2020-04-07 19:05 ` Eric Sandeen 2020-04-09 7:44 ` Christoph Hellwig 2 siblings, 0 replies; 16+ messages in thread From: Eric Sandeen @ 2020-04-07 19:05 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs On 4/6/20 1:52 PM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > xfs_da_read_buf dropped the 'mappedbno' argument in favor of a flags > argument. Foolishly, we're passing that parameter (which is -1 in all > callers) to xfs_da_read_buf, which gets us the wrong behavior. > > Since mappedbno == -1 meant "complain if we fall into a hole" (which is > the default behavior of xfs_da_read_buf) we can fix this by passing a > zero flags argument and getting rid of mappedbno entirely. > > Coverity-id: 1457898 > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Looks good; I think this patch Fixes: 5f356ae6d ("xfs: remove the mappedbno argument to xfs_da_read_buf") (i.e. the merge of that kernel commit, which I biffed on, apparently) Reviewed-by: Eric Sandeen <sandeen@redhat.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf 2020-04-06 18:52 ` [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf Darrick J. Wong 2020-04-06 22:09 ` Allison Collins 2020-04-07 19:05 ` Eric Sandeen @ 2020-04-09 7:44 ` Christoph Hellwig 2 siblings, 0 replies; 16+ messages in thread From: Christoph Hellwig @ 2020-04-09 7:44 UTC (permalink / raw) To: Darrick J. Wong; +Cc: sandeen, linux-xfs Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/5] xfs_scrub: fix type error in render_ino_from_handle 2020-04-06 18:52 [PATCH 0/5] xfsprogs: rollup of various fixes for 5.6 Darrick J. Wong ` (3 preceding siblings ...) 2020-04-06 18:52 ` [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf Darrick J. Wong @ 2020-04-06 18:52 ` Darrick J. Wong 2020-04-06 22:09 ` Allison Collins 2020-04-09 7:44 ` Christoph Hellwig 4 siblings, 2 replies; 16+ messages in thread From: Darrick J. Wong @ 2020-04-06 18:52 UTC (permalink / raw) To: sandeen, darrick.wong; +Cc: linux-xfs From: Darrick J. Wong <darrick.wong@oracle.com> render_ino_from_handle is passed a struct xfs_bulkstat, not xfs_bstat. Fix this. Fixes: 4cca629d6ae3807 ("misc: convert xfrog_bulkstat functions to have v5 semantics") Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> --- scrub/phase5.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrub/phase5.c b/scrub/phase5.c index 540b840d..fcd5ba27 100644 --- a/scrub/phase5.c +++ b/scrub/phase5.c @@ -242,7 +242,7 @@ render_ino_from_handle( size_t buflen, void *data) { - struct xfs_bstat *bstat = data; + struct xfs_bulkstat *bstat = data; return scrub_render_ino_descr(ctx, buf, buflen, bstat->bs_ino, bstat->bs_gen, NULL); ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] xfs_scrub: fix type error in render_ino_from_handle 2020-04-06 18:52 ` [PATCH 5/5] xfs_scrub: fix type error in render_ino_from_handle Darrick J. Wong @ 2020-04-06 22:09 ` Allison Collins 2020-04-09 7:44 ` Christoph Hellwig 1 sibling, 0 replies; 16+ messages in thread From: Allison Collins @ 2020-04-06 22:09 UTC (permalink / raw) To: Darrick J. Wong, sandeen; +Cc: linux-xfs On 4/6/20 11:52 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > render_ino_from_handle is passed a struct xfs_bulkstat, not xfs_bstat. > Fix this. > > Fixes: 4cca629d6ae3807 ("misc: convert xfrog_bulkstat functions to have v5 semantics") > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Ok, looks good Reviewed-by: Allison Collins <allison.henderson@oracle.com> > --- > scrub/phase5.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > diff --git a/scrub/phase5.c b/scrub/phase5.c > index 540b840d..fcd5ba27 100644 > --- a/scrub/phase5.c > +++ b/scrub/phase5.c > @@ -242,7 +242,7 @@ render_ino_from_handle( > size_t buflen, > void *data) > { > - struct xfs_bstat *bstat = data; > + struct xfs_bulkstat *bstat = data; > > return scrub_render_ino_descr(ctx, buf, buflen, bstat->bs_ino, > bstat->bs_gen, NULL); > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] xfs_scrub: fix type error in render_ino_from_handle 2020-04-06 18:52 ` [PATCH 5/5] xfs_scrub: fix type error in render_ino_from_handle Darrick J. Wong 2020-04-06 22:09 ` Allison Collins @ 2020-04-09 7:44 ` Christoph Hellwig 1 sibling, 0 replies; 16+ messages in thread From: Christoph Hellwig @ 2020-04-09 7:44 UTC (permalink / raw) To: Darrick J. Wong; +Cc: sandeen, linux-xfs Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-04-09 7:44 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-06 18:52 [PATCH 0/5] xfsprogs: rollup of various fixes for 5.6 Darrick J. Wong 2020-04-06 18:52 ` [PATCH 1/5] libxfs: don't barf in libxfs_bwrite on a null buffer ops name Darrick J. Wong 2020-04-06 22:06 ` Allison Collins 2020-04-09 7:43 ` Christoph Hellwig 2020-04-06 18:52 ` [PATCH 2/5] libxfs: check return value of device flush when closing device Darrick J. Wong 2020-04-06 22:06 ` Allison Collins 2020-04-09 7:43 ` Christoph Hellwig 2020-04-06 18:52 ` [PATCH 3/5] xfs_db: clean up the salvage read callsites in set_cur() Darrick J. Wong 2020-04-06 22:07 ` Allison Collins 2020-04-06 18:52 ` [PATCH 4/5] xfs_repair: fix dir_read_buf use of libxfs_da_read_buf Darrick J. Wong 2020-04-06 22:09 ` Allison Collins 2020-04-07 19:05 ` Eric Sandeen 2020-04-09 7:44 ` Christoph Hellwig 2020-04-06 18:52 ` [PATCH 5/5] xfs_scrub: fix type error in render_ino_from_handle Darrick J. Wong 2020-04-06 22:09 ` Allison Collins 2020-04-09 7:44 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox