|Summary:||[ZFS] Regression in recv_incremental_replication() after MFC r354116, r354120|
|Product:||Base System||Reporter:||Dmitry Wagin <dmitry.wagin>|
|Component:||kern||Assignee:||Alan Somers <asomers>|
|Severity:||Affects Some People||CC:||andriys, asomers, emaste, frans-jan, freebsd.org, kimmo, mmacy, so, terry-freebsd, tsuroerusu, vsasjason|
Description Dmitry Wagin 2020-09-18 15:33:39 UTC
Created attachment 218061 [details] libzfs_sendrecv.c.diff After https://svnweb.freebsd.org/base?view=revision&revision=354568 > zfs send -R -I pool/test1@snap | zfs recv -F pool/test2 works incorrectly. Snapshots deleted on pool/test1 are not deleted on pool/test2.
Comment 1 Dmitry Wagin 2020-10-17 10:22:09 UTC
Comment 2 Alan Somers 2020-10-17 13:40:30 UTC
"Works incorrectly" is not a bug report. When reporting a bug to FreeBSD or any other project, you should provide a complete description of the problem, including exact steps to reproduce it, the problem's symptoms, and what you expect to happen instead.
Comment 3 Dmitry Wagin 2020-10-19 13:05:33 UTC
(In reply to Alan Somers from comment #2) >Snapshots deleted on pool/test1 are not deleted on pool/test2. ALL steps: > zfs create pool/test1 > zfs snap pool/test1@snap1 > zfs send -R pool/test1@snap1 | zfs recv -F pool/test2 > zfs snap pool/test1@snap2 > zfs send -R -I pool/test1@snap1 pool/test1@snap2 | zfs recv -F pool/test2 > zfs destroy pool/test1@snap1 > zfs snap pool/test1@snap3 > zfs send -R -I pool/test1@snap2 pool/test1@snap3 | zfs recv -F pool/test2 ^^^ after last step, @snap1 should also be deleted on pool/test2.
Comment 4 Alan Somers 2020-10-19 15:55:24 UTC
Reproduced on stable/12 at r366189M. Cannot reproduce on head.
Comment 5 Dmitry Wagin 2020-10-19 16:01:30 UTC
(In reply to Alan Somers from comment #4) HEAD doesn't have this typo.
Comment 6 Alan Somers 2020-10-21 20:36:25 UTC
Created attachment 218957 [details] Fix error merging r354116 from OpenZFS When we merged 4c0883fb4af0d5565459099b98fcf90ecbfa1ca1 from OpenZFS (svn r354116), there were some merge conflicts. One of those was resolved incorrectly, causing "zfs receive" to fail to delete snapshots that a "zfs send -R" stream has deleted. This change corrects that merge conflict, and also reduces some harmless diffs vis-a-vis OpenZFS that were also introduced by r354116. Direct commit to stable/12 because head has moved on to OpenZFS.
Comment 7 andriys 2020-11-15 10:24:24 UTC
Is this going to be fixed, and in case it is, will it be merged to 12.2? I am asking because 12.1 is approaching its EoL, but this issue seems to be serious enough for me to refrain from upgrading to 12.2.
Comment 8 Terry Kennedy 2020-11-15 21:08:08 UTC
(In reply to andriys from comment #7) It doesn't seem to have made it into 12.2-RELEASE as the last code commit to the affected file in the release/12.2.0 tree was on October 6th. I don't know what the plans are to include it in 12.2-p<something>, if any. You might either need to hand merge it yourself or use 12-STABLE instead of 12.2-RELEASE. Note that if you choose this path, freebsd-update will no longer work correctly on your system(s) as it only supports -RELEASE and -RELEASE-p<something> systems, except when it says it specifically supports beta releases during the beta period.
Comment 9 commit-hook 2020-12-01 15:16:09 UTC
A commit references this bug: Author: asomers Date: Tue Dec 1 15:15:19 UTC 2020 New revision: 368233 URL: https://svnweb.freebsd.org/changeset/base/368233 Log: Fix error merging r354116 from OpenZFS When we merged 4c0883fb4af0d5565459099b98fcf90ecbfa1ca1 from OpenZFS (svn r354116), there were some merge conflicts. One of those was resolved incorrectly, causing "zfs receive" to fail to delete snapshots that a "zfs send -R" stream has deleted. This change corrects that merge conflict, and also reduces some harmless diffs vis-a-vis OpenZFS that were also introduced by the same revision. Direct commit to stable/12 because head has moved on to OpenZFS. PR: 249438 Reported by: Dmitry Wagin <email@example.com> Reviewed by: mmacy Sponsored by: Axcient Changes: stable/12/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c stable/12/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h
Comment 10 tsuroerusu 2020-12-02 00:19:02 UTC
Is there any ETA for this fix being issued as an errata for 12.2-RELEASE now that has been committed to stable/12 ?
Comment 11 Alan Somers 2020-12-02 00:42:42 UTC
Only the security officer, so@FreeBSD.org, can approve an errata. Dear Security Officer (CC'd), do you consider this bug serious enough to warrant an errata? The bad stuff is that ZFS snapshots could be wrongly left around after a zfs recv.
Comment 12 tsuroerusu 2020-12-02 00:52:59 UTC
(In reply to Alan Somers from comment #11) Just to add my two cents, I find this to be a pretty big issue because if somebody has an entire setup with a bunch of automated scripts set up to log and/or handle any errors, like snapshots not being deleted, this would mess that up. Frankly, as I understand this issue, it could even be said that this is, de facto, a new behaviour of "zfs recv" in specific situations in the -STABLE branch which goes against the classic FreeBSD ethos of being predictable and not introducing breaking changes and behaviours overnight. By the way, thanks for your efforts in resolving the issue.
Comment 13 Ed Maste 2020-12-02 01:45:06 UTC
(In reply to Alan Somers from comment #11) The errata template is available at https://www.freebsd.org/security/errata-template.txt; if you can fill out as much of that as possible (in particular the background, impact etc. sections) secteam can take care of including it in the next errata batch.