On Wed, Jan 17, 2024 at 06:59:54PM +0100, Jan Kara wrote:
> On Tue 09-01-24 14:46:05, cem@xxxxxxxxxx wrote:
> > From: Carlos Maiolino <cem@xxxxxxxxxx>
> >
> > To achieve so, add a new function handle_quota() to the quotaio subsystem,
> > this will call do_quotactl() with or without a valid quotadev, according to the
> > filesystem type.
> >
> > Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
>
> Thanks for the patch. Some comments bewow.
>
> > diff --git a/quotaio.c b/quotaio.c
> > index 9bebb5e..3cc2bb7 100644
> > --- a/quotaio.c
> > +++ b/quotaio.c
> > @@ -34,6 +34,22 @@ struct disk_dqheader {
> > u_int32_t dqh_version;
> > } __attribute__ ((packed));
> >
> > +int handle_quota(int cmd, struct quota_handle *h, int id, void *addr)
>
> Call this quotactl_handle()?
>
> > +{
> > + int err = -EINVAL;
> > +
> > + if (!h)
> > + return err;
> > +
> > + if (!strcmp(h->qh_fstype, MNTTYPE_TMPFS))
> > + err = do_quotactl(QCMD(cmd, h->qh_type), NULL, h->qh_dir,
> > + id, addr);
> > + else
> > + err = do_quotactl(QCMD(cmd, h->qh_type), h->qh_quotadev,
> > + h->qh_dir, id, addr);
> > +
> > + return err;
> > +}
>
> ...
>
> > diff --git a/quotasys.c b/quotasys.c
> > index 903816b..1f66302 100644
> > --- a/quotasys.c
> > +++ b/quotasys.c
> > @@ -1384,7 +1390,11 @@ alloc:
> > continue;
> > }
> >
> > - if (!nfs_fstype(mnt->mnt_type)) {
> > + /*
> > + * If devname and mnt->mnt_fsname matches, there is no real
> > + * underlyin device, so skip these checks
> > + */
> > + if (!nfs_fstype(mnt->mnt_type) && strcmp(devname, mnt->mnt_fsname)) {
> > if (stat(devname, &st) < 0) { /* Can't stat mounted device? */
> > errstr(_("Cannot stat() mounted device %s: %s\n"), devname, strerror(errno));
> > free((char *)devname);
>
> I'm a bit uneasy about the added check because using device name the same
> as filesystem name is just a common agreement but not enforced in any way.
> So perhaps just add an explicit check for tmpfs? Later we can generalize
> this if there are more filesystems like this...
What about adding a new tmpfs_fstype() helper, to mimic nfs_fstype, and use it
here? like:
if (!nfs_fstype(mnt->mnt_type) && tmpfs_fstype(mnt->mnt_type))) {
/* skipe S_ISBLK && S_ISCHR checks */
}
We could open code !strcmp(mnt->mnt_type, MNTTYPE_TMPFS), but it seems to me
adding a new tmpfs_fstype() helper is easier on the eyes, and also OCFS2 does
something similar.
Perhaps that's exactly what you meant as having an explicit check for tmpfs, but
I'm not really sure?!
Carlos
>
> Honza
> --
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
