Re: libquota proposal

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Thu, Mar 24, 2011 at 01:20:10AM +0000, David Holland wrote:
> On Fri, Mar 18, 2011 at 08:30:41PM +0100, Manuel Bouyer wrote:
>  > looking at pkgsrc/net/netatalk to make it use the new quota interface
>  > convinced be that we need a libquota, which can return the quota status
>  > for a id in a simple way, as independant as possible from the underlying
>  > filesystem. So I propose to hide the magic in a libquota,
>  > which propose a:
>  > int getfsquota(const char *path, struct ufs_quota_entry *qv, uid_t id,
>  >   const char *class)
>                                            ^^^
> 
> Any fs-independent library should not be using fs-specific
> structures.

the ufs here is for "UFS-like quota semantic". It's not restricted to
ufs, but to any filesystem implementing ufs-like quota (that is,
quotas with user and group class, each class with block and file
set of limits, each set of limits has soft and hard limits with grace
times).

> 
>  > (path is path to a file, not necesserely a mount point.
>  > class is "user" or "group" at this time, as defined in sys/quota.h).
> 
> These should be constants, not strings.

I guess you mean "constant integer". This is a pointer to some const strings
defined in <sys/quota.h>; I can't see much difference with a constant
integer.

>  > For tools that care, a getufsquota() and getnfsquota() is also accessible
>  > (it's free as they are needed for getfsquota() anyway).
> 
> This does not make sense. What about e.g. zfs, or whatever else comes
> down the pipe? Also, what about fses that have more than (or less
> than) just block and inode quotas?

see above. if they have UFS quota semantics, this will work. It
can maybe be extended for other class of quotas, or other limit types
when these shows up, or maybe it needs something different. getfsquota()
is what is supposed to hide these details.

> 
>  > ? common/include/quota
>  > ? common/lib/libquota
>  > ? lib/libquota
> 
> helps to use -N when generating diffs for people to read.

I did use -N, but this doesn't work for new directory which have not
yet been cvs added (and I didn't want to cvs add them before time because
they are created in repository at add time, not commit time).

> 
>  > +bool              prop_array_add_and_rel(prop_array_t, prop_object_t);
> 
> I'm going to let someone else worry about whether this is a sensible
> public proplib operation.
> 
>  > -  ufs_quota.c ufs_quota2.c quota2_subr.c
>  > +  ufs_quota.c ufs_quota2.c quota2_subr.c 
> 
> noo! not trailing whitespace! :-)

removed

> 
>  > --- sys/ufs/ufs/Makefile   6 Mar 2011 17:08:39 -0000       1.7
>  > +++ sys/ufs/ufs/Makefile   18 Mar 2011 16:00:30 -0000
>  > @@ -2,7 +2,7 @@
>  >  
>  >  INCSDIR= /usr/include/ufs/ufs
>  >  
>  > -INCS=     dinode.h dir.h extattr.h inode.h quota.h quota1.h quota2.h \
>  > +INCS=     dinode.h dir.h extattr.h inode.h quota.h \
>  >    ufs_bswap.h ufs_extern.h ufs_wapbl.h ufsmount.h
>  >  
>  >  .include <bsd.kinc.mk>
> 
> uh... that doesn't seem like a good idea.

Yes, I figured it out after sending the diff. I backed out this part
after commit.

> 
>  > Index: sys/ufs/ufs/quota.h
>  > ===================================================================
>  > RCS file: /cvsroot/src/sys/ufs/ufs/quota.h,v
>  > retrieving revision 1.26
>  > diff -u -r1.26 quota.h
>  > --- sys/ufs/ufs/quota.h    6 Mar 2011 17:08:39 -0000       1.26
>  > +++ sys/ufs/ufs/quota.h    18 Mar 2011 16:00:30 -0000
>  > @@ -36,6 +36,7 @@
>  >  
>  >  #ifndef   _UFS_UFS_QUOTA_H_
>  >  #define   _UFS_UFS_QUOTA_H_
>  > +#include <quota/quotaprop.h>
> 
> er, in the kernel?

yes; there are conversion from binary to proplib (and reverse) in the
kernel too.

> 
>  > +__inline static int __unused
>  > +ufsclass2qtype(int class)
>  > +{
>  > +  switch(class) {
>  > +  case QUOTA_CLASS_USER:
>  > +          return USRQUOTA;
>  > +  case QUOTA_CLASS_GROUP:
>  > +          return GRPQUOTA;
>  > +  default:
>  > +          return -1;
>  > +  }
>  > +}
>  >  
>  > +static __inline int __unused
>  > +qtype2ufsclass(int type)
>  > +{
>  > +  switch(type) {
>  > +  case USRQUOTA:
>  > +          return QUOTA_CLASS_USER;
>  > +  case GRPQUOTA:
>  > +          return QUOTA_CLASS_GROUP;
>  > +  default:
>  > +          return -1;
>  > +  }
>  > +}
> 
> Surely these don't have to be exposed from the kernel?

they're used for legacy tools as well.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--