From patchwork Fri Sep 1 07:21:49 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Hellwig X-Patchwork-Id: 9933619 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 6DDE26016C for ; Fri, 1 Sep 2017 07:21:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5ECF32855D for ; Fri, 1 Sep 2017 07:21:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5160328581; Fri, 1 Sep 2017 07:21:53 +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=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID 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 7F1B62857D for ; Fri, 1 Sep 2017 07:21:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751422AbdIAHVw (ORCPT ); Fri, 1 Sep 2017 03:21:52 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:32789 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751237AbdIAHVv (ORCPT ); Fri, 1 Sep 2017 03:21:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=3S/iEiy5NiWnntajDdamNro7ueZy77/XU2FuIsR4NfY=; b=cSrmRP5DKO/Dh1lm4Z/wLfiTr c7KCBrNMCVYJRSrZ6I4vpWDjtxh4y4YrL5vx1OrtgyBeieVJ8wlYDLPCFpVvxv9SopGjkecjtdG0K 5ulf1ASWiCMIHIVB5BIkmp3W510WzZtPEiofRw7HIldoeg410ia8s5kJkkrKAYIwWGkEY8ITNK2x9 S6rehJBHnzGu9GZIMBYh6GEbZPfajFT4VFjVErWM5p+bMjbJb+QhmLHbJrA0rFNWAGUtL+BRqfZlq u8VUszBF08sA3T2NxwKkm2fGtPsqcAU9P4wmJT415f5x4XmbGDnMYTM/gEuN8PfSCmnhj+/o5NBne vhaMA3DCg==; Received: from hch by bombadil.infradead.org with local (Exim 4.87 #1 (Red Hat Linux)) id 1dngGz-0007TI-MM; Fri, 01 Sep 2017 07:21:49 +0000 Date: Fri, 1 Sep 2017 00:21:49 -0700 From: Christoph Hellwig To: "Darrick J. Wong" Cc: Christoph Hellwig , Brian Foster , xfs , Jan Kara Subject: Re: [PATCH v2] xfs: don't set v3 xflags for v2 inodes Message-ID: <20170901072149.GA7443@infradead.org> References: <20170830155517.GI4757@magnolia> <20170830163825.GQ4757@magnolia> <20170831131723.GD19544@infradead.org> <20170831133420.GB21939@bfoster.bfoster> <20170831140932.GB26555@infradead.org> <20170831195729.GM3775@magnolia> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170831195729.GM3775@magnolia> User-Agent: Mutt/1.8.3 (2017-05-23) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Aug 31, 2017 at 12:57:29PM -0700, Darrick J. Wong wrote: > TBH I like this less because now the responsibility for checking valid > inputs is split between xfs_ioctl_setattr_xflags and xfs_set_diflags. > I'd rather increase the function count by one than morph the setting > function into check-and-set. I'm not worried about the function count - I'm worried about duplicating the information of which flags are stored in di_flags2. With this patch we have one point where we can naturally check this. In your patch we need another define that needs to be kept uptodate. If you are worried about the check and set we could move the set into the caller, but to me that doesn't seem any cleaner. Example attached below: --- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 9c0c7a920304..511cd7c830ab 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -931,16 +931,15 @@ xfs_ioc_fsgetxattr( return 0; } -STATIC void -xfs_set_diflags( +STATIC unsigned int +xfs_flags2diflags( struct xfs_inode *ip, unsigned int xflags) { - unsigned int di_flags; - uint64_t di_flags2; - /* can't set PREALLOC this way, just preserve it */ - di_flags = (ip->i_d.di_flags & XFS_DIFLAG_PREALLOC); + unsigned int di_flags = + (ip->i_d.di_flags & XFS_DIFLAG_PREALLOC); + if (xflags & FS_XFLAG_IMMUTABLE) di_flags |= XFS_DIFLAG_IMMUTABLE; if (xflags & FS_XFLAG_APPEND) @@ -970,19 +969,24 @@ xfs_set_diflags( if (xflags & FS_XFLAG_EXTSIZE) di_flags |= XFS_DIFLAG_EXTSIZE; } - ip->i_d.di_flags = di_flags; - /* diflags2 only valid for v3 inodes. */ - if (ip->i_d.di_version < 3) - return; + return di_flags; +} + +STATIC uint64_t +xfs_flags2diflags2( + struct xfs_inode *ip, + unsigned int xflags) +{ + uint64_t di_flags2 = + (ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK); - di_flags2 = (ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK); if (xflags & FS_XFLAG_DAX) di_flags2 |= XFS_DIFLAG2_DAX; if (xflags & FS_XFLAG_COWEXTSIZE) di_flags2 |= XFS_DIFLAG2_COWEXTSIZE; - ip->i_d.di_flags2 = di_flags2; + return di_flags2; } STATIC void @@ -1022,6 +1026,7 @@ xfs_ioctl_setattr_xflags( struct fsxattr *fa) { struct xfs_mount *mp = ip->i_mount; + uint64_t di_flags2; /* Can't change realtime flag if any extents are allocated. */ if ((ip->i_d.di_nextents || ip->i_delayed_blks) && @@ -1052,7 +1057,14 @@ xfs_ioctl_setattr_xflags( !capable(CAP_LINUX_IMMUTABLE)) return -EPERM; - xfs_set_diflags(ip, fa->fsx_xflags); + /* diflags2 only valid for v3 inodes. */ + di_flags2 = xfs_flags2diflags(ip, fa->fsx_xflags); + if (di_flags2 && ip->i_d.di_version < 3) + return -EINVAL; + + ip->i_d.di_flags = xfs_flags2diflags(ip, fa->fsx_xflags); + ip->i_d.di_flags2 = di_flags2; + xfs_diflags_to_linux(ip); xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG); xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);