Re: [PATCH 1/8] quota: Allow to pass mount path to quotactl

[Jan Kara <jack@suse.cz>]

On Tue 26-01-21 11:45:57, Sascha Hauer wrote:
> On Mon, Jan 25, 2021 at 04:45:07PM +0100, Jan Kara wrote:
> > On Mon 25-01-21 09:38:54, Sascha Hauer wrote:
> > > On Fri, Jan 22, 2021 at 05:16:58PM +0000, Christoph Hellwig wrote:
> > > > On Fri, Jan 22, 2021 at 04:15:29PM +0100, Sascha Hauer wrote:
> > > > > This patch introduces the Q_PATH flag to the quotactl cmd argument.
> > > > > When given, the path given in the special argument to quotactl will
> > > > > be the mount path where the filesystem is mounted, instead of a path
> > > > > to the block device.
> > > > > This is necessary for filesystems which do not have a block device as
> > > > > backing store. Particularly this is done for upcoming UBIFS support.
> > > > > 
> > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > 
> > > > I hate overloading quotactl even more.  Why not add a new quotactl_path
> > > > syscall instead?
> > > 
> > > We can probably do that. Honza, what do you think?
> > 
> > Hum, yes, probably it would be cleaner to add a new syscall for this so
> > that we don't overload quotactl(2). I just didn't think of this.
> 
> How should the semantics of that new syscall look like?
> 
> The easiest and most obvious way would be to do it like the quotactl(2)
> and just replace the special argument with a path:
> 
> int quotactl_path(int cmd, const char *path, int id, caddr_t addr);

Yes, that's what I meant.

> If we try adding a new syscall then we could completely redefine the API
> and avoid the shortcomings of the original quotactl(2) if there are any.
> Can you foresee the discussions we end up in? I am afraid I am opening a
> can of worms here.
> OTOH there might be value in keeping the new syscall compatible to the
> existing one, but I don't know how much this argument counts.

That's a good question but also a can of worms as you write :). One obvious
problem with quotactl() is that's it's ioctl-like interface. So we have
several different operations mixed into a single syscall. Currently there
are these operations:

#define Q_SYNC     0x800001     /* sync disk copy of a filesystems quotas */
#define Q_QUOTAON  0x800002     /* turn quotas on */
#define Q_QUOTAOFF 0x800003     /* turn quotas off */
#define Q_GETFMT   0x800004     /* get quota format used on given filesystem */
#define Q_GETINFO  0x800005     /* get information about quota files */
#define Q_SETINFO  0x800006     /* set information about quota files */
#define Q_GETQUOTA 0x800007     /* get user quota structure */
#define Q_SETQUOTA 0x800008     /* set user quota structure */
#define Q_GETNEXTQUOTA 0x800009 /* get disk limits and usage >= ID */
<plus their XFS variants>

In a puristic world they'd be 9 different syscalls ... or somewhat less
because Q_GETNEXTQUOTA is a superset of Q_GETQUOTA, we could drop Q_SYNC
and Q_GETFMT because they have dubious value these days so we'd be left
with 6. I don't have a strong opinion whether 6 syscalls are worth the
cleanliness or whether we should go with just one new quotactl_path()
syscall. I've CCed linux-api in case other people have opinion.

Anyway, even if we go with single quotactl_path() syscall we should remove
the duplication between VFS and XFS quotactls when we are creating a new
syscall. Thoughts?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/