Re: [PATCH 1/2] quota: Add mountpath based quota support

[Christoph Hellwig]

On Tue, Feb 02, 2021 at 07:02:41PM +0100, Jan Kara wrote:
> Hum, let me think out loud. The path we pass to Q_QUOTAON is a path to
> quota file - unless the filesystem stores quota in hidden files in which
> case this argument is just ignored. You're right we could require that
> specifically for Q_QUOTAON, the mountpoint path would actually need to
> point to the quota file if it is relevant, otherwise anywhere in the
> appropriate filesystem. We don't allow quota file to reside on a different
> filesystem (for a past decade or so) so it should work fine.
> 
> So the only problem I have is whether requiring the mountpoint argument to
> point quota file for Q_QUOTAON isn't going to be somewhat confusing to
> users. At the very least it would require some careful explanation in the
> manpage to explain the difference between quotactl_path() and quotactl()
> in this regard. But is saving the second path for Q_QUOTAON really worth the
> bother?

I find the doubled path argument a really horrible API, so I'd pretty
strongly prefer to avoid that.

> > Given how cheap quotactl_cmd_onoff and quotactl_cmd_write are we
> > could probably simplify this down do:
> > 
> > 	if (quotactl_cmd_write(cmd)) {
> 
> This needs to be (quotactl_cmd_write(cmd) || quotactl_cmd_onoff(cmd)).
> Otherwise I agree what you suggest is somewhat more readable given how
> small the function is.

The way I read quotactl_cmd_write, it only special cases a few commands
and returns 0 there, so we should not need the extra quotactl_cmd_onoff
call, as all those commands are not explicitly listed.