Re: [PATCH] userns: Add basic quota support v4

2012-09-19 Thread Eric W. Biederman

Dave thank you earlier taking some time to do review.  It made me
realize that my code was not as mature as it needed to be.

That said you missed a lot of important details, and I will aim at
to address some of the highlights.

Precedent in naming.  that has been merged since 3.5, earlier
there is make_pte.   Much of your review sounds like you are unhappy
with the general infrastructure that was built for user namespaces,
and already merged.  Short of an overwhelmingly better suggestion
at this point I am not going to revisit naming.

Passing structures by value is a core part of the design and a necessity
for strong type safety in C, and again already in common use in the
kernel.

Using BUG on code paths that can not be triggered, simply means that
in that rare and unlikely event there is a clear trace to follow,
and xfs already has plenty of invocations of BUG, some of which
I managed to trigger with the xfs test suite.  So I can not find
much sense in your arguments about being clear that something is a BUG.

Your comments on where xfs uses projid are just laughable.

>> So I am inclined to rename this field projid, but I don't know if it is
>> worth respinning the patch for something so trivial.
>
> I'd like to think you are joking, given that the patch series is all
> about using consistent, verifiable identifiers... :/

The patch series is all about implementing the user namespace.

To make it hard to miss places where conversions need to between the
internal kernel form and the form stored in filesystems or the form that
userspace uses I use types that are not assignment compatible.  Pushing
the types deeper in the code keeps me honest and let's me find cases
I was not expecting like the ioctl interface of xfs that largely
duplicates much of the vfs functionality.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] userns: Add basic quota support v4

2012-09-19 Thread Eric W. Biederman

Dave thank you earlier taking some time to do review.  It made me
realize that my code was not as mature as it needed to be.

That said you missed a lot of important details, and I will aim at
to address some of the highlights.

Precedent in naming. uidgid.h that has been merged since 3.5, earlier
there is make_pte.   Much of your review sounds like you are unhappy
with the general infrastructure that was built for user namespaces,
and already merged.  Short of an overwhelmingly better suggestion
at this point I am not going to revisit naming.

Passing structures by value is a core part of the design and a necessity
for strong type safety in C, and again already in common use in the
kernel.

Using BUG on code paths that can not be triggered, simply means that
in that rare and unlikely event there is a clear trace to follow,
and xfs already has plenty of invocations of BUG, some of which
I managed to trigger with the xfs test suite.  So I can not find
much sense in your arguments about being clear that something is a BUG.

Your comments on where xfs uses projid are just laughable.

 So I am inclined to rename this field projid, but I don't know if it is
 worth respinning the patch for something so trivial.

 I'd like to think you are joking, given that the patch series is all
 about using consistent, verifiable identifiers... :/

The patch series is all about implementing the user namespace.

To make it hard to miss places where conversions need to between the
internal kernel form and the form stored in filesystems or the form that
userspace uses I use types that are not assignment compatible.  Pushing
the types deeper in the code keeps me honest and let's me find cases
I was not expecting like the ioctl interface of xfs that largely
duplicates much of the vfs functionality.

Eric
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] userns: Add basic quota support v4

2012-09-04 Thread Eric W. Biederman


Dave Chinner  writes:

> On Wed, Aug 29, 2012 at 02:31:26AM -0700, Eric W. Biederman wrote:
>> 
>> Dave thanks for taking the time to take a detailed look at this code.
>> 
>> Dave Chinner  writes:
>> 
>> > On Tue, Aug 28, 2012 at 12:09:56PM -0700, Eric W. Biederman wrote:



>> > How did you test that this all works?
>> 
>> By making it a compile error if you get a conversion wrong and making it
>> a rule not to make any logic changes.
>>
>> That combined with code review
>> and running the code a bit to make certain I did not horribly mess up.
>
> But no actual regression testing. You're messing with code that I
> will have to triage when it goes wrong for a user, so IMO your code
> has to pass the same bar as the code I write has to pass for review
> - please regression test your code and write new regression tests
> for new functionality.

I like the idea of regression tests.  In practice and also with xfstests
I find that I spend lots of time debugging and fixing and improving
tests and at the end of the day I find regression tests tell me
very little.

But I did figure I should give them a try since I have a rather
substantial xfs patch in my queue.

I added tests 111 and 232 to the expunged file because the don't
run to completion.

ltp/rwtest.sh needs to be run with #!/bin/bash instead of #!/bin/sh as
it contains serveral bashisms.

You need to have gawk installed instead of mawk because of a non-posix
call to asort somewhere in the test framework.

On my branch userns-always-map-user-v53 or on v3.6-rc1+
xfs: check for possible overflow in xfs_ioc_trim
xfs: unlock the AGI buffer when looping in xfs_dialloc
xfs: fix uninitialised variable in xfs_rtbuf_get()

When I run ./check in the from xfstests I get

Tue Sep  4 05:06:12 PDT 2012
001 002 003 004 005 006 007 008 009 010 011 012 013 014 015 016 017 018
019 020 021 022 023 024 025 026 027 028 029 030 031 032 033 034 035 036
037 038 039 040 041 042 043 044 045 046 047 048 049 050 051 052 053 054
055 056 057 058 059 060 061 062 063 064 065 066 067 068 069 070 071 072
073 074 075 076 077 078 079 080 081 082 083 084 085 086 087 088 089 090
091 092 093 094 095 096 097 098 099 100 101 102 103 104 105 106 107 108
109 110 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127
128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145
146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163
164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181
182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199
200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217
218 219 220 221 222 223 224 225 226 227 228 229 230 231 233 234 235 236
237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254
255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272
273 274 275 276 277 278 279 280 281 282 283 284 285 286
Not run:2
Failures: 018 081 082 106 107 136 167 171 206 219 229 234 250 280
Failed 14 of 165 tests

But since the results came back the same either way I think the tests
told me all they can.  The 14 failed tests and 1 bug don't seem to say
good things about xfs in general though.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] userns: Add basic quota support v4

