Re: [PATCH 3/3] Enable support for tmpfs quotas

[Jan Kara <jack@suse.cz>]

On Tue 09-01-24 14:46:05, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> 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@redhat.com>

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

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