Thread (1 message) 1 message, 1 author, 2012-08-27

Re: [REVIEW][PATCH 13/15] userns: Add basic quota support

From: Eric W. Biederman <hidden>
Date: 2012-08-27 15:54:21
Also in: linux-fsdevel, linux-xfs, lkml

Jan Kara [off-list ref] writes:
  Hello,

On Sat 25-08-12 17:05:35, Eric W. Biederman wrote:
quoted
Two helper are added dqgetusr and dqgetgrp to allow the quota
infrastructure to be called with a kuid and a kgid respectively.  This
creates type safe variants of dqget and leads to shorter more
comprehensible code.
  It would look more comprehensible to me to have functions like:
kuid2qown() and kgid2qown() and then call dqget(sb, kuid2qown(attr->uid))
(see below for qown_t change proposal). The code then at the first look
explains what is going on... Hmm?
quoted
Place the USRQUOTA and GRPQUOTA defines into enum quota_type.  This
brings with it the ability for the compiler to check that switch
statements handle every quota type, and the ability to mark which
values store the type of a quota entry.
  OK, makes sense.
quoted
Add the data type qown_t a union of kuid_t and kgid_t. qown_t is a
replacement for the implicit union of uid and gid stored in an
unsigned int that is was used in the quota data structures.  Making
the data type explicit allows the kuid_t and kgid_t type safety to
propogate more thoroughly through the code, revealing more places
where uid/gid conversions need be made.
  Hum, when we already do this, wouldn't it make more sense to embed quota
type in qown_t? Because with the union thing you have no meaningful way of
accessing that type without having quota type anyway. So having that in a
single structure makes a lot of sense, plus it makes prototypes shorter...
And you have to call make_qown() anyway...
So I think there was a reason for having the type separate but I'm not
seeing that reason off the top of my head.

I think it was filesystems like gfs2 doing weird things.  But I tell
you want I will play with this and if I can't reproduce find a reason
for putting them in one structure I will because there are definitely
good reasons for doing that.

quoted
Allong with the data type qown_t comes the helper functions
qown_eq, from_qown, from_qown_munged, qown_valid, and make_qown.

Update struct dquot dq_id to be a qown_t.

Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
and dquot_set_dqblk to use enum quota_type and qown_t.

Make minimal changes to gfs2, ocfs2, and xfs to deal with the change
in quota structures and signatures.  The ocfs2 changes are larger than
most because of the extensive tracing throughout the ocfs2 quota code
that prints out dq_id.
  Otherwise the changes look OK to me, although I didn't check them in
detail yet (as above suggestions will change the code anyway).

								Honza
Thanks, 

Eric
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help