Chrishtoph, After we figured that there are ABI/API issues with adding pquota information to the end (while using the same command), I posted what you are suggesting now (http://oss.sgi.com/archives/xfs/2013-07/msg00212.html) as I also do not like redundant code. Please see Dave's comment at the link above in which he wanted me to change the code so that the two commands are totally independent. There was no objections to Dave's suggestion, so I made the changes as he suggested. On Wed, 2013-08-21 at 11:19 -0700, Christoph Hellwig wrote: > On Wed, Aug 21, 2013e at 01:12:47PM -0500, Ben Myers wrote: > > So you don't like the addition of .get_xstatev in quotactl_ops, and > > fs_quota_stat would need to match with fs_quota_statv, adding the project quota > > to the end of the structure? > > That was what I had in mind initially, before the additional > complication were pointed out, except that we don't need it to look > exactly the same - if we use put_user calls instead of copy_to_user that > layout doesn't have to be the same. > > However it seems like going down the stat route and having a kquota_info > structure might be the better way to fully separate the in-kernel API > from the userspace ABI. > > > > Well, the trouble is with gquota vs pquota - previously we report in > > > qs_gquota field either group quotas or project quotas depending on what is > > > turned on. Generic quota code doesn't know this so xfs get_xstatev() would > > > have to recognize whether it is being called from the old Q_XGETSTAT > > > quotactl or from the new Q_XGETSTATV quotactl to know where to fill in > > > project quotas. And at that point you somewhat loose the elegancy of using > > > one interface - we could set qs_version to some special value so that > > > .get_xstatev() recognizes this and does the magic but that doesn't seem very > > > different from the extra call... > > > > IIUC to make this happen without the addition of .get_xstate in quotactl_ops, > > .get_xstate could also grow an argument to determine whether it was called as > > Q_XGETSTAT vs Q_XGETSTATV. If called as Q_XGETSTATV it can look at qs_version > > to determine how much to copy. That might be a bit cleaner than the qs_version > > magic number, as long as you don't mind changing the .get_xstate interface. > > I don't think we'd need that argument - the fs would always fill out > both fields, then the implementation of Q_XGETSTAT would: > > (1) fail if both group and project quota information is present There was a discussion on this issue and it was decided to provide back only gquota information when both quotas are present and Q_XGETSTAT command was used (instead of failing, which will break API) > (2) export the pquota fields as gqouta if only project quota is present > > > Anyway, please give a shout if I need to revert this. I believe the commit > > raced with the commentary. ;) > > As this is in-kernel only I don't think we need to revert anything, but > it would be nice to fix it before 3.12 is released. I didn't exactly > race either, your reply to Jan made me look a it a bit more. > > _______________________________________________ > xfs mailing list > xfs@xxxxxxxxxxx > http://oss.sgi.com/mailman/listinfo/xfs > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
