Re: [PATCH 5/6] libfrog: refactor open-coded bulkstat calls

[Dave Chinner <david@fromorbit.com>]

On Tue, Aug 20, 2019 at 01:31:03PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Refactor the BULKSTAT_SINGLE and BULKSTAT ioctl callsites into helper
> functions.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
.....
> ---
> @@ -617,25 +592,27 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
>  		return -1;
>  	}
>  
> -	if ((fsfd = open(mntdir, O_RDONLY)) < 0) {
> +	if ((fsxfd.fd = open(mntdir, O_RDONLY)) < 0) {
>  		fsrprintf(_("unable to open: %s: %s\n"),
>  		          mntdir, strerror( errno ));
>  		free(fshandlep);
>  		return -1;
>  	}
>  
> -	if (xfrog_geometry(fsfd, &fsgeom) < 0 ) {
> +	ret = xfrog_prepare_geometry(&fsxfd);
> +	if (ret) {
>  		fsrprintf(_("Skipping %s: could not get XFS geometry\n"),
>  			  mntdir);
> -		close(fsfd);
> +		xfrog_close(&fsxfd);
>  		free(fshandlep);
>  		return -1;
>  	}

/me wonders if this would be better wrapped as xfd_open(&xfd, path, flags,
mode) ?

> @@ -684,16 +661,16 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
>  		}
>  		if (endtime && endtime < time(NULL)) {
>  			tmp_close(mntdir);
> -			close(fsfd);
> +			xfrog_close(&fsxfd);
>  			fsrall_cleanup(1);
>  			exit(1);
>  		}
>  	}
>  	if (ret < 0)
> -		fsrprintf(_("%s: xfs_bulkstat: %s\n"), progname, strerror(errno));
> +		fsrprintf(_("%s: xfrog_bulkstat: %s\n"), progname, strerror(errno));

It'd change this to just "bulkstat" - it's a user facing error, and
they aren't going to know what "xfrog" means...

> @@ -745,14 +725,21 @@ fsrfile(char *fname, xfs_ino_t ino)
>  	 * Need to open something on the same filesystem as the
>  	 * file.  Open the parent.
>  	 */
> -	fsfd = open(getparent(fname), O_RDONLY);
> -	if (fsfd < 0) {
> +	fsxfd.fd = open(getparent(fname), O_RDONLY);
> +	if (fsxfd.fd < 0) {
>  		fsrprintf(_("unable to open sys handle for %s: %s\n"),
>  			fname, strerror(errno));
>  		goto out;
>  	}
>  
> -	if ((xfs_bulkstat_single(fsfd, &ino, &statbuf)) < 0) {
> +	error = xfrog_prepare_geometry(&fsxfd);
> +	if (error) {
> +		fsrprintf(_("Unable to get geom on fs for: %s\n"), fname);
> +		goto out;
> +	}
> +

same xfd_open() code here.

> diff --git a/io/open.c b/io/open.c
> index e70c8cb0..67976f7f 100644
> --- a/io/open.c
> +++ b/io/open.c
....
> @@ -767,35 +766,36 @@ inode_f(
>  			exitcode = 1;
>  			return 0;
>  		}
> +	} else if (ret_next) {
> +		struct xfs_fd	xfd = XFS_FD_INIT(file->fd);
> +
> +		/* get next inode */
> +		ret = xfrog_bulkstat(&xfd, &userino, 1, &bstat, &count);
> +		if (ret) {
> +			perror("xfsctl");

perror("bulkstat");

....

Otherwise looks ok.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com