Re: [PATCH 06/27] xfs: split xfs_setattr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 29, 2011 at 05:13:16PM -0500, Alex Elder wrote:
> Looks good but I think that you need to mask off the
> ia_valid bits in the calls now made in xfs_vn_setattr().

Why? We call xfs_setattr_size if ATTR_SIZE is set.  The ATTR_SIZE
may also have a few other attributes we can handle, and assert on
those that it can't just to make sure.  Similarly xfs_setattr_nonsize
can handle everything but ATTR_SIZE, and again we have an assert to
protect against breeding incorrect XFS-internal callers.

> Also, I think you may still need to check the file type
> for the size-setting function.  Details below.

The VFS only ever does an ATTR_SIZE setattr on regular files.  We have
an assert to ensure that for debug builds, which is a lot more than
most other filesystems do.

> > +	ASSERT((mask & (ATTR_MODE|ATTR_UID|ATTR_GID|ATTR_ATIME|ATTR_ATIME_SET|
> > +			ATTR_MTIME_SET|ATTR_KILL_SUID|ATTR_KILL_SGID|
> > +			ATTR_KILL_PRIV|ATTR_TIMES_SET)) == 0);
> 
> You'll have to mask these off in xfs_vn_setattr() if you're
> going to make this assertion.

No, this is the (implicit) calling convention by the VFS.

> > -		if (S_ISDIR(ip->i_d.di_mode)) {
> > -			code = XFS_ERROR(EISDIR);
> > -			goto error_return;
> > -		} else if (!S_ISREG(ip->i_d.di_mode)) {
> > -			code = XFS_ERROR(EINVAL);
> > -			goto error_return;
> > -		}
> 
> This is the file type checking code I referred to above.

It simply was a leftover from IRIX that we can't hit on Linux.

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs


[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux