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

Don't persist temporary pool name on devices #5515

Merged
merged 1 commit into from
Dec 22, 2016

Conversation

loli10K
Copy link
Contributor

@loli10K loli10K commented Dec 21, 2016

Description

Temporary pool names (zpool import -t poolname tempname) should not be persisted to disk (kind of defeats the purpose of being "temporary"). This is a regression accidentally introduced in e0ab3ab.

Motivation and Context

Fixes #5466

How Has This Been Tested?

Tested with script similar to the one included in commit.

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)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Change has been approved by a ZFS on Linux member.

Fix a regression accidentally introduced by e0ab3ab.

Additionally, add a new script zpool_import_014_pos.ksh to the ZFS test suite
to exercise 'zpool import -t' functionality.

Signed-off-by: loli10K <[email protected]>
@mention-bot
Copy link

@loli10K, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jas14, @behlendorf and @ahrens to be potential reviewers.

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. This completely explains the issue, fixes it and adds a test case so it won't happen again. The temporary pool name functionality doesn't exist in OpenZFS which helps explain how this regression snuck in.

@behlendorf behlendorf merged commit 3500a14 into openzfs:master Dec 22, 2016
@loli10K loli10K deleted the issue-5466 branch December 22, 2016 18:51
wli5 pushed a commit to wli5/zfs that referenced this pull request Feb 28, 2017
Fix a regression accidentally introduced by e0ab3ab.

Additionally, add a new script zpool_import_014_pos.ksh to
the ZFS test suite to exercise 'zpool import -t' functionality.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: loli10K <[email protected]>
Closes openzfs#5466 
Closes openzfs#5515
wli5 pushed a commit to wli5/zfs that referenced this pull request Feb 28, 2017
Fix a regression accidentally introduced by e0ab3ab.

Additionally, add a new script zpool_import_014_pos.ksh to
the ZFS test suite to exercise 'zpool import -t' functionality.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: loli10K <[email protected]>
Closes openzfs#5466
Closes openzfs#5515
@loli10K loli10K restored the issue-5466 branch November 1, 2018 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zpool import -t is actually alterting pool names on master
3 participants