Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <3bd566c0-6d4b-11ab-1402-a8ee63502dec@paragon-software.com>
Date:   Tue, 19 Oct 2021 20:11:26 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
From:   Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
Subject: Re: [PATCH 1/2] fs/ntfs3: Remove unnecessary functions
To:     Kari Argillander <kari.argillander@gmail.com>
CC:     <ntfs3@lists.linux.dev>, <linux-kernel@vger.kernel.org>,
        <linux-fsdevel@vger.kernel.org>
References: <992eee8f-bed8-4019-a966-1988bd4dd5de@paragon-software.com>
 <2ce78ab6-453d-d7bf-9969-eb47b7347098@paragon-software.com>
 <20211016104209.r6mgz2ote4jcmgcj@kari-VirtualBox>
Content-Language: en-US
In-Reply-To: <20211016104209.r6mgz2ote4jcmgcj@kari-VirtualBox>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 16.10.2021 13:42, Kari Argillander wrote:
> On Fri, Oct 01, 2021 at 07:02:12PM +0300, Konstantin Komarov wrote:
>> We don't need ntfs_xattr_get_acl and ntfs_xattr_set_acl.
>> There are ntfs_get_acl_ex and ntfs_set_acl_ex.
> 
> I just bisect this commit after tests
> 
> "generic/099,generic/105,generic/307,generic/318,generic/319,generic/375,generic/444"
> 
> fails for me. Fails happends because mount option acl was not defined.
> Before they where skipped, but now fail occurs. Also generic/099 was
> passing if acl mount option was defined, but after this patch it also
> fail. Every other test fail for me as well, but that is not related to
> this patch.
> 
> So should we revert or do you make new patch to fix the issue or do you
> think we won't have any issue here?
> 
>   Argillander
> 

We skipped these tests(muted them in beginning and forgot to unmute).
Thanks for pointing this out. There will be patch to fix them.

>> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
>> ---
>>  fs/ntfs3/xattr.c | 94 ------------------------------------------------
>>  1 file changed, 94 deletions(-)
>>
>> diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
>> index 83bbee277e12..111355692163 100644
>> --- a/fs/ntfs3/xattr.c
>> +++ b/fs/ntfs3/xattr.c
>> @@ -621,67 +621,6 @@ int ntfs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
>>  	return ntfs_set_acl_ex(mnt_userns, inode, acl, type, 0);
>>  }
>>  
>> -static int ntfs_xattr_get_acl(struct user_namespace *mnt_userns,
>> -			      struct inode *inode, int type, void *buffer,
>> -			      size_t size)
>> -{
>> -	struct posix_acl *acl;
>> -	int err;
>> -
>> -	if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
>> -		ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
>> -		return -EOPNOTSUPP;
>> -	}
>> -
>> -	acl = ntfs_get_acl(inode, type);
>> -	if (IS_ERR(acl))
>> -		return PTR_ERR(acl);
>> -
>> -	if (!acl)
>> -		return -ENODATA;
>> -
>> -	err = posix_acl_to_xattr(mnt_userns, acl, buffer, size);
>> -	ntfs_posix_acl_release(acl);
>> -
>> -	return err;
>> -}
>> -
>> -static int ntfs_xattr_set_acl(struct user_namespace *mnt_userns,
>> -			      struct inode *inode, int type, const void *value,
>> -			      size_t size)
>> -{
>> -	struct posix_acl *acl;
>> -	int err;
>> -
>> -	if (!(inode->i_sb->s_flags & SB_POSIXACL)) {
>> -		ntfs_inode_warn(inode, "add mount option \"acl\" to use acl");
>> -		return -EOPNOTSUPP;
>> -	}
>> -
>> -	if (!inode_owner_or_capable(mnt_userns, inode))
>> -		return -EPERM;
>> -
>> -	if (!value) {
>> -		acl = NULL;
>> -	} else {
>> -		acl = posix_acl_from_xattr(mnt_userns, value, size);
>> -		if (IS_ERR(acl))
>> -			return PTR_ERR(acl);
>> -
>> -		if (acl) {
>> -			err = posix_acl_valid(mnt_userns, acl);
>> -			if (err)
>> -				goto release_and_out;
>> -		}
>> -	}
>> -
>> -	err = ntfs_set_acl(mnt_userns, inode, acl, type);
>> -
>> -release_and_out:
>> -	ntfs_posix_acl_release(acl);
>> -	return err;
>> -}
>> -
>>  /*
>>   * ntfs_init_acl - Initialize the ACLs of a new inode.
>>   *
>> @@ -848,23 +787,6 @@ static int ntfs_getxattr(const struct xattr_handler *handler, struct dentry *de,
>>  		goto out;
>>  	}
>>  
>> -#ifdef CONFIG_NTFS3_FS_POSIX_ACL
>> -	if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
>> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
>> -		     sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
>> -	    (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
>> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
>> -		     sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
>> -		/* TODO: init_user_ns? */
>> -		err = ntfs_xattr_get_acl(
>> -			&init_user_ns, inode,
>> -			name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
>> -				? ACL_TYPE_ACCESS
>> -				: ACL_TYPE_DEFAULT,
>> -			buffer, size);
>> -		goto out;
>> -	}
>> -#endif
>>  	/* Deal with NTFS extended attribute. */
>>  	err = ntfs_get_ea(inode, name, name_len, buffer, size, NULL);
>>  
>> @@ -977,22 +899,6 @@ static noinline int ntfs_setxattr(const struct xattr_handler *handler,
>>  		goto out;
>>  	}
>>  
>> -#ifdef CONFIG_NTFS3_FS_POSIX_ACL
>> -	if ((name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1 &&
>> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
>> -		     sizeof(XATTR_NAME_POSIX_ACL_ACCESS))) ||
>> -	    (name_len == sizeof(XATTR_NAME_POSIX_ACL_DEFAULT) - 1 &&
>> -	     !memcmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
>> -		     sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)))) {
>> -		err = ntfs_xattr_set_acl(
>> -			mnt_userns, inode,
>> -			name_len == sizeof(XATTR_NAME_POSIX_ACL_ACCESS) - 1
>> -				? ACL_TYPE_ACCESS
>> -				: ACL_TYPE_DEFAULT,
>> -			value, size);
>> -		goto out;
>> -	}
>> -#endif
>>  	/* Deal with NTFS extended attribute. */
>>  	err = ntfs_set_ea(inode, name, name_len, value, size, flags, 0);
>>  
>> -- 
>> 2.33.0
>>