Re: [v10 4/5] ext4: adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR interface support

[Jan Kara <jack@suse.cz>]

On Thu 19-03-15 04:04:56, Li Xi wrote:
> This patch adds FS_IOC_FSSETXATTR/FS_IOC_FSGETXATTR ioctl interface
> support for ext4. The interface is kept consistent with
> XFS_IOC_FSGETXATTR/XFS_IOC_FSGETXATTR.
  Two more comments below.

> 
> Signed-off-by: Li Xi <lixi@ddn.com>
...
> +static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
> +{
> +	struct inode *inode = file_inode(filp);
> +	struct super_block *sb = inode->i_sb;
> +	struct ext4_inode_info *ei = EXT4_I(inode);
> +	int err;
> +	handle_t *handle;
> +	kprojid_t kprojid;
> +	struct ext4_iloc iloc;
> +	struct ext4_inode *raw_inode;
> +
> +	struct dquot *transfer_to[EXT4_MAXQUOTAS] = { };
> +
> +	/* Make sure caller can change project. */
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EACCES;
  Why do you test capabilities here when you already test permission in
EXT4_IOC_FSSETXATTR? Furthermore this test is too restrictive. Just delete
it.

...
> -flags_out:
> +		err = ext4_ioctl_setflags(inode, flags);
>  		mutex_unlock(&inode->i_mutex);
> -		mnt_drop_write_file(filp);
> +		mnt_drop_write(filp->f_path.mnt);
  Why did you change this? mnt_drop_write_file() should stay here.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR