On Thu, May 26, 2016 at 06:19:56PM +0200, Jan Kara wrote:
> To avoid clearing of capabilities or security related extended
> attributes too early, inode_change_ok() will need to take dentry instead
> of inode. Propagate dentry down to functions calling inode_change_ok().
> This is rather straightforward except for xfs_set_mode() function which
> does not have dentry easily available. Luckily that function does not
> call inode_change_ok() anyway so we just have to do a little dance with
> function prototypes.
The idea behind the change is good, but I think the little dance
could be improved as it makes the layering of the code seem weirdly
unbalanced to me. e.g.
xfs_vn_setattr()
xfs_vn_setattr_size() <<<< inode_change_ok() here
xfs_vn_setattr()
xfs_vn_setattr_nonsize() <<<< inode_change_ok() here
xfs_setattr_nonsize()
xfs_vn_setattr()
xfs_vn_setattr_size()
xfs_vn_setattr_nonsize() <<<< inode_change_ok() here
xfs_setattr_nonsize()
And to be more confusing, the externally callable functions for the
rest of the XFS code are now xfs_vn_setattr_size() and
xfs_setattr_nonsize() which now have different calling context
limitations.
I think adding a little symmetric make sense. i.e:
xfs_vn_change_ok(dentry, iattr)
{
+ if (mp->m_flags & XFS_MOUNT_RDONLY)
+ return -EROFS;
+
+ if (XFS_FORCED_SHUTDOWN(mp))
+ return -EIO;
+
+ error = inode_change_ok(inode, iattr);
+ if (error)
+ return error;
+
}
xfs_vn_setattr_size(d, i)
{
xfs_vn_change_ok(d, i)
xfs_setattr_size(ip, i)
}
xfs_vn_setattr_nonsize(d, i)
{
xfs_vn_change_ok(d, i)
xfs_setattr_nonsize(ip, i)
}
xfs_vn_setattr(d, i)
{
xfs_vn_change_ok(d, i)
<rest of xfs_vn_setattr unchanged>
}
And remove the inode_change_ok() code from xfs_setattr_size and
xfs_setattr_nonsize() completely. You've already done this with
xfs_vn_setattr_nonsize() - it just needs to be made symmetric to
keep a clean layering between VFS interfaces and internal XFS
interfaces...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
