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

[Andrey Albershteyn <aalbersh@redhat.com>]

On Mon, Apr 25, 2022 at 09:28:36AM -0700, Darrick J. Wong wrote:
> 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

yeah, I also think that names are not very descriptive, but I didn't
want to change too many things. There's also space to merge report*
functions :)

> 
> 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
> > 
> 

-- 
- Andrey