> + if (op != XFS_ATTRUPDATE_REMOVE || xfs_inode_hasattr(dp)) {
> error = xfs_iext_count_may_overflow(dp, XFS_ATTR_FORK,
> XFS_IEXT_ATTR_MANIP_CNT(rmt_blks));
> if (error == -EFBIG)
No need to change this in the current patch, but checking for a remove
on an inodes without attrs just to skip the extent count upgrade
and not fail the whole operation is a little silly :)
> @@ -203,7 +203,8 @@ __xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> xfs_acl_to_disk(args.value, acl);
> }
>
> - error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERT);
> + error = xfs_attr_change(&args, args.value ? XFS_ATTRUPDATE_UPSERT :
> + XFS_ATTRUPDATE_REMOVE);
Given that we have a conditional for removing vs setting right above
i'd clean this up a bit:
xfs_acl_to_disk(args.value, acl);
error = xfs_attr_change(&args, XFS_ATTRUPDATE_UPSERT)
kvfree(args.value);
} else {
error = xfs_attr_change(&args, XFS_ATTRUPDATE_REMOVE)
/*
* If the attribute didn't exist to start with that's fine.
*/
if (error == -ENOATTR)
error = 0;
}
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
