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

Activate filesystem features only in syncing context #14304

Merged
merged 1 commit into from
Jan 12, 2023
Merged

Activate filesystem features only in syncing context #14304

merged 1 commit into from
Jan 12, 2023

Conversation

gamanakis
Copy link
Contributor

@gamanakis gamanakis commented Dec 20, 2022 •
edited
Loading

Motivation and Context

Closes #14252

Description

When activating filesystem features after receiving a snapshot, do so only in syncing context.

How Has This Been Tested?

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:

pharshalp
pharshalp reviewed Dec 20, 2022
View reviewed changes
module/zfs/dsl_dataset.c Outdated Show resolved Hide resolved
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 20, 2022
@gamanakis gamanakis marked this pull request as ready for review December 21, 2022 11:46
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 22, 2022
@mabod mabod mentioned this pull request Dec 24, 2022
Closed
@gamanakis
Copy link
Contributor Author

We probably should restrict this piece of code to be executed only in case of raw receiving. Looking into it.

@gamanakis
Copy link
Contributor Author

Restricting to raw receiving doesn't help the original case: The problem arises already when the receive is done and the feature flags are not set prior to raw sending.

@gamanakis
Copy link
Contributor Author

@behlendorf this patch does not address all the issues with #14252. I am looking into it.

@gamanakis gamanakis marked this pull request as draft December 29, 2022 20:53
@gamanakis gamanakis marked this pull request as ready for review January 1, 2023 08:12
@gamanakis gamanakis requested review from behlendorf and a user and removed request for behlendorf and a user January 1, 2023 08:12
@gamanakis
Copy link
Contributor Author

I updated the PR to address all the issues with #14252. I do not think I introduced any new regressions.

@gamanakis
Copy link
Contributor Author

@rincebrain would you mind me adding the test you provided in #14252 here?

@rincebrain
Copy link
Contributor

By all means, go for it. I'm sure there's a simpler test case that works, but I haven't spent any time trying to minimize it, since I wasn't even...trying to make it when I did. Heh.

@gamanakis
Copy link
Contributor Author

Added a test.

@ryao ryao mentioned this pull request Jan 11, 2023
Merged
13 tasks
@behlendorf
Copy link
Contributor

It seems like we're still running in to an issue with the new test case:

http://build.zfsonlinux.org/builders/FreeBSD%20stable%2F13%20amd64%20%28TEST%29/builds/7600/steps/shell_4
/logs/summary

@gamanakis
Copy link
Contributor Author

@behlendorf should be an easy fix, I think it doesn't like the order of the arguments. I will get to it later today.

@gamanakis
Activate filesystem features only in syncing context
b16c67e 
Signed-off-by: George Amanakis <gamanakis@gmail.com>
@gamanakis
Copy link
Contributor Author

done!

@behlendorf
Copy link
Contributor

Much better, thanks!

@behlendorf behlendorf merged commit eee9362 into openzfs:master Jan 12, 2023
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Jan 13, 2023
@gamanakis @tonyhutter
Activate filesystem features only in syncing context
8ba9327 
When activating filesystem features after receiving a snapshot, do
so only in syncing context.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Amanakis <gamanakis@gmail.com>
Closes openzfs#14304
Closes openzfs#14252
@peterska
Copy link

Will this be backported to 2.1.8 ?

@ryao
Copy link
Contributor

ryao commented Jan 17, 2023 •
edited
Loading

Will this be backported to 2.1.8 ?

It is already in the 2.1.8 PR.

tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Jan 18, 2023
@gamanakis @tonyhutter
Activate filesystem features only in syncing context
3d5111f 
When activating filesystem features after receiving a snapshot, do
so only in syncing context.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Amanakis <gamanakis@gmail.com>
Closes openzfs#14304
Closes openzfs#14252
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Jan 19, 2023
@gamanakis @tonyhutter
Activate filesystem features only in syncing context
f806306 
When activating filesystem features after receiving a snapshot, do
so only in syncing context.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Amanakis <gamanakis@gmail.com>
Closes openzfs#14304
Closes openzfs#14252
archlinux-github pushed a commit to archlinux/aur that referenced this pull request Feb 9, 2023
@kstolp
Backport fix for issue related to send/receive.
adff772 
Issue: openzfs/zfs#14252
Pull request: openzfs/zfs#14304
archlinux-github pushed a commit to archlinux/aur that referenced this pull request Feb 9, 2023
@kstolp
Backport fix for issue related to send/receive.
184e232 
Issue: openzfs/zfs#14252
Pull request: openzfs/zfs#14304
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 3, 2023
@gamanakis @lundman
Activate filesystem features only in syncing context
9081c75 
When activating filesystem features after receiving a snapshot, do 
so only in syncing context.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Amanakis <gamanakis@gmail.com>
Closes openzfs#14304 
Closes openzfs#14252
allanjude pushed a commit to allanjude/zfs that referenced this pull request Apr 7, 2023
@gamanakis @allanjude
Activate filesystem features only in syncing context
3e0468b 
When activating filesystem features after receiving a snapshot, do
so only in syncing context.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: George Amanakis <gamanakis@gmail.com>
Closes openzfs#14304
Closes openzfs#14252
(cherry picked from commit eee9362)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pharshalp pharshalp pharshalp left review comments

@behlendorf behlendorf behlendorf approved these changes

@ryao ryao ryao 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.

zfs-2.1.7 zfs recv panic in dsl_dataset_deactivate_feature_impl
6 participants
@gamanakis @rincebrain @behlendorf @peterska @ryao @pharshalp