Re: [PATCH 3/4] libxfs: return flush failures

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Feb 06, 2020 at 12:38:06PM -0700, Allison Collins wrote:
> 
> On 2/4/20 5:47 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Modify platform_flush_device so that we can return error status when
> > device flushes fail.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> I think there's one other place in init.c where platform_flush_device is
> called, but I suppose it didn't need the return code before?

I think the lack of error checking is/was just plain broken, seeing as
the current libxfs_close() code path just eats errors.

However, now that everyone calls libxfs_flush() to find out which
devices (if any) succeeded in flushing, I think there's less need to go
reworking that whole code path.

--D

> Other than that it looks ok.

> Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> 
> > ---
> >   libfrog/linux.c    |   25 +++++++++++++++++--------
> >   libfrog/platform.h |    2 +-
> >   2 files changed, 18 insertions(+), 9 deletions(-)
> > 
> > 
> > diff --git a/libfrog/linux.c b/libfrog/linux.c
> > index 41a168b4..60bc1dc4 100644
> > --- a/libfrog/linux.c
> > +++ b/libfrog/linux.c
> > @@ -140,20 +140,29 @@ platform_set_blocksize(int fd, char *path, dev_t device, int blocksize, int fata
> >   	return error;
> >   }
> > -void
> > -platform_flush_device(int fd, dev_t device)
> > +/*
> > + * Flush dirty pagecache and disk write cache to stable media.  Returns 0 for
> > + * success or -1 (with errno set) for failure.
> > + */
> > +int
> > +platform_flush_device(
> > +	int		fd,
> > +	dev_t		device)
> >   {
> >   	struct stat	st;
> > +	int		ret;
> > +
> >   	if (major(device) == RAMDISK_MAJOR)
> > -		return;
> > +		return 0;
> > -	if (fstat(fd, &st) < 0)
> > -		return;
> > +	ret = fstat(fd, &st);
> > +	if (ret)
> > +		return ret;
> >   	if (S_ISREG(st.st_mode))
> > -		fsync(fd);
> > -	else
> > -		ioctl(fd, BLKFLSBUF, 0);
> > +		return fsync(fd);
> > +
> > +	return ioctl(fd, BLKFLSBUF, 0);
> >   }
> >   void
> > diff --git a/libfrog/platform.h b/libfrog/platform.h
> > index 76887e5e..0aef318a 100644
> > --- a/libfrog/platform.h
> > +++ b/libfrog/platform.h
> > @@ -12,7 +12,7 @@ int platform_check_ismounted(char *path, char *block, struct stat *sptr,
> >   int platform_check_iswritable(char *path, char *block, struct stat *sptr);
> >   int platform_set_blocksize(int fd, char *path, dev_t device, int bsz,
> >   		int fatal);
> > -void platform_flush_device(int fd, dev_t device);
> > +int platform_flush_device(int fd, dev_t device);
> >   char *platform_findrawpath(char *path);
> >   char *platform_findblockpath(char *path);
> >   int platform_direct_blockdev(void);
> >