Re: [PATCH 2/7] quota: add new quotactl Q_XGETNEXTQUOTA

Jan Kara
2016-01-26T20:40:09Z

On Tue 26-01-16 12:39:59, Eric Sandeen wrote:
> On 1/26/16 11:52 AM, Jan Kara wrote:
>
> > Yeah, so VFS quotas use ESRCH when quota for particular fs is not enabled
> > (while ENOENT means device you passed in doesn't exist).
>
> And what does it return for "that ID has no quota?" Anything? Maybe not,
> see below?
>
> > So probably a
> > solution that keeps XFS and VFS interfaces most selfconsistent is to return
> > ENOENT from Q_XGETNEXTQUOTA and ESRCH from Q_GETNEXTQUOTA. I'll update your
> > patches in this sense in the comments and changelogs. But XFS patches
> > (which I don't carry) need updating the actual code...
>
> Actually, ok, now I'm a little more confused.
>
> Today, Q_XGETQUOTA and Q_GETQUOTA will both return -ENOENT on XFS if a
> quota doesn't exist for that ID:
>
> quotactl(Q_GETQUOTA|USRQUOTA, "/dev/sdb2", 123456789, 0x7ffdf2edbc80) = -1 ENOENT (No such file or directory)
> quotactl(Q_XGETQUOTA|USRQUOTA, "/dev/sdb2", 123456789, 0x7ffdf2edbc10) = -1 ENOENT (No such file or directory)
>
> It seems like the *NEXT* variants (on xfs at least?) may as well continue
> to do the same...
>
> But on ext4, I see 0 returned for a nonexistent quota:
>
> quotactl(Q_GETQUOTA|USRQUOTA, "/dev/sdb1", 123456789, {bhardlimit=0, bsoftlimit=0, curspace=0, ihardlimit=0, isoftlimit=0, curinodes=0, ...}) = 0
> quotactl(Q_XGETQUOTA|USRQUOTA, "/dev/sdb1", 123456789, {version=1, flags=XFS_USER_QUOTA, fieldmask=0, id=123456789, blk_hardlimit=0, blk_softlimit=0, ino_hardlimit=0, ino_softlimit=0, bcount=0, icount=0, ...}) = 0
>
> So the difference doesn't seem to be XGETQUOTA vs GETQUOTA, rather it's
> the filesystem handling the call?

Yes, it's about whether fs/quota/dquot.c or XFS handles the call. VFS
traditionally returns structure full of zeros when you call Q_GETQUOTA for
non-existent ID on a system that otherwise supports quotas (that is a
historical heritage of old quota file format). This is however
impractical for Q_GETNEXTQUOTA so there we have to define some error
terminating the iteration.

> Still, we do need a way to pass back "No more quotas to find" from
> Q_[X]GETNEXTQUOTA; XFS will do -ENOENT, but if -ENOENT and -ESRCH already
> have specific meanings on non-xfs filesystems, I'm not sure where we go
> from there.

Yeah, we have to simply overload one of the error codes for VFS
implementation and I'm not strongly convinced which one. But since you
pointed out that it's not really about the interface but about backing fs,
I think making both ENOENT is probably going to be less confusing for
userspace.

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