On Fri, Nov 17, 2017 at 11:21:43AM -0700, Allison Henderson wrote:
> This patch removes the old parent pointer attribute during the
> rename operation, and re-adds the updated parent pointer
>
> Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_dir2.c | 6 ++++--
> fs/xfs/xfs_inode.c | 26 ++++++++++++++++++++------
> 2 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index 25b370b..ed25203 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -324,10 +324,11 @@ xfs_dir_createname(
> else
> rval = xfs_dir2_node_addname(args);
>
> +out_free:
> /* return the location that this entry was place in the parent inode */
> if (offset)
> *offset = args->offset;
> -out_free:
> +
> kmem_free(args);
> return rval;
> }
> @@ -496,9 +497,10 @@ xfs_dir_removename(
> rval = xfs_dir2_leaf_removename(args);
> else
> rval = xfs_dir2_node_removename(args);
> +out_free:
> if (offset)
> *offset = args->offset;
> -out_free:
> +
Why do these labels need to be moved here? The *offset = ... lines are
new, so why not put them in their final place in the patch that adds
those lines?
> kmem_free(args);
> return rval;
> }
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index a289a40..da5c761 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2984,6 +2984,8 @@ xfs_rename(
> bool src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
> int spaceres;
> int error;
> + xfs_dir2_dataptr_t new_diroffset;
> + xfs_dir2_dataptr_t old_diroffset;
>
> trace_xfs_rename(src_dp, target_dp, src_name, target_name);
>
> @@ -3086,13 +3088,12 @@ xfs_rename(
> */
> error = xfs_dir_createname(tp, target_dp, target_name,
> src_ip->i_ino, &first_block, &dfops,
> - spaceres, NULL);
> + spaceres, &new_diroffset);
> if (error)
> goto out_bmap_cancel;
>
> xfs_trans_ichgtime(tp, target_dp,
> XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
> -
Huh?
> if (new_parent && src_is_directory) {
> error = xfs_bumplink(tp, target_dp);
> if (error)
> @@ -3126,7 +3127,7 @@ xfs_rename(
> */
> error = xfs_dir_replace(tp, target_dp, target_name,
> src_ip->i_ino, &first_block, &dfops,
> - spaceres, NULL);
> + spaceres, &new_diroffset);
> if (error)
> goto out_bmap_cancel;
>
> @@ -3161,7 +3162,7 @@ xfs_rename(
> */
> error = xfs_dir_replace(tp, src_ip, &xfs_name_dotdot,
> target_dp->i_ino, &first_block, &dfops,
> - spaceres, NULL);
> + spaceres, &new_diroffset);
> ASSERT(error != -EEXIST);
> if (error)
> goto out_bmap_cancel;
> @@ -3200,11 +3201,12 @@ xfs_rename(
> */
> if (wip) {
> error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino,
> - &first_block, &dfops, spaceres, NULL);
> + &first_block, &dfops, spaceres,
> + &old_diroffset);
> } else
> error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
> &first_block, &dfops, spaceres,
> - NULL);
> + &old_diroffset);
> if (error)
> goto out_bmap_cancel;
>
> @@ -3234,6 +3236,18 @@ xfs_rename(
> VFS_I(wip)->i_state &= ~I_LINKABLE;
> }
>
> + if (new_parent && xfs_sb_version_hasparent(&mp->m_sb)) {
I'm confused about checking new_parent -- does this cause us to forget
to update the pptr for a rename within a directory?
I'm assuming that all of these directory operations will get their own
xfstests in time... (create a file, check pptr; hardlink a file, check
both pptrs; delete one of the links, check pptrs; check pptr after a
rename within a dir; check pptr after a rename between dirs; etc.)
--D
> + error = xfs_parent_add(tp, target_dp, src_ip, target_name,
> + new_diroffset, &dfops, &first_block);
> + if (error)
> + goto out_bmap_cancel;
> +
> + error = xfs_parent_remove(tp, src_dp, src_ip,
> + old_diroffset, &dfops, &first_block);
> + if (error)
> + goto out_bmap_cancel;
> + }
> +
> xfs_trans_ichgtime(tp, src_dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
> xfs_trans_log_inode(tp, src_dp, XFS_ILOG_CORE);
> if (new_parent)
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
