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

[Pádraig Brady]

On 11/04/10 16:47, Jim Meyering wrote:
> 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.

Well the patch _reduces_ warnings so that they're only
shown in case 3 above. They should probably also
be output in case 5 also which currently doesn't happen
as explained above.

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

Well I had thought of that. I.E. output all
errors in cases 1,2,4 when --warnings is on.
There is the --verbose option but that also has
the side effect of outputting non warning text.
But the current situation is better than having
a new option I think.

cheers,
Pádraig.