[2/5] libxfs: check return value of device flush when closing device
diff mbox series

Message ID 158619915636.469742.17283369979015724938.stgit@magnolia
State Accepted
Headers show
Series
  • xfsprogs: rollup of various fixes for 5.6
Related show

Commit Message

Darrick J. Wong April 6, 2020, 6:52 p.m. UTC
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(-)

Comments

Allison Collins April 6, 2020, 10:06 p.m. UTC | #1
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;
>
Christoph Hellwig April 9, 2020, 7:43 a.m. UTC | #2
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>

Patch
diff mbox series

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;