Please don't reply to lustre-devel. Instead, comment in Bugzilla by using the
following link:
https://bugzilla.lustre.org/show_bug.cgi?id=11463
What |Removed |Added
----------------------------------------------------------------------------
Attachment #9366|review?([EMAIL PROTECTED])|review+
Flag| |
(From update of attachment 9366)
I have no problem with the technical parts of this patch, but I have some
concern that this will cause Lustre to violate POSIX requirements for the
behaviour of chmod.
SUSv2 says for chmod(2):
"If the effective UID of the process is not zero and the group of the
file does not match the effective group ID of the process or one of
its supplementary group IDs, the S_ISGID bit will be turned off, but
this will not cause an error to be returned."
That said, I don't _think_ that this change will cause us to change normal
behaviour, because the chmod(3) tool essentially does the same thing (likely to
do what the user _expects_ instead of what POSIX mandates).
The code and comments here are taken from chown_common() in the kernel so
should not be taken lightly. However, one flaw (IMHO) in the kernel
implementation is that it assumes ATTR_UID/ATTR_GID mean that the UID/GID are
changing, but in fact these might be set even if the values remain the same.
That is likely masked by the fact that chown(2) skips the syscall entirely if
the values are not changing.
Other things of note here:
- we could also do the chmod to reset the UID/GID easily, because the userspace
code already HAS the stat information from the ioctl to get the UID/GID in the
first place
- in theory it would be possible to just use "chown(file, -1, -1)" on all of
the files to avoid doing any ioctl/stat at all, and this would likely go faster
(fewer RPCs), but is not directly related to this bug
_______________________________________________
Lustre-devel mailing list
[email protected]
https://mail.clusterfs.com/mailman/listinfo/lustre-devel
