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.
