"Yan, Zheng" <zyan@xxxxxxxxxx> writes: >> On Apr 17, 2018, at 21:46, Luis Henriques <lhenriques@xxxxxxxx> wrote: >> >> Hi! >> >> I see the following commit is queued for the kernel cephfs client: >> >> commit e2311baa73a883eb8501c895cc817d3c96c3b896 >> Author: Yan, Zheng <zyan@xxxxxxxxxx> >> Date: Sun Apr 8 09:54:39 2018 +0800 >> >> ceph: check if mds create snaprealm when setting quota >> >> If mds does not, return -EOPNOTSUPP. >> >> [ This is related with http://tracker.ceph.com/issues/23491 ] >> >> However, I'm not sure it is doing the right thing (although I confess >> I'm not sure either what the right thing would be...) >> >> This commit is doing 2 things: >> >> 1) it returns an error if a user tries to set quotas and the MDS doesn't >> support it (using the new snaprealm method) >> >> 2) it forbids reading the ceph.quota.* xattrs, even if they're set >> >> Regarding 1), it's a change in behaviour from previous kernels -- older >> kernels allow these attributes to be set (but not read!), even if they >> don't really support quotas. Also, the error that is returned >> (-EOPNOTSUPP) is a bit misleading as the xattr is actually set in the >> directory. > > any better error code? The error code is fine, my complain was about returning an error for an operation that actually succeeded: if a different (fuse) client does a getfattr it will see that the attribute was set and it *will* have quotas enabled (at least the old versions of the fuse client, I haven't check if this has changed in new versions). >> >> Regarding 2), it's a bit artificial to forbid the user to get these >> attributes as we do actually have them available. > > If attributes have no effect, why can’t we treat them as nonexistence. True, the attribute does not have an effect for *this* client. > >> >> I understand the rational behind this patch, but since quotas won't be >> enforced anyway in this scenario, is there really any value added with >> this patch? >> >> I would suggest 2 alternatives: >> >> 1. check cluster version and return the error in setxattr if version < >> Mimic (but maybe still allow reading the xattrs?) >> > > The problem is we run out of feature bits. No straight way to do the check > Ah! I missed that :-/ > >> 2. do nothing -- if we're using a cluster that does not support quotas >> anyway, maybe there's no point in having these different behaviours >> for old and new clients. >> > > This is confusing. we reclaim that > 4.16 kernel supports quota. I don’t think it’s a good that > user successfully set quota on > 4.16 kernel, but find that quota has no effect. Again, this is not completely true -- old fuse clients will see the side effects of this operation. > 4.16 kernel is the first one that support quota, does behavior change really matter? Right, I don't really have any strong feelings against this patch. As I said, my preference would be one of the 2 alternatives above, but I guess that at least we'll need to update doc/cephfs/quota.rst (see suggestion bellow). Cheers, -- Luis >From 5a034c3d43e4828e2caaa6156f067155ca2f6c39 Mon Sep 17 00:00:00 2001 From: Luis Henriques <lhenriques@xxxxxxxx> Date: Wed, 18 Apr 2018 10:28:55 +0100 Subject: [PATCH] doc/cephfs: update kernel client quotas support info Signed-off-by: Luis Henriques <lhenriques@xxxxxxxx> --- doc/cephfs/quota.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/cephfs/quota.rst b/doc/cephfs/quota.rst index aad0e0bbf0c2..60e826b40139 100644 --- a/doc/cephfs/quota.rst +++ b/doc/cephfs/quota.rst @@ -23,9 +23,12 @@ Limitations of data. Generally speaking writers will be stopped within 10s of seconds of crossing the configured limit. -#. *Quotas are not yet implemented in the kernel client.* Quotas are - supported by the userspace client (libcephfs, ceph-fuse) but are - not yet implemented in the Linux kernel client. +#. *Quotas are implemented in the kernel client > 4.16 only.* Quotas + are supported by the userspace client (libcephfs, ceph-fuse). + Linux kernel clients > 4.16 support CephFS quotas but only on + mimic+ clusters. Kernel clients (even recent versions) will fail + to handle quotas on older clusters, even if they may be able to set + the quotas extended attributes. #. *Quotas must be configured carefully when used with path-based mount restrictions.* The client needs to have access to the -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
