Subject: bug#5775: [PATCH] doc: fix info to say cp -a does _not_ diagnose xattr failures

From: Jim Meyering <[email-masked]>
Date: Sun, 11 Apr 2010 17:47:31 +0200

Pádraig Brady wrote:
> On 11/04/10 14:37, Jim Meyering wrote:
>> Pádraig Brady wrote:
>>> On 26/03/10 11:53, Pádraig Brady wrote:
>>>> I was surprised that `cp --preserve=all file.xattr /dev/shm` produced no warnings
>>>> In any case, this makes the docs match the behaviour.
>>>
>>> Actually some warnings could be output in this case so my patch is incorrect.
>>> The xattr/selinux diagnostics from cp/mv/install are done like:
>>>
>>> 1. cp -a			outputs no warnings
>>> 2. cp --preserve=all		outputs all but ENOTSUP
>>> 3. cp --preserve=xattr,context	outputs all errors
>>> 4. mv				outputs all but ENOTSUP
>>> 5. install --preserve-context	outputs all but ENOTSUP
>>>
>>> I'm not sure about treating xattr errors differently
>>> to other attribute preservation errors, and in
>>> addition ENOTSUP differently from other errors.
>>> It just seems a bit inconsistent to me.
>>
>> It's a delicate balance.
>> We try very hard not to suppress diagnostics that may indicate a failure.
>
> I can see the tradeoff, but the result is currently confusing.
> For example with /dev/shm (tmpfs) on my Fedora system I can preserve
> security.selinux and system.posix_acl_access xattrs but
> not security.capability or user.*. That's confusing and inconsistent
> in itself, but when combined with ENOTSUP special casing from cp/mv
> one really doesn't know what's going on.
> In other words we shouldn't make any assumptions about what the
> user might expect of a file system.  We really can just print
> error messages if they need the particular attribute preserved
> and keep quiet otherwise.
>
> Hrm I just noticed that you only get an error to copy context
> when the file already exists (so fsetfilecon is used rather
> than setfscreatecon)
>
> $ sudo ./src/cp --preserve=context file vfat/
> $ sudo ./src/cp --preserve=context file vfat/
> ./src/cp: failed to set the security context of `vfat/file' to
> `system_u:object_r:user_home_t:s0': Operation not supported
>
> Note `install` does an unlink() rather than open(O_TRUNC)
> and so will not show an error
>
> $ sudo ./src/ginstall --preserve-context file vfat/
> $ sudo ./src/ginstall --preserve-context file vfat/
>
> I also notice that install.c never sets require_preserve_context=true
> which seems to contradict the description of --preserve-context,
> though the point is somewhat moot due to fsetfilecon not
> being called in any case.

I agree that the current state is unsatisfying.
If you can find a way to improve things without
making commonly used options (cp -a, mv) produce
new warnings, I'm all for it.

Could it be that you want a new option (groan, maybe...)
to remove all of this diagnostic-suppressing behavior?