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.

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

Fix error text for EINVAL in zfs_receive_one() #8977

Merged
merged 1 commit into from
Jul 3, 2019

Conversation

tcaputi
Copy link
Contributor

@tcaputi tcaputi commented Jul 1, 2019

This small patch fixes the EINVAL case for zfs_receive_one(). A
missing 'else' has been added to the two possible cases, which
will ensure the intended error message is printed.

Signed-off-by: Tom Caputi tcaputi@datto.com

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

This small patch fixes the EINVAL case for zfs_receive_one(). A
missing 'else' has been added to the two possible cases, which
will ensure the intended error message is printed.

Signed-off-by: Tom Caputi <tcaputi@datto.com>
@behlendorf behlendorf requested a review from loli10K July 1, 2019 18:18
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 1, 2019
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"kernel modules must be upgraded to "
"receive this stream."));
if (embedded && !raw)
} else if (embedded && !raw) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

encrypted is not the same as raw. encrypted means that the resulting dataset from the receive was encrypted. Raw means that the encrypted dataset was sent using the raw send feature flag. It is confusing, and a guess at best, but this code should be more accurate than what is there now. I have proposed more general fixes for error reporting in cases like this, which I will hopefully get to some time soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

i also found this:

https://github.com/zfsonlinux/zfs/blob/681a85cb01a9f83b9f742d8fb634edddc68c5679/module/zfs/dmu_recv.c#L573-L580

embedded data is incompatible with encryption and raw recv

unless i'm missing something we should theoretically handle EINVAL when both DMU_BACKUP_FEATURE_RAW and DMU_BACKUP_FEATURE_EMBED_DATA are set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should never happen. The flags are mutually exclusive on the send side. There are many places where the recv code can return EINVAL that will be misinterpreted. The idea of this patch is not to correct all of these cases, but simply to make this case (that a few people have hit) function as it was originally intended. We will address the broader problem of badly interpreted error codes in another pull request.

@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #8977 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8977      +/-   ##
==========================================
- Coverage   78.74%   78.72%   -0.02%     
==========================================
  Files         388      388              
  Lines      119972   119972              
==========================================
- Hits        94466    94447      -19     
- Misses      25506    25525      +19
Flag Coverage Δ
#kernel 79.46% <ø> (-0.05%) ⬇️
#user 66.64% <100%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 681a85c...767871c. Read the comment docs.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jul 2, 2019
@behlendorf behlendorf merged commit 2ba59fa into openzfs:master Jul 3, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 13, 2019
This small patch fixes the EINVAL case for zfs_receive_one(). A
missing 'else' has been added to the two possible cases, which
will ensure the intended error message is printed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: loli10K <ezomori.nozomu@gmail.com>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes openzfs#8977
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 22, 2019
This small patch fixes the EINVAL case for zfs_receive_one(). A
missing 'else' has been added to the two possible cases, which
will ensure the intended error message is printed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: loli10K <ezomori.nozomu@gmail.com>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes openzfs#8977
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 23, 2019
This small patch fixes the EINVAL case for zfs_receive_one(). A
missing 'else' has been added to the two possible cases, which
will ensure the intended error message is printed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: loli10K <ezomori.nozomu@gmail.com>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes openzfs#8977
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 17, 2019
This small patch fixes the EINVAL case for zfs_receive_one(). A
missing 'else' has been added to the two possible cases, which
will ensure the intended error message is printed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: loli10K <ezomori.nozomu@gmail.com>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes openzfs#8977
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 18, 2019
This small patch fixes the EINVAL case for zfs_receive_one(). A
missing 'else' has been added to the two possible cases, which
will ensure the intended error message is printed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: loli10K <ezomori.nozomu@gmail.com>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes openzfs#8977
tonyhutter pushed a commit that referenced this pull request Sep 26, 2019
This small patch fixes the EINVAL case for zfs_receive_one(). A
missing 'else' has been added to the two possible cases, which
will ensure the intended error message is printed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: loli10K <ezomori.nozomu@gmail.com>
Signed-off-by: Tom Caputi <tcaputi@datto.com>
Closes #8977
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants