From patchwork Wed Apr 18 09:38:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Luis Henriques X-Patchwork-Id: 10347697 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6BD3660244 for ; Wed, 18 Apr 2018 09:38:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 620122823E for ; Wed, 18 Apr 2018 09:38:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 568FE2847E; Wed, 18 Apr 2018 09:38:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 868872823E for ; Wed, 18 Apr 2018 09:38:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752145AbeDRJiB convert rfc822-to-8bit (ORCPT ); Wed, 18 Apr 2018 05:38:01 -0400 Received: from mx2.suse.de ([195.135.220.15]:46383 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750983AbeDRJiA (ORCPT ); Wed, 18 Apr 2018 05:38:00 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id EA354ACA4; Wed, 18 Apr 2018 09:37:58 +0000 (UTC) From: Luis Henriques To: "Yan\, Zheng" Cc: Ilya Dryomov , Patrick Donnelly , Ceph Development Subject: Re: new (> 4.16) kernel cephfs clients behaviour on < mimic References: <87vacq9c8u.fsf@suse.com> Date: Wed, 18 Apr 2018 10:38:29 +0100 In-Reply-To: (Zheng Yan's message of "Wed, 18 Apr 2018 10:05:45 +0800") Message-ID: <877ep46ei2.fsf@suse.com> MIME-Version: 1.0 Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP "Yan, Zheng" writes: >> On Apr 17, 2018, at 21:46, Luis Henriques wrote: >> >> Hi! >> >> I see the following commit is queued for the kernel cephfs client: >> >> commit e2311baa73a883eb8501c895cc817d3c96c3b896 >> Author: Yan, Zheng >> 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, 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