Bug 249438 - [ZFS] Regression in recv_incremental_replication() after MFC r354116, r354120
Summary: [ZFS] Regression in recv_incremental_replication() after MFC r354116, r354120
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.1-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Alan Somers
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2020-09-18 15:33 UTC by Dmitry Wagin
Modified: 2021-11-02 19:48 UTC (History)
15 users (show)

See Also:


Attachments
libzfs_sendrecv.c.diff (595 bytes, patch)
2020-09-18 15:33 UTC, Dmitry Wagin
no flags Details | Diff
Fix error merging r354116 from OpenZFS (5.05 KB, patch)
2020-10-21 20:36 UTC, Alan Somers
no flags Details | Diff
Partially complete errata notice (3.93 KB, text/plain)
2020-12-02 02:38 UTC, Alan Somers
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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
ping
Comment 2 Alan Somers freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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 <dmitry.wagin@ya.ru>
  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 Troels Just 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 freebsd_committer freebsd_triage 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 Troels Just 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 freebsd_committer freebsd_triage 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.
Comment 14 Alan Somers freebsd_committer freebsd_triage 2020-12-02 02:38:03 UTC
Created attachment 220151 [details]
Partially complete errata notice

Here is a partially completed errata notice that secteam can use, if they decide to issue an errata for this.
Comment 15 Anton Saietskii 2020-12-21 11:06:30 UTC
Any updates on this?
Comment 16 andriys 2021-01-24 14:35:21 UTC
Any updates on merging this fix to 12.2, please? The https://www.freebsd.org/releases/12.2R/errata.html says "An Errata Notice is planned for 12.2-RELEASE" for this issue. And with 12.1 approaching its EOL in just one week this is getting pretty important.
Comment 17 Troels Just 2021-01-24 14:43:19 UTC
(In reply to andriys from comment #16)
I agree with this, especially considering that an errata was actively promised at the release of 12.2.

I could understand if we were talking about an errata for some obscure utility in base that has few uses but this is something as huge as ZFS.

So, I would hope that the Security Team, so@FreeBSD.org, would give priority to this considering the explicit promise of an errata.
Comment 18 Philip Paeps freebsd_committer freebsd_triage 2021-01-26 08:33:59 UTC
This is in the pipeline for the upcoming batch of errata.  freebsd-update builds are running now.
Comment 19 commit-hook freebsd_committer freebsd_triage 2021-01-29 01:21:24 UTC
A commit in branch releng/12.2 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=545f860883cf02fc152696186436ffe6adf8f1a7

commit 545f860883cf02fc152696186436ffe6adf8f1a7
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2020-12-01 15:15:18 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2021-01-29 01:14:30 +0000

    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 <dmitry.wagin@ya.ru>
    Reviewed by:    mmacy
    Sponsored by:   Axcient
    (cherry picked from commit 861515418ac385f4198c38c28f6203135d72e651)

    Approved by:    so

 .../lib/libzfs/common/libzfs_sendrecv.c            | 43 +++++++++++-----------
 .../contrib/opensolaris/uts/common/sys/fs/zfs.h    | 14 +++----
 2 files changed, 28 insertions(+), 29 deletions(-)
Comment 20 Ahmed 2021-11-02 19:48:12 UTC
MARKED AS SPAM