2012-09-04 Thread Eric W. Biederman


Dave Chinner da...@fromorbit.com writes:

 On Wed, Aug 29, 2012 at 02:31:26AM -0700, Eric W. Biederman wrote:
 
 Dave thanks for taking the time to take a detailed look at this code.
 
 Dave Chinner da...@fromorbit.com writes:
 
  On Tue, Aug 28, 2012 at 12:09:56PM -0700, Eric W. Biederman wrote:



  How did you test that this all works?
 
 By making it a compile error if you get a conversion wrong and making it
 a rule not to make any logic changes.

 That combined with code review
 and running the code a bit to make certain I did not horribly mess up.

 But no actual regression testing. You're messing with code that I
 will have to triage when it goes wrong for a user, so IMO your code
 has to pass the same bar as the code I write has to pass for review
 - please regression test your code and write new regression tests
 for new functionality.

I like the idea of regression tests.  In practice and also with xfstests
I find that I spend lots of time debugging and fixing and improving
tests and at the end of the day I find regression tests tell me
very little.

But I did figure I should give them a try since I have a rather
substantial xfs patch in my queue.

I added tests 111 and 232 to the expunged file because the don't
run to completion.

ltp/rwtest.sh needs to be run with #!/bin/bash instead of #!/bin/sh as
it contains serveral bashisms.

You need to have gawk installed instead of mawk because of a non-posix
call to asort somewhere in the test framework.

On my branch userns-always-map-user-v53 or on v3.6-rc1+
xfs: check for possible overflow in xfs_ioc_trim
xfs: unlock the AGI buffer when looping in xfs_dialloc
xfs: fix uninitialised variable in xfs_rtbuf_get()

When I run ./check in the from xfstests I get

Tue Sep  4 05:06:12 PDT 2012
001 002 003 004 005 006 007 008 009 010 011 012 013 014 015 016 017 018
019 020 021 022 023 024 025 026 027 028 029 030 031 032 033 034 035 036
037 038 039 040 041 042 043 044 045 046 047 048 049 050 051 052 053 054
055 056 057 058 059 060 061 062 063 064 065 066 067 068 069 070 071 072
073 074 075 076 077 078 079 080 081 082 083 084 085 086 087 088 089 090
091 092 093 094 095 096 097 098 099 100 101 102 103 104 105 106 107 108
109 110 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127
128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145
146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163
164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181
182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199
200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217
218 219 220 221 222 223 224 225 226 227 228 229 230 231 233 234 235 236
237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254
255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272
273 274 275 276 277 278 279 280 281 282 283 284 285 286
Not run:2
Failures: 018 081 082 106 107 136 167 171 206 219 229 234 250 280
Failed 14 of 165 tests

But since the results came back the same either way I think the tests
told me all they can.  The 14 failed tests and 1 bug don't seem to say
good things about xfs in general though.

Eric
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] userns: Add basic quota support v4

2012-08-30 Thread Dave Chinner
On Wed, Aug 29, 2012 at 02:31:26AM -0700, Eric W. Biederman wrote:
> 
> Dave thanks for taking the time to take a detailed look at this code.
> 
> Dave Chinner  writes:
> 
> > On Tue, Aug 28, 2012 at 12:09:56PM -0700, Eric W. Biederman wrote:
> >> 
> >> Add the data type struct kqid which holds the kernel internal form of
> >> the owning identifier of a quota.  struct kqid is a replacement for
> >> the implicit union of uid, gid and project stored in an unsigned int
> >> and the quota type field 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.
> >> 
> >> Along with the data type struct kqid comes the helper functions
> >> qid_eq, qid_lt, from_kqid, from_kqid_munged, qid_valid, make_kqid,
> >
> > I think Jan's comment about from_kqid being named id_from_kgid is
> > better, though I also think it would read better as kqid_to_id().
> > ie:
> >
> > id = kqid_to_id(ns, qid);
> 
> kqid and qid are the same thing just in a different encoding.
> Emphasizing the quota identifier instead of the kernel vs user encoding
> change is paying attention to the wrong thing.

Not from a quota perspective. The only thing the quota code really
cares about is the quota identifier, not the encoding.

Fundamentally, from_kqid() doen't tell me anything about what I'm
getting from the kqid. There's code all over the place that used the
"_to_" convention because it's obvious what is
being converted from/to. e.g. cpu_to_beXX, compat_to_ptr,
dma_to_phys, pfn_to_page, etc.  Best practises say "follow existing
conventions".

> Using make_kqid and from_kqid follows the exact same conventions as I have
> established for kuids and kgids.  So if you learn one you have learned
> them all.

For those of us that have to look at it once every few months,
following the same conventions as all the other code in the kernel
(i.e. kqid_to_id()) tells me everything I need to know without
having to go through the process of looking up the unusual
from_kqid() function and then from_kuid() to find out what it is
actually doing

> >> make_kqid_invalid, make_kqid_uid, make_kqid_gid.
> >
> > and these named something like uid_to_kqid()
> 
> The last two are indeed weird, and definitely not the common case,
> since there is no precedent I can almost see doing something different
> but I don't see a good case for a different name.

There's plenty of precendence in other code that converts format.
A very common convention that is used everywhere is DEFINE_...().
That would be make the code easier to grasp than "make...".

> >> Change struct dquot dq_id to a struct kqid and remove the now
> >> unecessary dq_type.
> >> 
> >> Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
> >> and dquot_set_dqblk to use struct kqid.
> >> 
> >> Make minimal changes to ext3, ext4, 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.
> >
> > How did you test that this all works?
> 
> By making it a compile error if you get a conversion wrong and making it
> a rule not to make any logic changes.
>
> That combined with code review
> and running the code a bit to make certain I did not horribly mess up.

But no actual regression testing. You're messing with code that I
will have to triage when it goes wrong for a user, so IMO your code
has to pass the same bar as the code I write has to pass for review
- please regression test your code and write new regression tests
for new functionality.

