On Wed, Oct 24, 2012 at 07:02:33PM -0400, Brian Foster wrote: > On 10/24/2012 03:41 PM, Dave Chinner wrote: > > On Wed, Oct 24, 2012 at 12:18:58PM -0400, Brian Foster wrote: > >> On 10/22/2012 09:42 PM, Dave Chinner wrote: > >>> On Fri, Oct 05, 2012 at 10:17:14AM -0400, Brian Foster wrote: > >>>> Support quota ID filtering in the eofblocks scan. The caller must > >>>> set the XFS_EOF_FLAGS_QUOTA flags bit, quota ID and quota type. The > >>>> associated quota type must be enabled on the mounted filesystem. > >>> > >>> I'm wondering if this even needs quota enabled to filter based on a > >>> uid, gid, or prid? > >>> > >> > >> Not really... > >> > >>> The quota part of it seem irrelevant to the filtering that is being > >>> executed - we are only matching against the inode uid/gid/prid, not > >>> against dquots - and I can see situations where just being able to > >>> filter on a given uid/gid might be useful in a multi-user > >>> environment regardless of whether quotas are being used. > >>> > >> > >> This is how my rfc version was implemented (with a comment to the same > >> effect). I added the explicit checking in response to: > >> > >> http://oss.sgi.com/archives/xfs/2012-09/msg00024.html > > > > Ok, that was in the context of "we need to be specific about what > > the ID is". Now the question is "is quota ID the right ID to use?". > > Different question. ;) > > > > Fair enough. :) I just want to make sure we're on the same page. I'll > take this as an Ok to remove the quota enabled checks. The resulting > behavior will now be that quota ID filtering is supported regardless of > whether quotas are enabled on the targeted mount. Right, and that point it isn't a quota ID - it's a uid, gid or prid we are filtering on.... > > Seeing as it is just a filter, enabling the above (even though it > > would be rarely used) might make sense. i.e. separate field for each > > id. It's not like we have to keep the size of the structure to a > > minimum... > > > > ... but that is also starting to further complicate the filtering and > testing required. I'm also not quite sure how we would currently specify > a union of id's in the flags given that we migrated towards the use of > the real quota type field (eof_q_type) rather than my original approach > of specifying the quota id type as a flag. Well, quota type just goes away completely - it's irrelevant. > Would we be sufficiently prepared for the future if we broke the id into > three fields, i.e.: > > struct xfs_eofblocks { > ... > __u32 eof_uquota_id; > __u32 eof_gquota_id; > __u32 eof_pquota_id; > __u32 eof_q_type; > ... I'd suggest uid, gid and prid as the types, dropping the quota type altogether. Then add specific flags to indicate each field is set rather than a flag to say "look at the q_type field for which id field to read"... i.e. the API is indepedent of quota configurations and quota IDs, but is still capable of expressing such control when required. > ... but the in kernel implementation remains as is by using the > associated field based on the quota type? To be honest, even if we had a > way to specify multiple id's, I think I would prefer to leave the > implementation as is and do the enhanced filtering in future patches > (e.g., get the data structure right, but EINVAL for now if multiple id's > are flagged), Sure, that's fine. > if for nothing else than the testing I've already done > against this implementation. But I suspect if there isn't currently a > standard way to specify multiple quota types, we're fated to have to > update the structure with a new version anyways..? There is a standard way of recording a *single* "quota ID" in kernel space now - see quota.h: struct kqid { /* Type in which we store the quota identifier */ union { kuid_t uid; kgid_t gid; kprojid_t projid; }; enum quota_type type; /* USRQUOTA (uid) or GRPQUOTA (gid) or PRJQUOTA (projid) */ } and a bunch or wrappers for manipulating them. That's the sort of thing that we'd need to build internally for filtering so we end up with user namespace support. This is shiny and new, just merged in 3.7-rc1.... > > XQM_* are the userspace XFS quota interface definitions. They are > > what xfs_quota uses. They just so happen to be the same as the > > generic quota definitions except the generic quotas don't define > > project quota types (hence the need for mapping). See > > include/uapi/linux/dqblk_xfs.h. > > > > I just figured that rather than mapping something unknown to > > something known, just using something known in the first place is > > much simpler... > > > > Right, at the time I think I dug through the various quota type > definitions, saw that we provided this mapping function and got the > impression we should be consistent throughout the filesystem. But I > think your point here is that the expected input should be the > definitions we already define for userspace whereas I was thinking the > generic definitions were ubiquitous from userspace. Well, the generic definitions never defined project quota. If you look now in the 3.7 codebase you'll see that PRJQUOTA has been added to include/linux/quota.h, as has a kprojid_t type. Hence project quota IDs are now considered a first class quota citizen by the generic quota *kernel* code. However, include/uapi/linux/quota.h is completely unchanged, so the generic quota userspace interface still knows nothing about project IDs, which means the only actual user facing definition is still in include/uapi/linux/dqblks_xfs.h - XQM_PRJQUOTA. Hence we assume a quota ID that doesn't match anything the generic quota understands to be a project quota as that's the only way stuff works in a predictable, reliable manner... The origin of this mess are historic as XFS brought along it's own quota implementation and API from Irix years ago because the linux quota subsystem was not robust or fully featured enough to even consider integration at the time (e.g. generic quotas didn't even support journalled quotas). So, yeah, a mess but one that is slowly getting cleaned up. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
