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@xxxxxxxxxxxxxx>
>
> 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?
>
> > +static struct super_block *quotactl_sb(dev_t dev, int cmd)
> > {
> > struct super_block *sb;
> > bool excl = false, thawed = false;
> >
> > if (quotactl_cmd_onoff(cmd)) {
> > excl = true;
> > @@ -901,12 +887,50 @@ static struct super_block *quotactl_block(const char __user *special, int cmd)
> > goto retry;
> > }
> > return sb;
> > +}
> > +
> > +/*
> > + * look up a superblock on which quota ops will be performed
> > + * - use the name of a block device to find the superblock thereon
> > + */
> > +static struct super_block *quotactl_block(const char __user *special, int cmd)
> > +{
> > +#ifdef CONFIG_BLOCK
> > + struct filename *tmp = getname(special);
> > + int error;
> > + dev_t dev;
> >
> > + if (IS_ERR(tmp))
> > + return ERR_CAST(tmp);
> > + error = lookup_bdev(tmp->name, &dev);
> > + putname(tmp);
> > + if (error)
> > + return ERR_PTR(error);
> > +
> > + return quotactl_sb(dev, cmd);
> > #else
> > return ERR_PTR(-ENODEV);
> > #endif
>
> Normal kernel style would be to keep the ifdef entirely outside the
> function.
It has been like this before, so changing this should be done in an
extra patch. Not sure if it's worth it though.
>
> > +static struct super_block *quotactl_path(const char __user *special, int cmd)
> > +{
> > + struct super_block *sb;
> > + struct path path;
> > + int error;
> > +
> > + error = user_path_at(AT_FDCWD, special, LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT,
> > + &path);
>
> This adds an overly long line.
ok, will change.
>
> > + if (error)
> > + return ERR_PTR(error);
> > +
> > + sb = quotactl_sb(path.mnt->mnt_sb->s_dev, cmd);
>
> I think quotactl_sb should take the superblock directly. This will
> need a little refactoring of user_get_super, but will lead to much
> better logic.
What do you mean by "take"? Take the superblock as an argument to
quotactl_sb() or take a reference to the superblock?
Sorry, I don't really get where you aiming at.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
