Bug 2160675
| Summary: | [RFE] cp -p should copy NFSv4 acls | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Ondrej <ondrej.valousek> |
| Component: | coreutils | Assignee: | Lukáš Zaoral <lzaoral> |
| Status: | ASSIGNED --- | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 37 | CC: | admiller, jamartis, jarodwilson, kdudka, kzak, ooprala, ovasik, p, svashisht |
| Target Milestone: | --- | Keywords: | FutureFeature, Patch |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | Type: | Bug | |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
Thank you for working on it upstream! Unfortunately, I do not think that the proposed changes are appropriate for an already released major version of RHEL. There might be deployments out there that rely on the current behavior. One can copy NFSv4 ACLs with `cp --preserve=xattr ...` on RHEL-9. The solution you propose should rather be introduced in RHEL-10. I see, bit unfortunate, but understandable. What about Fedora 37/38 at least? also, I see no changes to xattr.conf would be needed for RHEL-8, so perhaps chance to backport there? Not really. We should not introduce new features into RHEL-8 if they are not available in RHEL-9, because customers would see regressions after a RHEL-8 -> RHEL-9 upgrade. Moreover, the changed behavior of `cp -p` in a minor update could be a problem on its own. Putting the changes to Fedora now is a good idea. This should give us some feedback before it goes to RHEL-10. (In reply to Ondrej from comment #0) > Note, you'd also need to revert the changes you made to /etc/xattr.conf to make this work. Why do we actually need to make any changes to xattr.conf? `cp -p` does not seem to read the file anyway. I think one needs to use `cp --preserve=xattr`, `cp -a`, or such to copy extended attributes. With the patch I made upstream, we do read xattr.conf in file qcopy-acl.c in Gnulib. Hence, if you need the "cp -p" to copy Posix ACLs as well as NFSv4 ACLs, the xattr.conf needs to be restored to default (i.e. must contain "system.nfs4_acl"). I think it's just more appropriate - people do expect cp -p would copy ACLs, no matter which, right? Actually that reminded me of another bug - "man cp" says nothing about the fact that cp --preserve=mode also preserves Posix ACLs (and with this patch applied, we also preserve NFSv4 acls). I think it's fair to have a note about this behavior in the man page. Thanks for explanation! The cp(1) man page is generated from the output of `cp --help`. So we need to patch src/cp.c in coreutils to change it. The upstream changes are now included in coreutils-9.2-1.fc39 and newer. Hmm, unfortunately it does not work for me. Just upgraded freshly to F39, coreutils-9.3-1.fc39.x86_64 and "ls -l" does not detect NFSv4 ACLs any longer But cp -p appears to handle NFSv4 ACLs just fine. So there must be smth wrong with the file-has-acl.c...? I think so. There were some massive changes after your patch was merged: https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=35bd46f0c816948dc1a0430c8ba8b10a01167320 https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=d65e5a8ba77595a598c9ddb8dfa09c4aea732659 Well the problem is actually in getxattr() call which no longer returns ENOTSUP on Fedora39 as it did on previous systems. I have sent a patch to Gnulib with a workaround, but I am not that excited with the patch as it yields into one more getxattr() call -> the function will be bit slower. There is an ongoing discussion upstream: https://lists.gnu.org/archive/html/bug-gnulib/2023-05/msg00092.html |
Description of problem: Kamile, kindly please merge latest changes to Gnulib that implement the changes requested, see the recent commit: commit eb6a8a4dfbe89f9f74666df082255cfefa10a407 Author: Ondrej Valousek <ondrej.valousek.xm> Date: Wed Jan 4 15:34:26 2023 +0100 qcopy-acl: Optimize copying of ACLs by directly copying the attributes. * lib/qcopy-acl.c (is_attr_permissions): New functions. (qcopy_acl): If USE_XATTR, copy the ACL related attributes directly. * m4/xattr.m4: New file. * modules/qcopy-acl (Files): Add it. (configure.ac): Invoke gl_FUNC_XATTR. which implements the requested functionality. Note, you'd also need to revert the changes you made to /etc/xattr.conf to make this work. Many thanks