Re: [PATCH v2 2/5] xfs_quota: separate get_dquot() and dump_file()

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Apr 20, 2022 at 04:45:05PM +0200, Andrey Albershteyn wrote:
> Separate quota info acquisition from outputting it to file. This
> allows upper functions to filter obtained info (e.g. within specific
> ID range).
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> ---
>  quota/report.c | 86 ++++++++++++++++++++++++++------------------------
>  1 file changed, 45 insertions(+), 41 deletions(-)
> 
> diff --git a/quota/report.c b/quota/report.c
> index cccc654e..d5c6f84f 100644
> --- a/quota/report.c
> +++ b/quota/report.c
> @@ -96,39 +96,31 @@ get_dquot(
>  static int
>  dump_file(

I kinda wonder if this ought to be named 'dump_dquot' since it's not
really dumping a file or even the dquots of a specifc file.  OTOH, the
rest of the utility seems to have similar naming conventions for "report
something to a FILE* stream" so

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  	FILE		*fp,
> -	uint		id,
> -	uint		*oid,
> -	uint		type,
> -	char		*dev,
> -	int		flags)
> +	struct fs_disk_quota *d,
> +	char		*dev)
>  {
> -	fs_disk_quota_t	d;
> -
> -	if (!get_dquot(&d, id, oid, type, dev, flags))
> -		return 0;
> -
> -	if (!d.d_blk_softlimit && !d.d_blk_hardlimit &&
> -	    !d.d_ino_softlimit && !d.d_ino_hardlimit &&
> -	    !d.d_rtb_softlimit && !d.d_rtb_hardlimit)
> +	if (!d->d_blk_softlimit && !d->d_blk_hardlimit &&
> +	    !d->d_ino_softlimit && !d->d_ino_hardlimit &&
> +	    !d->d_rtb_softlimit && !d->d_rtb_hardlimit)
>  		return 1;
>  	fprintf(fp, "fs = %s\n", dev);
>  	/* this branch is for backward compatibility reasons */
> -	if (d.d_rtb_softlimit || d.d_rtb_hardlimit)
> +	if (d->d_rtb_softlimit || d->d_rtb_hardlimit)
>  		fprintf(fp, "%-10d %7llu %7llu %7llu %7llu %7llu %7llu\n",
> -			d.d_id,
> -			(unsigned long long)d.d_blk_softlimit,
> -			(unsigned long long)d.d_blk_hardlimit,
> -			(unsigned long long)d.d_ino_softlimit,
> -			(unsigned long long)d.d_ino_hardlimit,
> -			(unsigned long long)d.d_rtb_softlimit,
> -			(unsigned long long)d.d_rtb_hardlimit);
> +			d->d_id,
> +			(unsigned long long)d->d_blk_softlimit,
> +			(unsigned long long)d->d_blk_hardlimit,
> +			(unsigned long long)d->d_ino_softlimit,
> +			(unsigned long long)d->d_ino_hardlimit,
> +			(unsigned long long)d->d_rtb_softlimit,
> +			(unsigned long long)d->d_rtb_hardlimit);
>  	else
>  		fprintf(fp, "%-10d %7llu %7llu %7llu %7llu\n",
> -			d.d_id,
> -			(unsigned long long)d.d_blk_softlimit,
> -			(unsigned long long)d.d_blk_hardlimit,
> -			(unsigned long long)d.d_ino_softlimit,
> -			(unsigned long long)d.d_ino_hardlimit);
> +			d->d_id,
> +			(unsigned long long)d->d_blk_softlimit,
> +			(unsigned long long)d->d_blk_hardlimit,
> +			(unsigned long long)d->d_ino_softlimit,
> +			(unsigned long long)d->d_ino_hardlimit);
>  
>  	return 1;
>  }
> @@ -142,6 +134,7 @@ dump_limits_any_type(
>  	uint		upper)
>  {
>  	fs_path_t	*mount;
> +	struct fs_disk_quota d;
>  	uint		id = 0, oid;
>  
>  	if ((mount = fs_table_lookup(dir, FS_MOUNT_POINT)) == NULL) {
> @@ -153,46 +146,57 @@ dump_limits_any_type(
>  
>  	/* Range was specified; query everything in it */
>  	if (upper) {
> -		for (id = lower; id <= upper; id++)
> -			dump_file(fp, id, NULL, type, mount->fs_name, 0);
> +		for (id = lower; id <= upper; id++) {
> +			get_dquot(&d, id, &oid, type, mount->fs_name, 0);
> +			dump_file(fp, &d, mount->fs_name);
> +		}
>  		return;
>  	}
>  
>  	/* Use GETNEXTQUOTA if it's available */
> -	if (dump_file(fp, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
> +	if (get_dquot(&d, id, &oid, type, mount->fs_name, GETNEXTQUOTA_FLAG)) {
> +		dump_file(fp, &d, mount->fs_name);
>  		id = oid + 1;
> -		while (dump_file(fp, id, &oid, type, mount->fs_name,
> -				 GETNEXTQUOTA_FLAG))
> +		while (get_dquot(&d, id, &oid, type, mount->fs_name,
> +					GETNEXTQUOTA_FLAG)) {
> +			dump_file(fp, &d, mount->fs_name);
>  			id = oid + 1;
> +		}
>  		return;
> -        }
> +	}
>  
>  	/* Otherwise fall back to iterating over each uid/gid/prjid */
>  	switch (type) {
>  	case XFS_GROUP_QUOTA: {
>  			struct group *g;
>  			setgrent();
> -			while ((g = getgrent()) != NULL)
> -				dump_file(fp, g->gr_gid, NULL, type,
> -					  mount->fs_name, 0);
> +			while ((g = getgrent()) != NULL) {
> +				get_dquot(&d, g->gr_gid, NULL, type,
> +						mount->fs_name, 0);
> +				dump_file(fp, &d, mount->fs_name);
> +			}
>  			endgrent();
>  			break;
>  		}
>  	case XFS_PROJ_QUOTA: {
>  			struct fs_project *p;
>  			setprent();
> -			while ((p = getprent()) != NULL)
> -				dump_file(fp, p->pr_prid, NULL, type,
> -					  mount->fs_name, 0);
> +			while ((p = getprent()) != NULL) {
> +				get_dquot(&d, p->pr_prid, NULL, type,
> +						mount->fs_name, 0);
> +				dump_file(fp, &d, mount->fs_name);
> +			}
>  			endprent();
>  			break;
>  		}
>  	case XFS_USER_QUOTA: {
>  			struct passwd *u;
>  			setpwent();
> -			while ((u = getpwent()) != NULL)
> -				dump_file(fp, u->pw_uid, NULL, type,
> -					  mount->fs_name, 0);
> +			while ((u = getpwent()) != NULL) {
> +				get_dquot(&d, u->pw_uid, NULL, type,
> +						mount->fs_name, 0);
> +				dump_file(fp, &d, mount->fs_name);
> +			}
>  			endpwent();
>  			break;
>  		}
> -- 
> 2.27.0
>