Re: [PATCH 1/3] xfs: ensure that the inode uid/gid match values match the icdinode ones

[Brian Foster <bfoster@redhat.com>]

On Tue, Feb 18, 2020 at 01:00:18PM -0800, Christoph Hellwig wrote:
> Instead of only synchronizing the uid/gid values in xfs_setup_inode,
> ensure that they always match to prepare for removing the icdinode
> fields.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

The patch subject looks like it has an extra "match" in there. Otherwise
looks Ok to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_inode_buf.c | 2 ++
>  fs/xfs/xfs_icache.c           | 4 ++++
>  fs/xfs/xfs_inode.c            | 8 ++++++--
>  fs/xfs/xfs_iops.c             | 3 ---
>  4 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 8afacfe4be0a..cc4efd34843a 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -223,7 +223,9 @@ xfs_inode_from_disk(
>  
>  	to->di_format = from->di_format;
>  	to->di_uid = be32_to_cpu(from->di_uid);
> +	inode->i_uid = xfs_uid_to_kuid(to->di_uid);
>  	to->di_gid = be32_to_cpu(from->di_gid);
> +	inode->i_gid = xfs_gid_to_kgid(to->di_gid);
>  	to->di_flushiter = be16_to_cpu(from->di_flushiter);
>  
>  	/*
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 8dc2e5414276..a7be7a9e5c1a 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -289,6 +289,8 @@ xfs_reinit_inode(
>  	uint64_t	version = inode_peek_iversion(inode);
>  	umode_t		mode = inode->i_mode;
>  	dev_t		dev = inode->i_rdev;
> +	kuid_t		uid = inode->i_uid;
> +	kgid_t		gid = inode->i_gid;
>  
>  	error = inode_init_always(mp->m_super, inode);
>  
> @@ -297,6 +299,8 @@ xfs_reinit_inode(
>  	inode_set_iversion_queried(inode, version);
>  	inode->i_mode = mode;
>  	inode->i_rdev = dev;
> +	inode->i_uid = uid;
> +	inode->i_gid = gid;
>  	return error;
>  }
>  
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index c5077e6326c7..938b0943bd95 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -812,15 +812,19 @@ xfs_ialloc(
>  
>  	inode->i_mode = mode;
>  	set_nlink(inode, nlink);
> -	ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
> -	ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
> +	inode->i_uid = current_fsuid();
> +	ip->i_d.di_uid = xfs_kuid_to_uid(inode->i_uid);
>  	inode->i_rdev = rdev;
>  	ip->i_d.di_projid = prid;
>  
>  	if (pip && XFS_INHERIT_GID(pip)) {
> +		inode->i_gid = VFS_I(pip)->i_gid;
>  		ip->i_d.di_gid = pip->i_d.di_gid;
>  		if ((VFS_I(pip)->i_mode & S_ISGID) && S_ISDIR(mode))
>  			inode->i_mode |= S_ISGID;
> +	} else {
> +		inode->i_gid = current_fsgid();
> +		ip->i_d.di_gid = xfs_kgid_to_gid(inode->i_gid);
>  	}
>  
>  	/*
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 81f2f93caec0..b818b261918f 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1304,9 +1304,6 @@ xfs_setup_inode(
>  	/* make the inode look hashed for the writeback code */
>  	inode_fake_hash(inode);
>  
> -	inode->i_uid    = xfs_uid_to_kuid(ip->i_d.di_uid);
> -	inode->i_gid    = xfs_gid_to_kgid(ip->i_d.di_gid);
> -
>  	i_size_write(inode, ip->i_d.di_size);
>  	xfs_diflags_to_iflags(inode, ip);
>  
> -- 
> 2.24.1
>