Btrfs: return EUCLEAN rather than ENXIO once internal error has occurred for SEEK_DATA/SEEK_HOLE inquiry
PATCH 1/1 Btrfs: return EUCLEAN rather than ENXIO once internal error has occurred for SEEK_DATA/SEEK_HOLE inquiry
By referring to http://linux.die.net/man/2/lseek, return ENXIO only
when "offset beyond EOF" for either SEEK_DATA or SEEK_HOLE inquiry.
But we return it in case of internal issue too if btrfs_get_extent_fiemap() failed due to other issues. This will confuse the user applications to be expecting ENXIO when trying to find a specific data or hole location once it has occurred.
Thanks Dave for pointing that out in XFS thread.
This patch fix it to return EUCLEAN, or maybe another particular errno is more reasonable in Btrfs to indicate this fatal error?
Thanks,
-Jeff
--- fs/btrfs/file.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
fs/btrfs/file.c +2 -2
@@ -1761,7 +1761,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin) start - root->sectorsize, root->sectorsize, 0);if (IS_ERR (em)) { - ret = -ENXIO ; + ret = -EUCLEAN ; goto out; } last_end = em->start + em->len;
@@ -1773,7 +1773,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin)while (1 ) { em =btrfs_get_extent_fiemap (inode,NULL ,0 , start, len,0 );if (IS_ERR (em)) { - ret = -ENXIO ; + ret = -EUCLEAN ;break ; }
Signature · 1 line
1.7.9
Quoted 9 lines · Jeff Liu
By referring to http://linux.die.net/man/2/lseek, return ENXIO only
when "offset beyond EOF" for eitherSEEK_DATAorSEEK_HOLEinquiry.
But we return it in case of internal issue too ifbtrfs_get_extent_fiemap()failed due to other issues. This will confuse the user applications to be expecting ENXIO when trying to find a specific data or hole location once it has occurred.Thanks Dave for pointing that out in XFS thread.
This patch fix it to return EUCLEAN, or maybe another particular errno is more reasonable in Btrfs to indicate this fatal error?
Or maybe just return the error that was happened at internal routine, to give user more accurate error info, which is better?
Thanks,
-Jeff
Quoted 34 lines
Thanks,
-JeffSigned-off-by Jie Liu <[email protected]>--- fs/btrfs/file.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)fs/btrfs/file.c +2
-2@@ -1761,7 +1761,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin) start - root->sectorsize, root->sectorsize, 0);if (IS_ERR (em)) { - ret = -ENXIO ; + ret = -EUCLEAN ; goto out; } last_end = em->start + em->len;@@ -1773,7 +1773,7 @@ static int find_desired_extent(struct inode *inode, loff_t *offset, int origin)while (1 ) { em =btrfs_get_extent_fiemap (inode,NULL ,0 , start, len,0 );if (IS_ERR (em)) { - ret = -ENXIO ; + ret = -EUCLEAN ;break ; }
Quoted 8 lines · Jeff Liu
Quoted 6 lines · Jeff Liu
By referring to http://linux.die.net/man/2/lseek, return ENXIO only
when "offset beyond EOF" for eitherSEEK_DATAorSEEK_HOLEinquiry.
But we return it in case of internal issue too ifbtrfs_get_extent_fiemap()failed due to other issues. This will confuse the user applications to be expecting ENXIO when trying to find a specific data or hole location once it has occurred.
Quoted 6 lines
Quoted 3 lines
Thanks Dave for pointing that out in XFS thread.
This patch fix it to return EUCLEAN, or maybe another particular errno is more reasonable in Btrfs to indicate this fatal error?
Or maybe just return the error that was happened at internal routine, to give user more accurate error info, which is better?
Return the internal error unchanged - a failure to read the extent list (EIO) is different to a corruption detected in the extent map read from disk (EUCLEAN). Having a user report the appropriate error makes our life much simpler when it comes to trying to understand their problem....
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 13 lines · Dave Chinner
Quoted 12 lines · Jeff Liu
Quoted 9 lines · Jeff Liu
By referring to http://linux.die.net/man/2/lseek, return ENXIO only
when "offset beyond EOF" for eitherSEEK_DATAorSEEK_HOLEinquiry.
But we return it in case of internal issue too ifbtrfs_get_extent_fiemap()failed due to other issues. This will confuse the user applications to be expecting ENXIO when trying to find a specific data or hole location once it has occurred.Thanks Dave for pointing that out in XFS thread.
This patch fix it to return EUCLEAN, or maybe another particular errno is more reasonable in Btrfs to indicate this fatal error?
Quoted 8 lines
Quoted 2 lines
Or maybe just return the error that was happened at internal routine, to give user more accurate error info, which is better?
Return the internal error unchanged - a failure to read the extent list (EIO) is different to a corruption detected in the extent map read from disk (EUCLEAN). Having a user report the appropriate error makes our life much simpler when it comes to trying to understand their problem....
Definitely. I will repost this patch later.
Thanks,
-Jeff
Quoted 4 lines
Cheers,
Dave.