From patchwork Wed Feb 12 02:37:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Yan, Zheng" X-Patchwork-Id: 3633741 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E52969F1EE for ; Wed, 12 Feb 2014 02:38:19 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A2B5020136 for ; Wed, 12 Feb 2014 02:38:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4E38B2012D for ; Wed, 12 Feb 2014 02:38:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752760AbaBLCiP (ORCPT ); Tue, 11 Feb 2014 21:38:15 -0500 Received: from mga11.intel.com ([192.55.52.93]:1221 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752392AbaBLCiP (ORCPT ); Tue, 11 Feb 2014 21:38:15 -0500 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 11 Feb 2014 18:38:14 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.95,829,1384329600"; d="scan'208";a="479996189" Received: from zyan5-mobl.sh.intel.com (HELO [10.239.13.5]) ([10.239.13.5]) by fmsmga002.fm.intel.com with ESMTP; 11 Feb 2014 18:37:11 -0800 Message-ID: <52FADE57.50107@intel.com> Date: Wed, 12 Feb 2014 10:37:11 +0800 From: "Yan, Zheng" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: Alex Elder , "Yan, Zheng" CC: ceph-devel , Sage Weil Subject: Re: [PATCH 2/5] ceph: remove zero-size xattr References: <1392096612-11481-1-git-send-email-zheng.z.yan@intel.com> <1392096612-11481-2-git-send-email-zheng.z.yan@intel.com> <52FA37F1.2050903@ieee.org> <52FA5D08.6060108@ieee.org> In-Reply-To: <52FA5D08.6060108@ieee.org> Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 02/12/2014 01:25 AM, Alex Elder wrote: > On 02/11/2014 09:10 AM, Yan, Zheng wrote: >> On Tue, Feb 11, 2014 at 10:47 PM, Alex Elder wrote: >>> On 02/10/2014 11:30 PM, Yan, Zheng wrote: >>>> Signed-off-by: Yan, Zheng >>> >>> You really need to explain better under what circumstances >>> a zero-size xattr is getting removed. >>> >>> But apparently it's only when you're updating an xattr >>> (not building it up from a blob from storage). >>> >>> Why are you doing this? Why can't an xattr exist with >>> an empty value? >> >> That is how other FS behave, at least for ext* and btrfs. > > I haven't tested this, I'm just glancing through code. > But it looks to me like a zero-length value is OK, but > a null value pointer means it should be deleted. Note > in btrfs_setxattr(), for example, the same bit of code > I referenced before: > > if (size == 0) > value = ""; /* empty EA, do not remove */ > > And ext4 seems to delete for a null value, but handle > an xattr whose value *length* is zero. Same with XFS. > > So again, I haven't verified through testing, but my > reading of the code (though rusty) still seems to show > that an attribute can have an empty (zero-size) value. You are right, Thanks. how about below patch? --- From f11d5da84230e4993333a063019bad68c67b50d4 Mon Sep 17 00:00:00 2001 From: "Yan, Zheng" Date: Tue, 11 Feb 2014 13:04:19 +0800 Subject: [PATCH 2/5] ceph: remove xattr when null value is given to setxattr() For the setxattr request, introduce a new flag CEPH_XATTR_REMOVE to distinguish null value case from the zero-length value case. Signed-off-by: Yan, Zheng --- fs/ceph/xattr.c | 16 ++++++++++++++-- include/linux/ceph/ceph_fs.h | 5 +++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c index 28f9793..f6becf6 100644 --- a/fs/ceph/xattr.c +++ b/fs/ceph/xattr.c @@ -12,6 +12,9 @@ #define XATTR_CEPH_PREFIX "ceph." #define XATTR_CEPH_PREFIX_LEN (sizeof (XATTR_CEPH_PREFIX) - 1) +static int __remove_xattr(struct ceph_inode_info *ci, + struct ceph_inode_xattr *xattr); + /* * List of handlers for synthetic system.* attributes. Other * attributes are handled directly. @@ -359,6 +362,12 @@ static int __set_xattr(struct ceph_inode_info *ci, kfree(val); return err; } + if (update_xattr < 0) { + if (xattr) + __remove_xattr(ci, xattr); + kfree(name); + return 0; + } } if (!xattr) { @@ -862,6 +871,9 @@ static int ceph_sync_setxattr(struct dentry *dentry, const char *name, dout("setxattr value=%.*s\n", (int)size, value); + if (!value) + flags |= CEPH_XATTR_REMOVE; + /* do request */ req = ceph_mdsc_create_request(mdsc, CEPH_MDS_OP_SETXATTR, USE_AUTH_MDS); @@ -965,8 +977,8 @@ retry: goto retry; } - err = __set_xattr(ci, newname, name_len, newval, - val_len, flags, 1, &xattr); + err = __set_xattr(ci, newname, name_len, newval, val_len, + flags, val ? 1 : -1, &xattr); if (!err) { dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_XATTR_EXCL); diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h index 2623cff..25bfb0e 100644 --- a/include/linux/ceph/ceph_fs.h +++ b/include/linux/ceph/ceph_fs.h @@ -373,8 +373,9 @@ extern const char *ceph_mds_op_name(int op); /* * Ceph setxattr request flags. */ -#define CEPH_XATTR_CREATE 1 -#define CEPH_XATTR_REPLACE 2 +#define CEPH_XATTR_CREATE (1 << 0) +#define CEPH_XATTR_REPLACE (1 << 1) +#define CEPH_XATTR_REMOVE (1 << 31) union ceph_mds_request_args { struct {