* [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
* [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
* [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
* [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
* [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 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 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 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
* 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 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 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 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
* 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
* 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
* 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