Bug 249119 - net/rsync: (3.2.3) fails to copy ZFS ACLs
Summary: net/rsync: (3.2.3) fails to copy ZFS ACLs
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Rodrigo Osorio
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-04 23:12 UTC by Peter Eriksson
Modified: 2021-04-21 16:18 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (rodrigo)


Attachments
Patch to add back NFSv4 ACL support (24.32 KB, patch)
2020-10-29 18:29 UTC, walker.aj325
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Eriksson 2020-09-04 23:12:33 UTC
rsync 3.2.3 doesn't seem to copy ZFS ACLs correctly for some reason. 

FreeBSD 12.1

root# getfacl tst2
# file: tst2
# owner: root
# group: employee-liu.se
            owner@:rwxpDdaARWcCos:fd----I:allow
      user:parst38:r-----a-R-c---:-------:allow
            group@:--------------:fd----I:allow
         everyone@:--------------:fd----I:allow

root# getfacl tst
# file: tst
# owner: peter86
# group: employee-liu.se
            owner@:rwxpDdaARWcCos:fd----I:allow
            group@:--------------:fd----I:allow
         everyone@:--------------:fd----I:allow

root# rsync -aA -vv tst2/ tst
sending incremental file list
delta-transmission disabled for local transfer or --whole-file
./
total: matches=0  hash_hits=0  false_alarms=0 data=0

sent 76 bytes  received 86 bytes  324.00 bytes/sec
total size is 0  speedup is 0.00

root# getfacl tst
# file: tst
# owner: root
# group: employee-liu.se
            owner@:rwxpDdaARWcCos:fd----I:allow
            group@:--------------:fd----I:allow
         everyone@:--------------:fd----I:allow
Comment 1 Peter Eriksson 2020-09-05 16:57:31 UTC
Hmm... Reading the acl code in rsync it seems it just supports POSIX ACLs. So not so surprising that it doesn't really transfers any ACLs as it is.

In the extrapatch-acl file from the sync 3.1 port there is some NFS4 ACL support that probably should/could be forward-ported to 3.2

(Even though that patch isn't 100% - it has a tendency of causing retransmission again and again)


There is some half-baked code in 3.2 that maps OSX ACLs (which uses almost the same calls as FreeBSD for ACLs) into something that can be sent over the wire but in my opinion this whole code should be rewritten and probably use some standard representation (probably based on NFS4/Windows/ZFS ACLs) - with the added benefit for cross-OS compatibility. But that is probably something for the rsync folks and not a FreeBSD port...
Comment 2 walker.aj325 2020-10-29 18:28:20 UTC
I noticed this problem as well as I was testing the updated rsync. We also need to clean up references to "acl.diff" in the Makefile. I have attached a WIP patch for the port.
Comment 3 walker.aj325 2020-10-29 18:29:24 UTC
Created attachment 219205 [details]
Patch to add back NFSv4 ACL support
Comment 4 Rodrigo Osorio freebsd_committer 2021-01-17 09:59:18 UTC
Hi,

As ZFS is now a well-shared feature across many Unixes,
I suggest you to submit the patch to upstream project.

Patches in ports tree are not intended to add new features
or fix implementations, but only to fix non-freebsd behaviors.

All the best
-- rodrigo
Comment 5 walker.aj325 2021-01-20 13:24:22 UTC
Excuse me, but this has nothing to do with ZFS support. This is fixing NFSv4 ACL support, which is FreeBSD specific. It's also a significant issue for FreeBSD users because without the patch permissions will be stripped when trying to rsync between FreeBSD servers. This is also not adding a new feature, it is adding something back which was stripped out. Without this rsync is significantly broken for a large number of FreeBSD users.
Comment 6 Kris Moore freebsd_committer 2021-01-20 13:32:20 UTC
Re-opening this. Please re-read the comments, since this is not ZFS related. Patch to correct the expected behavior is attached.
Comment 7 Rodrigo Osorio freebsd_committer 2021-01-21 00:13:41 UTC
(In reply to Kris Moore from comment #6)

Thanks Kris for the heads up, but I assure you,
I'm totally capable to reopen the PRs on my own.

@all, few questions:

I totally understand that this issue is about ACL management,
and I suppose the proposed patch covers the ZFS ACL issues, right ?

That said, if we are reintroducing rsync 3.1 code into rsync 3.2,
we probably should ask why this code was originally removed, and if
is not better to push/request the changes to the upstream project.

You can probably understand that  I'm reluctant to push a complex
WIP patch into a production version of rsync without more details.

Regards,
-- rodrigo
Comment 8 walker.aj325 2021-04-21 16:18:45 UTC
(In reply to Rodrigo Osorio from comment #7)
> I totally understand that this issue is about ACL management,
and I suppose the proposed patch covers the ZFS ACL issues, right ?

It does cover ZFS ACL issues in that they are NFSv4 ACLs.

>That said, if we are reintroducing rsync 3.1 code into rsync 3.2,
we probably should ask why this code was originally removed, and if
is not better to push/request the changes to the upstream project.

This is not really reintroducing rsync 3.1 code. It's updating a FreeBSD ports patchset that implements a FreeBSD-specific rsync feature that has existed for quite a long time. This feature was _broken_ during the porting of rsync 3.2 in a terrifically bad manner in that rsync 3.2 now _strips_ NFSv4 ACLs from files. It would be good to know why it was removed from the FreeBSD port of rsync when 3.2 was introduced. This is a significant breakage of existing functionality. I don't see this discussed anywhere.