Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix error propagation from lzc_send_redacted #12766

Merged
merged 1 commit into from
Dec 20, 2021
Merged

Fix error propagation from lzc_send_redacted #12766

merged 1 commit into from
Dec 20, 2021

Conversation

toelke
Copy link
Contributor

@toelke toelke commented Nov 15, 2021 •
edited
Loading

Motivation and Context

Any error from lzc_send_redacted is overwritten by the error of send_conclusion_record; so we should skip writing the conclusion record if there was an earlier error.

I noticed this when doing zfs send as user by accident and got a valid empty replication stream that zfs receive would process without any error and without doing anything.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Nov 30, 2021
@behlendorf
Copy link
Contributor

Good find. Would you mind addressing these two checkstyle warnings with the commit.

error: missing "Signed-off-by"
error: commit message body contains line over 72 characters

@toelke
Copy link
Contributor Author

toelke commented Nov 30, 2021

And I would sign it off myself?

@behlendorf
Copy link
Contributor

Yes, please just go ahead and add yourself. Here's a bit more explanation of the rational.

@toelke
Copy link
Contributor Author

toelke commented Nov 30, 2021

Will do. Thank you!

behlendorf
behlendorf approved these changes Dec 17, 2021
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 17, 2021
@toelke
Copy link
Contributor Author

toelke commented Dec 17, 2021

Is there a way I can address the failing tests?

@behlendorf
Copy link
Contributor

@toelke if you could rebase this against the latest master branch and force update the PR we should be able to get a cleaner CI run. You say still see a few unrelated failures for Ubuntu 20.04.

@toelke
Fix error propagation from lzc_send_redacted
4c21323 
Any error from lzc_send_redacted is overwritten by the error of
send_conclusion_record; skip writing the conclusion record if there was
an earlier error.

Signed-Off-By: Philipp Riederer <philipp@riederer.email>
@toelke
Copy link
Contributor Author

toelke commented Dec 18, 2021

I have rebased onto the current master.

@toelke
Copy link
Contributor Author

toelke commented Dec 18, 2021

I do not understand https://github.com/openzfs/zfs/runs/4570245119?check_suite_focus=true -- I have a Signed-Off-By in the commit?!?

@behlendorf
Copy link
Contributor

@toelke it's the capitalization which is causing the warning, it's expecting Signed-off-by. I'll fix it as part of the merge.

@behlendorf behlendorf merged commit 8623bd9 into openzfs:master Dec 20, 2021
@toelke
Copy link
Contributor Author

toelke commented Dec 20, 2021

Thanks!

@toelke toelke deleted the fix-error-propagation-send branch December 20, 2021 19:04
ghost pushed a commit to truenas/zfs that referenced this pull request Jan 19, 2022
@toelke
Fix error propagation from lzc_send_redacted
bbdf8f2 
Any error from lzc_send_redacted is overwritten by the error of
send_conclusion_record; skip writing the conclusion record if there
was an earlier error.

Reviewed-by: Paul Dagnelie <pcd@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Philipp Riederer <philipp@riederer.email>
Closes openzfs#12766
ghost pushed a commit to truenas/zfs that referenced this pull request Jan 20, 2022
@toelke
Fix error propagation from lzc_send_redacted
751015a 
Any error from lzc_send_redacted is overwritten by the error of
send_conclusion_record; skip writing the conclusion record if there
was an earlier error.

Reviewed-by: Paul Dagnelie <pcd@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Philipp Riederer <philipp@riederer.email>
Closes openzfs#12766
ghost pushed a commit to truenas/zfs that referenced this pull request Feb 2, 2022
@toelke
Fix error propagation from lzc_send_redacted
d7c8497 
Any error from lzc_send_redacted is overwritten by the error of
send_conclusion_record; skip writing the conclusion record if there
was an earlier error.

Reviewed-by: Paul Dagnelie <pcd@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Philipp Riederer <philipp@riederer.email>
Closes openzfs#12766
tonyhutter pushed a commit that referenced this pull request Feb 3, 2022
@toelke @tonyhutter
Fix error propagation from lzc_send_redacted
1833de8 
Any error from lzc_send_redacted is overwritten by the error of
send_conclusion_record; skip writing the conclusion record if there
was an earlier error.

Reviewed-by: Paul Dagnelie <pcd@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Philipp Riederer <philipp@riederer.email>
Closes #12766
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
@toelke @nicman23
Fix error propagation from lzc_send_redacted
01e7570 
Any error from lzc_send_redacted is overwritten by the error of
send_conclusion_record; skip writing the conclusion record if there
was an earlier error.

Reviewed-by: Paul Dagnelie <pcd@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Philipp Riederer <philipp@riederer.email>
Closes openzfs#12766
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
@toelke @nicman23
Fix error propagation from lzc_send_redacted
cf296e4 
Any error from lzc_send_redacted is overwritten by the error of
send_conclusion_record; skip writing the conclusion record if there
was an earlier error.

Reviewed-by: Paul Dagnelie <pcd@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Philipp Riederer <philipp@riederer.email>
Closes openzfs#12766
bsdjhb pushed a commit to CTSRD-CHERI/zfs that referenced this pull request Jul 13, 2023
@mmatuska @bsdjhb
lz4_zfs: FreeBSD-local changes to use a custom allocator.
f514117 
This is not associated with a specific upstream commit but apparently
a local diff applied as part of:

commit e92ffd9b626833ebdbf2742c8ffddc6cd94b963e
Merge: 3c3df3660072 17b2ae0
Author: Martin Matuska <mm@FreeBSD.org>
Date:   Sat Jan 22 23:05:15 2022 +0100

    zfs: merge openzfs/zfs@17b2ae0b2 (master) into main

    Notable upstream pull request merges:
      openzfs#12766 Fix error propagation from lzc_send_redacted
      openzfs#12805 Updated the lz4 decompressor
      openzfs#12851 FreeBSD: Provide correct file generation number
      openzfs#12857 Verify dRAID empty sectors
      openzfs#12874 FreeBSD: Update argument types for VOP_READDIR
      openzfs#12896 Reduce number of arc_prune threads
      openzfs#12934 FreeBSD: Fix zvol_*_open() locking
      openzfs#12947 lz4: Cherrypick fix for CVE-2021-3520
      openzfs#12961 FreeBSD: Fix leaked strings in libspl mnttab
      openzfs#12964 Fix handling of errors from dmu_write_uio_dbuf() on FreeBSD
      openzfs#12981 Introduce a flag to skip comparing the local mac when raw sending
      openzfs#12985 Avoid memory allocations in the ARC eviction thread

    Obtained from:  OpenZFS
    OpenZFS commit: 17b2ae0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@pcd1193182 pcd1193182 pcd1193182 approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@toelke @behlendorf @pcd1193182