| Message ID | 20191007131938.23839-3-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | xfs: fix sf to block inode fork logging | expand |
On Mon, Oct 07, 2019 at 09:19:37AM -0400, Brian Foster wrote: > xfs_attr_shortform_to_leaf() attempts to put the shortform fork back > together after a failed attempt to convert from shortform to leaf > format. While this code reallocates and copies back the shortform > attr fork data, it never resets the inode format field back to local > format. Further, now that the inode is properly logged after the > initial switch from local format, any error that triggers the > recovery code will eventually abort the transaction and shutdown the > fs. Therefore, remove the broken and unnecessary error handling > code. > > Signed-off-by: Brian Foster <bfoster@redhat.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
On Mon, Oct 07, 2019 at 09:19:37AM -0400, Brian Foster wrote: > xfs_attr_shortform_to_leaf() attempts to put the shortform fork back > together after a failed attempt to convert from shortform to leaf > format. While this code reallocates and copies back the shortform > attr fork data, it never resets the inode format field back to local > format. Further, now that the inode is properly logged after the > initial switch from local format, any error that triggers the > recovery code will eventually abort the transaction and shutdown the > fs. Therefore, remove the broken and unnecessary error handling > code. > > Signed-off-by: Brian Foster <bfoster@redhat.com> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/libxfs/xfs_attr_leaf.c | 19 ++----------------- > 1 file changed, 2 insertions(+), 17 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c > index 36c0a32cefcf..1b956c313b6b 100644 > --- a/fs/xfs/libxfs/xfs_attr_leaf.c > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c > @@ -831,28 +831,13 @@ xfs_attr_shortform_to_leaf( > > bp = NULL; > error = xfs_da_grow_inode(args, &blkno); > - if (error) { > - /* > - * If we hit an IO error middle of the transaction inside > - * grow_inode(), we may have inconsistent data. Bail out. > - */ > - if (error == -EIO) > - goto out; > - xfs_idata_realloc(dp, size, XFS_ATTR_FORK); /* try to put */ > - memcpy(ifp->if_u1.if_data, tmpbuffer, size); /* it back */ > + if (error) > goto out; > - } > > ASSERT(blkno == 0); > error = xfs_attr3_leaf_create(args, blkno, &bp); > - if (error) { > - /* xfs_attr3_leaf_create may not have instantiated a block */ > - if (bp && (xfs_da_shrink_inode(args, 0, bp) != 0)) > - goto out; > - xfs_idata_realloc(dp, size, XFS_ATTR_FORK); /* try to put */ > - memcpy(ifp->if_u1.if_data, tmpbuffer, size); /* it back */ > + if (error) > goto out; > - } > > memset((char *)&nargs, 0, sizeof(nargs)); > nargs.dp = dp; > -- > 2.20.1 >
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index 36c0a32cefcf..1b956c313b6b 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -831,28 +831,13 @@ xfs_attr_shortform_to_leaf( bp = NULL; error = xfs_da_grow_inode(args, &blkno); - if (error) { - /* - * If we hit an IO error middle of the transaction inside - * grow_inode(), we may have inconsistent data. Bail out. - */ - if (error == -EIO) - goto out; - xfs_idata_realloc(dp, size, XFS_ATTR_FORK); /* try to put */ - memcpy(ifp->if_u1.if_data, tmpbuffer, size); /* it back */ + if (error) goto out; - } ASSERT(blkno == 0); error = xfs_attr3_leaf_create(args, blkno, &bp); - if (error) { - /* xfs_attr3_leaf_create may not have instantiated a block */ - if (bp && (xfs_da_shrink_inode(args, 0, bp) != 0)) - goto out; - xfs_idata_realloc(dp, size, XFS_ATTR_FORK); /* try to put */ - memcpy(ifp->if_u1.if_data, tmpbuffer, size); /* it back */ + if (error) goto out; - } memset((char *)&nargs, 0, sizeof(nargs)); nargs.dp = dp;
xfs_attr_shortform_to_leaf() attempts to put the shortform fork back together after a failed attempt to convert from shortform to leaf format. While this code reallocates and copies back the shortform attr fork data, it never resets the inode format field back to local format. Further, now that the inode is properly logged after the initial switch from local format, any error that triggers the recovery code will eventually abort the transaction and shutdown the fs. Therefore, remove the broken and unnecessary error handling code. Signed-off-by: Brian Foster <bfoster@redhat.com> --- fs/xfs/libxfs/xfs_attr_leaf.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-)