> > e.g. run xfstests -g quota on
> > each of those filesystems and check for no regressions? And if you
> > wrote any tests, can you convert them to be part of xfstests so that
> > namespace aware quotas get tested regularly?
> 
> I have not written any tests, and running the xfstests in a namespace
> should roughly be a matter of "unshare -U xfstest -g quota"  It isn't
> quite that easy because  /proc/self/uid_map and /proc/self/gid_map need

Asking people to run the entire regression test suite differently
and with special setup magic won't get the code tested regularly.
Writing a new, self contained test that exercises quota in multiple
namespaces simultaneously is what is needed - that way people who
don't even know that namespaces exist will be regression testing
it...

> >> --- a/include/linux/quota.h
> >> +++ b/include/linux/quota.h
> >> @@ -181,10 +181,161 @@ enum {
> >>  #include 
> >>  
> >>  #include 
> >> +#include 
> >>  
> >>  typedef __kernel_uid32_t qid_t; /* Type in which we store ids in memory */
> >>  typedef long long qsize_t;/* Type in which we store sizes */
> >
> > From fs/xfs/xfs_types.h:
> >
> > typedef __uint32_t  prid_t; /* project ID */
> >
> > Perhaps it 

Re: [PATCH] userns: Add basic quota support v4

2012-08-30 Thread Dave Chinner
On Wed, Aug 29, 2012 at 02:31:26AM -0700, Eric W. Biederman wrote:
 
 Dave thanks for taking the time to take a detailed look at this code.
 
 Dave Chinner da...@fromorbit.com writes:
 
  On Tue, Aug 28, 2012 at 12:09:56PM -0700, Eric W. Biederman wrote:
  
  Add the data type struct kqid which holds the kernel internal form of
  the owning identifier of a quota.  struct kqid is a replacement for
  the implicit union of uid, gid and project stored in an unsigned int
  and the quota type field 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.
  
  Along with the data type struct kqid comes the helper functions
  qid_eq, qid_lt, from_kqid, from_kqid_munged, qid_valid, make_kqid,
 
  I think Jan's comment about from_kqid being named id_from_kgid is
  better, though I also think it would read better as kqid_to_id().
  ie:
 
  id = kqid_to_id(ns, qid);
 
 kqid and qid are the same thing just in a different encoding.
 Emphasizing the quota identifier instead of the kernel vs user encoding
 change is paying attention to the wrong thing.

Not from a quota perspective. The only thing the quota code really
cares about is the quota identifier, not the encoding.

Fundamentally, from_kqid() doen't tell me anything about what I'm
getting from the kqid. There's code all over the place that used the
format_to_other format convention because it's obvious what is
being converted from/to. e.g. cpu_to_beXX, compat_to_ptr,
dma_to_phys, pfn_to_page, etc.  Best practises say follow existing
conventions.

 Using make_kqid and from_kqid follows the exact same conventions as I have
 established for kuids and kgids.  So if you learn one you have learned
 them all.

For those of us that have to look at it once every few months,
following the same conventions as all the other code in the kernel
(i.e. kqid_to_id()) tells me everything I need to know without
having to go through the process of looking up the unusual
from_kqid() function and then from_kuid() to find out what it is
actually doing

  make_kqid_invalid, make_kqid_uid, make_kqid_gid.
 
  and these named something like uid_to_kqid()
 
 The last two are indeed weird, and definitely not the common case,
 since there is no precedent I can almost see doing something different
 but I don't see a good case for a different name.

There's plenty of precendence in other code that converts format.
A very common convention that is used everywhere is DEFINE_...().
That would be make the code easier to grasp than make

  Change struct dquot dq_id to a struct kqid and remove the now
  unecessary dq_type.
  
  Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
  and dquot_set_dqblk to use struct kqid.
  
  Make minimal changes to ext3, ext4, 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.
 
  How did you test that this all works?
 
 By making it a compile error if you get a conversion wrong and making it
 a rule not to make any logic changes.

 That combined with code review
 and running the code a bit to make certain I did not horribly mess up.

But no actual regression testing. You're messing with code that I
will have to triage when it goes wrong for a user, so IMO your code
has to pass the same bar as the code I write has to pass for review
- please regression test your code and write new regression tests
for new functionality.

  e.g. run xfstests -g quota on
  each of those filesystems and check for no regressions? And if you
  wrote any tests, can you convert them to be part of xfstests so that
  namespace aware quotas get tested regularly?
 
 I have not written any tests, and running the xfstests in a namespace
 should roughly be a matter of unshare -U xfstest -g quota  It isn't
 quite that easy because  /proc/self/uid_map and /proc/self/gid_map need

Asking people to run the entire regression test suite differently
and with special setup magic won't get the code tested regularly.
Writing a new, self contained test that exercises quota in multiple
namespaces simultaneously is what is needed - that way people who
don't even know that namespaces exist will be regression testing
it...

  --- a/include/linux/quota.h
  +++ b/include/linux/quota.h
  @@ -181,10 +181,161 @@ enum {
   #include linux/dqblk_v2.h
   
   #include linux/atomic.h
  +#include linux/uidgid.h
   
   typedef __kernel_uid32_t qid_t; /* Type in which we store ids in memory */
   typedef long long qsize_t;/* Type in which we store sizes */
 
  From fs/xfs/xfs_types.h:
 
  typedef __uint32_t  prid_t; /* project ID */
 
  Perhaps it would be better to have an official kprid_t definition
  here, i.e:
 
 We can always 

Re: [PATCH] userns: Add basic quota support v4

2012-08-29 Thread Eric W. Biederman

Dave thanks for taking the time to take a detailed look at this code.

Dave Chinner  writes:

> On Tue, Aug 28, 2012 at 12:09:56PM -0700, Eric W. Biederman wrote:
>> 
>> Add the data type struct kqid which holds the kernel internal form of
>> the owning identifier of a quota.  struct kqid is a replacement for
>> the implicit union of uid, gid and project stored in an unsigned int
>> and the quota type field 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.
>> 
>> Along with the data type struct kqid comes the helper functions
>> qid_eq, qid_lt, from_kqid, from_kqid_munged, qid_valid, make_kqid,
>
> I think Jan's comment about from_kqid being named id_from_kgid is
> better, though I also think it would read better as kqid_to_id().
> ie:
>
>   id = kqid_to_id(ns, qid);

kqid and qid are the same thing just in a different encoding.
Emphasizing the quota identifier instead of the kernel vs user encoding
change is paying attention to the wrong thing.

Using make_kqid and from_kqid follows the exact same conventions as I have
established for kuids and kgids.  So if you learn one you have learned
them all.

>> make_kqid_invalid, make_kqid_uid, make_kqid_gid.
>
> and these named something like uid_to_kqid()

The last two are indeed weird, and definitely not the common case,
since there is no precedent I can almost see doing something different
but I don't see a good case for a different name.

>> Change struct dquot dq_id to a struct kqid and remove the now
>> unecessary dq_type.
>> 
>> Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
>> and dquot_set_dqblk to use struct kqid.
>> 
>> Make minimal changes to ext3, ext4, 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.
>
> How did you test that this all works?

By making it a compile error if you get a conversion wrong and making it
a rule not to make any logic changes.  That combined with code review
and running the code a bit to make certain I did not horribly mess up.

> e.g. run xfstests -g quota on
> each of those filesystems and check for no regressions? And if you
> wrote any tests, can you convert them to be part of xfstests so that
> namespace aware quotas get tested regularly?

I have not written any tests, and running the xfstests in a namespace
should roughly be a matter of "unshare -U xfstest -g quota"  It isn't
quite that easy because  /proc/self/uid_map and /proc/self/gid_map need
to be written first.

Right now only ext2 ext3 and ext4 compile with user namespace support
enabled.  xfs and gfs2 and ocfs2 still only compile with user namespace
support disabled because my patches to convert them are waiting in the
wings until I get the core subsystems primarily quota and posix acls
reviewed.

>> v4:
>>   - Rename struct qown struct kqid and associated changes
>> to the naming of the helper functions.
>>   - Use qid_t to hold the userspace identifier representation
>> of quota identifiers in all new code.
>> v3:
>>   - Add missing negation on qown_valid
>> v2:
>>   - Renamed qown_t struct qown
>>   - Added the quota type to struct qown.
>>   - Removed enum quota_type (In this patch it was just noise)
>>   - Added qown_lt, make_qown_invalid, make_qown_uid, make_qown_gid
>>   - Taught qown to handle xfs project ids (but only in init_user_ns).
>> Q_XGETQUOTA calls .get_quotblk with project ids.
>
> Q_XSETQLIM was modified to handle project quotas as well, I assume?

I didn't break the project quota support for Q_XSETQLIM.


>> index fed504f..96944c0 100644
>> --- a/fs/xfs/xfs_quotaops.c
>> +++ b/fs/xfs/xfs_quotaops.c
>> @@ -97,28 +97,29 @@ xfs_fs_set_xstate(
>>  STATIC int
>>  xfs_fs_get_dqblk(
>>  struct super_block  *sb,
>> -int type,
>> -qid_t   id,
>> +struct kqid qid,
>>  struct fs_disk_quota*fdq)
>>  {
>>  struct xfs_mount*mp = XFS_M(sb);
>> +xfs_dqid_t  xfs_id;
>>  
>>  if (!XFS_IS_QUOTA_RUNNING(mp))
>>  return -ENOSYS;
>>  if (!XFS_IS_QUOTA_ON(mp))
>>  return -ESRCH;
>>  
>> -return -xfs_qm_scall_getquota(mp, id, xfs_quota_type(type), fdq);
>> +xfs_id = from_kqid(_user_ns, qid);
>> +return -xfs_qm_scall_getquota(mp, xfs_id, xfs_quota_type(qid.type), 
>> fdq);
>>  }
>
> Why a temporary variable? Why not just:
>
>   return -xfs_qm_scall_getquota(mp, from_kqid(_user_ns, qid),
> xfs_quota_type(qid.type), fdq);
>

Because I am not fond of very long lines and because I did the basic
conversion of gfs2 first where using a separate variable a larger
difference.

>From a code review 

Re: [PATCH] userns: Add basic quota support v4

2012-08-29 Thread Eric W. Biederman

Dave thanks for taking the time to take a detailed look at this code.

Dave Chinner da...@fromorbit.com writes:

 On Tue, Aug 28, 2012 at 12:09:56PM -0700, Eric W. Biederman wrote:
 
 Add the data type struct kqid which holds the kernel internal form of
 the owning identifier of a quota.  struct kqid is a replacement for
 the implicit union of uid, gid and project stored in an unsigned int
 and the quota type field 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.
 
 Along with the data type struct kqid comes the helper functions
 qid_eq, qid_lt, from_kqid, from_kqid_munged, qid_valid, make_kqid,

 I think Jan's comment about from_kqid being named id_from_kgid is
 better, though I also think it would read better as kqid_to_id().
 ie:

   id = kqid_to_id(ns, qid);

kqid and qid are the same thing just in a different encoding.
Emphasizing the quota identifier instead of the kernel vs user encoding
change is paying attention to the wrong thing.

Using make_kqid and from_kqid follows the exact same conventions as I have
established for kuids and kgids.  So if you learn one you have learned
them all.

 make_kqid_invalid, make_kqid_uid, make_kqid_gid.

 and these named something like uid_to_kqid()

The last two are indeed weird, and definitely not the common case,
since there is no precedent I can almost see doing something different
but I don't see a good case for a different name.

 Change struct dquot dq_id to a struct kqid and remove the now
 unecessary dq_type.
 
 Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
 and dquot_set_dqblk to use struct kqid.
 
 Make minimal changes to ext3, ext4, 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.

 How did you test that this all works?

By making it a compile error if you get a conversion wrong and making it
a rule not to make any logic changes.  That combined with code review
and running the code a bit to make certain I did not horribly mess up.

 e.g. run xfstests -g quota on
 each of those filesystems and check for no regressions? And if you
 wrote any tests, can you convert them to be part of xfstests so that
 namespace aware quotas get tested regularly?

I have not written any tests, and running the xfstests in a namespace
should roughly be a matter of unshare -U xfstest -g quota  It isn't
quite that easy because  /proc/self/uid_map and /proc/self/gid_map need
to be written first.

Right now only ext2 ext3 and ext4 compile with user namespace support
enabled.  xfs and gfs2 and ocfs2 still only compile with user namespace
support disabled because my patches to convert them are waiting in the
wings until I get the core subsystems primarily quota and posix acls
reviewed.

 v4:
   - Rename struct qown struct kqid and associated changes
 to the naming of the helper functions.
   - Use qid_t to hold the userspace identifier representation
 of quota identifiers in all new code.
 v3:
   - Add missing negation on qown_valid
 v2:
   - Renamed qown_t struct qown
   - Added the quota type to struct qown.
   - Removed enum quota_type (In this patch it was just noise)
   - Added qown_lt, make_qown_invalid, make_qown_uid, make_qown_gid
   - Taught qown to handle xfs project ids (but only in init_user_ns).
 Q_XGETQUOTA calls .get_quotblk with project ids.

 Q_XSETQLIM was modified to handle project quotas as well, I assume?

I didn't break the project quota support for Q_XSETQLIM.


 index fed504f..96944c0 100644
 --- a/fs/xfs/xfs_quotaops.c
 +++ b/fs/xfs/xfs_quotaops.c
 @@ -97,28 +97,29 @@ xfs_fs_set_xstate(
  STATIC int
  xfs_fs_get_dqblk(
  struct super_block  *sb,
 -int type,
 -qid_t   id,
 +struct kqid qid,
  struct fs_disk_quota*fdq)
  {
  struct xfs_mount*mp = XFS_M(sb);
 +xfs_dqid_t  xfs_id;
  
  if (!XFS_IS_QUOTA_RUNNING(mp))
  return -ENOSYS;
  if (!XFS_IS_QUOTA_ON(mp))
  return -ESRCH;
  
 -return -xfs_qm_scall_getquota(mp, id, xfs_quota_type(type), fdq);
 +xfs_id = from_kqid(init_user_ns, qid);
 +return -xfs_qm_scall_getquota(mp, xfs_id, xfs_quota_type(qid.type), 
 fdq);
  }

 Why a temporary variable? Why not just:

   return -xfs_qm_scall_getquota(mp, from_kqid(init_user_ns, qid),
 xfs_quota_type(qid.type), fdq);


Because I am not fond of very long lines and because I did the basic
conversion of gfs2 first where using a separate variable a larger
difference.

From a code review perspective I am still more comfortable with
introducing a temporary variable as the fundamental change is
easier to see.

 

Re: [PATCH] userns: Add basic quota support v4

2012-08-28 Thread Dave Chinner
On Tue, Aug 28, 2012 at 12:09:56PM -0700, Eric W. Biederman wrote:
> 
> Add the data type struct kqid which holds the kernel internal form of
> the owning identifier of a quota.  struct kqid is a replacement for
> the implicit union of uid, gid and project stored in an unsigned int
> and the quota type field 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.
> 
> Along with the data type struct kqid comes the helper functions
> qid_eq, qid_lt, from_kqid, from_kqid_munged, qid_valid, make_kqid,

I think Jan's comment about from_kqid being named id_from_kgid is
better, though I also think it would read better as kqid_to_id().
ie:

id = kqid_to_id(ns, qid);

> make_kqid_invalid, make_kqid_uid, make_kqid_gid.

and these named something like uid_to_kqid()

> Change struct dquot dq_id to a struct kqid and remove the now
> unecessary dq_type.
> 
> Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
> and dquot_set_dqblk to use struct kqid.
> 
> Make minimal changes to ext3, ext4, 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.

How did you test that this all works? e.g. run xfstests -g quota on
each of those filesystems and check for no regressions? And if you
wrote any tests, can you convert them to be part of xfstests so that
namespace aware quotas get tested regularly?

> 
> v4:
>   - Rename struct qown struct kqid and associated changes
> to the naming of the helper functions.
>   - Use qid_t to hold the userspace identifier representation
> of quota identifiers in all new code.
> v3:
>   - Add missing negation on qown_valid
> v2:
>   - Renamed qown_t struct qown
>   - Added the quota type to struct qown.
>   - Removed enum quota_type (In this patch it was just noise)
>   - Added qown_lt, make_qown_invalid, make_qown_uid, make_qown_gid
>   - Taught qown to handle xfs project ids (but only in init_user_ns).
> Q_XGETQUOTA calls .get_quotblk with project ids.

Q_XSETQLIM was modified to handle project quotas as well, I assume?

> index fed504f..96944c0 100644
> --- a/fs/xfs/xfs_quotaops.c
> +++ b/fs/xfs/xfs_quotaops.c
> @@ -97,28 +97,29 @@ xfs_fs_set_xstate(
>  STATIC int
>  xfs_fs_get_dqblk(
>   struct super_block  *sb,
> - int type,
> - qid_t   id,
> + struct kqid qid,
>   struct fs_disk_quota*fdq)
>  {
>   struct xfs_mount*mp = XFS_M(sb);
> + xfs_dqid_t  xfs_id;
>  
>   if (!XFS_IS_QUOTA_RUNNING(mp))
>   return -ENOSYS;
>   if (!XFS_IS_QUOTA_ON(mp))
>   return -ESRCH;
>  
> - return -xfs_qm_scall_getquota(mp, id, xfs_quota_type(type), fdq);
> + xfs_id = from_kqid(_user_ns, qid);
> + return -xfs_qm_scall_getquota(mp, xfs_id, xfs_quota_type(qid.type), 
> fdq);
>  }

Why a temporary variable? Why not just:

return -xfs_qm_scall_getquota(mp, from_kqid(_user_ns, qid),
  xfs_quota_type(qid.type), fdq);

Indeed, why not drive the struct kqid down another level into
xfs_qm_scall_getquota() where all they are used for is parameters to
the xfs_qm_dqget() function?

>  
>  STATIC int
>  xfs_fs_set_dqblk(
>   struct super_block  *sb,
> - int type,
> - qid_t   id,
> + struct kqid qid,
>   struct fs_disk_quota*fdq)
>  {
>   struct xfs_mount*mp = XFS_M(sb);
> + xfs_dqid_t  xfs_id;
>  
>   if (sb->s_flags & MS_RDONLY)
>   return -EROFS;
> @@ -127,7 +128,8 @@ xfs_fs_set_dqblk(
>   if (!XFS_IS_QUOTA_ON(mp))
>   return -ESRCH;
>  
> - return -xfs_qm_scall_setqlim(mp, id, xfs_quota_type(type), fdq);
> + xfs_id = from_kqid(_user_ns, qid);
> + return -xfs_qm_scall_setqlim(mp, xfs_id, xfs_quota_type(qid.type), fdq);
>  }

Same is true here

>  
>  const struct quotactl_ops xfs_quotactl_operations = {
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index bcb6054..46de393 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -575,12 +575,14 @@ xfs_quota_warn(
>   struct xfs_dquot*dqp,
>   int type)
>  {
> + int qtype;
> + struct kqid qid;
>   /* no warnings for project quotas - we just return ENOSPC later */
>   if (dqp->dq_flags & XFS_DQ_PROJ)
>   return;
> - quota_send_warning((dqp->dq_flags & XFS_DQ_USER) ? USRQUOTA : GRPQUOTA,
> -be32_to_cpu(dqp->q_core.d_id), mp->m_super->s_dev,
> -type);
> + qtype = (dqp->dq_flags & XFS_DQ_USER) ? USRQUOTA : 

[PATCH] userns: Add basic quota support v4

2012-08-28 Thread Eric W. Biederman

Add the data type struct kqid which holds the kernel internal form of
the owning identifier of a quota.  struct kqid is a replacement for
the implicit union of uid, gid and project stored in an unsigned int
and the quota type field 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.

Along with the data type struct kqid comes the helper functions
qid_eq, qid_lt, from_kqid, from_kqid_munged, qid_valid, make_kqid,
make_kqid_invalid, make_kqid_uid, make_kqid_gid.

Change struct dquot dq_id to a struct kqid and remove the now
unecessary dq_type.

Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
and dquot_set_dqblk to use struct kqid.

Make minimal changes to ext3, ext4, 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.

v4:
  - Rename struct qown struct kqid and associated changes
to the naming of the helper functions.
  - Use qid_t to hold the userspace identifier representation
of quota identifiers in all new code.
v3:
  - Add missing negation on qown_valid
v2:
  - Renamed qown_t struct qown
  - Added the quota type to struct qown.
  - Removed enum quota_type (In this patch it was just noise)
  - Added qown_lt, make_qown_invalid, make_qown_uid, make_qown_gid
  - Taught qown to handle xfs project ids (but only in init_user_ns).
Q_XGETQUOTA calls .get_quotblk with project ids.

Cc: Steven Whitehouse 
Cc: Mark Fasheh 
Cc: Joel Becker 
Cc: Ben Myers 
Cc: Alex Elder 
Cc: Jan Kara 
Cc: Dmitry Monakhov 
Signed-off-by: Eric W. Biederman 
---
 fs/ext3/super.c  |2 +-
 fs/ext4/super.c  |2 +-
 fs/gfs2/quota.c  |   52 +--
 fs/ocfs2/file.c  |6 +-
 fs/ocfs2/quota_global.c  |   43 +++-
 fs/ocfs2/quota_local.c   |   15 +++--
 fs/quota/dquot.c |  116 
 fs/quota/netlink.c   |   10 ++-
 fs/quota/quota.c |   28 ++--
 fs/quota/quota_tree.c|   23 ---
 fs/quota/quota_v1.c  |   12 ++--
 fs/quota/quota_v2.c  |   26 ---
 fs/xfs/xfs_quotaops.c|   14 ++--
 fs/xfs/xfs_trans_dquot.c |8 ++-
 include/linux/quota.h|  162 --
 include/linux/quotaops.h |6 +-
 init/Kconfig |2 -
 17 files changed, 364 insertions(+), 163 deletions(-)

diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index ff9bcdc..73e42f5 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -2814,7 +2814,7 @@ static int ext3_statfs (struct dentry * dentry, struct 
kstatfs * buf)
 
 static inline struct inode *dquot_to_inode(struct dquot *dquot)
 {
-   return sb_dqopt(dquot->dq_sb)->files[dquot->dq_type];
+   return sb_dqopt(dquot->dq_sb)->files[dquot->dq_id.type];
 }
 
 static int ext3_write_dquot(struct dquot *dquot)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d76ec82..78e6036 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4796,7 +4796,7 @@ static int ext4_statfs(struct dentry *dentry, struct 
kstatfs *buf)
 
 static inline struct inode *dquot_to_inode(struct dquot *dquot)
 {
-   return sb_dqopt(dquot->dq_sb)->files[dquot->dq_type];
+   return sb_dqopt(dquot->dq_sb)->files[dquot->dq_id.type];
 }
 
 static int ext4_write_dquot(struct dquot *dquot)
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index a3bde91..e27f8d6 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1057,6 +1057,8 @@ int gfs2_quota_check(struct gfs2_inode *ip, u32 uid, u32 
gid)
 return 0;
 
for (x = 0; x < ip->i_res->rs_qa_qd_num; x++) {
+   int qtype;
+   struct kqid qid;
qd = ip->i_res->rs_qa_qd[x];
 
if (!((qd->qd_id == uid && test_bit(QDF_USER, >qd_flags)) ||
@@ -1068,11 +1070,12 @@ int gfs2_quota_check(struct gfs2_inode *ip, u32 uid, 
u32 gid)
value += qd->qd_change;
spin_unlock(_lru_lock);
 
+   qtype = test_bit(QDF_USER, >qd_flags) ? USRQUOTA : GRPQUOTA;
+   qid = make_kqid(_user_ns, qtype, qd->qd_id);
if (be64_to_cpu(qd->qd_qb.qb_limit) && 
(s64)be64_to_cpu(qd->qd_qb.qb_limit) < value) {
print_message(qd, "exceeded");
-   quota_send_warning(test_bit(QDF_USER, >qd_flags) ?
-  USRQUOTA : GRPQUOTA, qd->qd_id,
-  sdp->sd_vfs->s_dev, 
QUOTA_NL_BHARDWARN);
+   quota_send_warning(qid, sdp->sd_vfs->s_dev,
+  QUOTA_NL_BHARDWARN);
 
error = -EDQUOT;
break;
@@ -1081,9 +1084,8 @@ int gfs2_quota_check(struct gfs2_inode *ip, u32 uid, 

[PATCH] userns: Add basic quota support v4

2012-08-28 Thread Eric W. Biederman

Add the data type struct kqid which holds the kernel internal form of
the owning identifier of a quota.  struct kqid is a replacement for
the implicit union of uid, gid and project stored in an unsigned int
and the quota type field 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.

Along with the data type struct kqid comes the helper functions
qid_eq, qid_lt, from_kqid, from_kqid_munged, qid_valid, make_kqid,
make_kqid_invalid, make_kqid_uid, make_kqid_gid.

Change struct dquot dq_id to a struct kqid and remove the now
unecessary dq_type.

Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
and dquot_set_dqblk to use struct kqid.

Make minimal changes to ext3, ext4, 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.

v4:
  - Rename struct qown struct kqid and associated changes
to the naming of the helper functions.
  - Use qid_t to hold the userspace identifier representation
of quota identifiers in all new code.
v3:
  - Add missing negation on qown_valid
v2:
  - Renamed qown_t struct qown
  - Added the quota type to struct qown.
  - Removed enum quota_type (In this patch it was just noise)
  - Added qown_lt, make_qown_invalid, make_qown_uid, make_qown_gid
  - Taught qown to handle xfs project ids (but only in init_user_ns).
Q_XGETQUOTA calls .get_quotblk with project ids.

Cc: Steven Whitehouse swhit...@redhat.com
Cc: Mark Fasheh mfas...@suse.com
Cc: Joel Becker jl...@evilplan.org
Cc: Ben Myers b...@sgi.com
Cc: Alex Elder el...@kernel.org
Cc: Jan Kara j...@suse.cz
Cc: Dmitry Monakhov dmonak...@openvz.org
Signed-off-by: Eric W. Biederman ebied...@xmission.com
---
 fs/ext3/super.c  |2 +-
 fs/ext4/super.c  |2 +-
 fs/gfs2/quota.c  |   52 +--
 fs/ocfs2/file.c  |6 +-
 fs/ocfs2/quota_global.c  |   43 +++-
 fs/ocfs2/quota_local.c   |   15 +++--
 fs/quota/dquot.c |  116 
 fs/quota/netlink.c   |   10 ++-
 fs/quota/quota.c |   28 ++--
 fs/quota/quota_tree.c|   23 ---
 fs/quota/quota_v1.c  |   12 ++--
 fs/quota/quota_v2.c  |   26 ---
 fs/xfs/xfs_quotaops.c|   14 ++--
 fs/xfs/xfs_trans_dquot.c |8 ++-
 include/linux/quota.h|  162 --
 include/linux/quotaops.h |6 +-
 init/Kconfig |2 -
 17 files changed, 364 insertions(+), 163 deletions(-)

diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index ff9bcdc..73e42f5 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -2814,7 +2814,7 @@ static int ext3_statfs (struct dentry * dentry, struct 
kstatfs * buf)
 
 static inline struct inode *dquot_to_inode(struct dquot *dquot)
 {
-   return sb_dqopt(dquot-dq_sb)-files[dquot-dq_type];
+   return sb_dqopt(dquot-dq_sb)-files[dquot-dq_id.type];
 }
 
 static int ext3_write_dquot(struct dquot *dquot)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d76ec82..78e6036 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4796,7 +4796,7 @@ static int ext4_statfs(struct dentry *dentry, struct 
kstatfs *buf)
 
 static inline struct inode *dquot_to_inode(struct dquot *dquot)
 {
-   return sb_dqopt(dquot-dq_sb)-files[dquot-dq_type];
+   return sb_dqopt(dquot-dq_sb)-files[dquot-dq_id.type];
 }
 
 static int ext4_write_dquot(struct dquot *dquot)
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index a3bde91..e27f8d6 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1057,6 +1057,8 @@ int gfs2_quota_check(struct gfs2_inode *ip, u32 uid, u32 
gid)
 return 0;
 
for (x = 0; x  ip-i_res-rs_qa_qd_num; x++) {
+   int qtype;
+   struct kqid qid;
qd = ip-i_res-rs_qa_qd[x];
 
if (!((qd-qd_id == uid  test_bit(QDF_USER, qd-qd_flags)) ||
@@ -1068,11 +1070,12 @@ int gfs2_quota_check(struct gfs2_inode *ip, u32 uid, 
u32 gid)
value += qd-qd_change;
spin_unlock(qd_lru_lock);
 
+   qtype = test_bit(QDF_USER, qd-qd_flags) ? USRQUOTA : GRPQUOTA;
+   qid = make_kqid(init_user_ns, qtype, qd-qd_id);
if (be64_to_cpu(qd-qd_qb.qb_limit)  
(s64)be64_to_cpu(qd-qd_qb.qb_limit)  value) {
print_message(qd, exceeded);
-   quota_send_warning(test_bit(QDF_USER, qd-qd_flags) ?
-  USRQUOTA : GRPQUOTA, qd-qd_id,
-  sdp-sd_vfs-s_dev, 
QUOTA_NL_BHARDWARN);
+   quota_send_warning(qid, sdp-sd_vfs-s_dev,
+  QUOTA_NL_BHARDWARN);
 
error = 

Re: [PATCH] userns: Add basic quota support v4

2012-08-28 Thread Dave Chinner
On Tue, Aug 28, 2012 at 12:09:56PM -0700, Eric W. Biederman wrote:
 
 Add the data type struct kqid which holds the kernel internal form of
 the owning identifier of a quota.  struct kqid is a replacement for
 the implicit union of uid, gid and project stored in an unsigned int
 and the quota type field 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.
 
 Along with the data type struct kqid comes the helper functions
 qid_eq, qid_lt, from_kqid, from_kqid_munged, qid_valid, make_kqid,

I think Jan's comment about from_kqid being named id_from_kgid is
better, though I also think it would read better as kqid_to_id().
ie:

id = kqid_to_id(ns, qid);

 make_kqid_invalid, make_kqid_uid, make_kqid_gid.

and these named something like uid_to_kqid()

 Change struct dquot dq_id to a struct kqid and remove the now
 unecessary dq_type.
 
 Update the signature of dqget, quota_send_warning, dquot_get_dqblk,
 and dquot_set_dqblk to use struct kqid.
 
 Make minimal changes to ext3, ext4, 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.

How did you test that this all works? e.g. run xfstests -g quota on
each of those filesystems and check for no regressions? And if you
wrote any tests, can you convert them to be part of xfstests so that
namespace aware quotas get tested regularly?

 
 v4:
   - Rename struct qown struct kqid and associated changes
 to the naming of the helper functions.
   - Use qid_t to hold the userspace identifier representation
 of quota identifiers in all new code.
 v3:
   - Add missing negation on qown_valid
 v2:
   - Renamed qown_t struct qown
   - Added the quota type to struct qown.
   - Removed enum quota_type (In this patch it was just noise)
   - Added qown_lt, make_qown_invalid, make_qown_uid, make_qown_gid
   - Taught qown to handle xfs project ids (but only in init_user_ns).
 Q_XGETQUOTA calls .get_quotblk with project ids.

Q_XSETQLIM was modified to handle project quotas as well, I assume?

 index fed504f..96944c0 100644
 --- a/fs/xfs/xfs_quotaops.c
 +++ b/fs/xfs/xfs_quotaops.c
 @@ -97,28 +97,29 @@ xfs_fs_set_xstate(
  STATIC int
  xfs_fs_get_dqblk(
   struct super_block  *sb,
 - int type,
 - qid_t   id,
 + struct kqid qid,
   struct fs_disk_quota*fdq)
  {
   struct xfs_mount*mp = XFS_M(sb);
 + xfs_dqid_t  xfs_id;
  
   if (!XFS_IS_QUOTA_RUNNING(mp))
   return -ENOSYS;
   if (!XFS_IS_QUOTA_ON(mp))
   return -ESRCH;
  
 - return -xfs_qm_scall_getquota(mp, id, xfs_quota_type(type), fdq);
 + xfs_id = from_kqid(init_user_ns, qid);
 + return -xfs_qm_scall_getquota(mp, xfs_id, xfs_quota_type(qid.type), 
 fdq);
  }

Why a temporary variable? Why not just:

return -xfs_qm_scall_getquota(mp, from_kqid(init_user_ns, qid),
  xfs_quota_type(qid.type), fdq);

Indeed, why not drive the struct kqid down another level into
xfs_qm_scall_getquota() where all they are used for is parameters to
the xfs_qm_dqget() function?

  
  STATIC int
  xfs_fs_set_dqblk(
   struct super_block  *sb,
 - int type,
 - qid_t   id,
 + struct kqid qid,
   struct fs_disk_quota*fdq)
  {
   struct xfs_mount*mp = XFS_M(sb);
 + xfs_dqid_t  xfs_id;
  
   if (sb-s_flags  MS_RDONLY)
   return -EROFS;
 @@ -127,7 +128,8 @@ xfs_fs_set_dqblk(
   if (!XFS_IS_QUOTA_ON(mp))
   return -ESRCH;
  
 - return -xfs_qm_scall_setqlim(mp, id, xfs_quota_type(type), fdq);
 + xfs_id = from_kqid(init_user_ns, qid);
 + return -xfs_qm_scall_setqlim(mp, xfs_id, xfs_quota_type(qid.type), fdq);
  }

Same is true here

  
  const struct quotactl_ops xfs_quotactl_operations = {
 diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
 index bcb6054..46de393 100644
 --- a/fs/xfs/xfs_trans_dquot.c
 +++ b/fs/xfs/xfs_trans_dquot.c
 @@ -575,12 +575,14 @@ xfs_quota_warn(
   struct xfs_dquot*dqp,
   int type)
  {
 + int qtype;
 + struct kqid qid;
   /* no warnings for project quotas - we just return ENOSPC later */
   if (dqp-dq_flags  XFS_DQ_PROJ)
   return;
 - quota_send_warning((dqp-dq_flags  XFS_DQ_USER) ? USRQUOTA : GRPQUOTA,
 -be32_to_cpu(dqp-q_core.d_id), mp-m_super-s_dev,
 -type);
 + qtype = (dqp-dq_flags  XFS_DQ_USER) ? USRQUOTA : GRPQUOTA;
 + qid = make_kqid(init_user_ns, qtype, be32_to_cpu(dqp-q_core.d_id));
 +