Prevent user accounting on readonly pool #8424
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
behlendorf
approved these changes
Feb 16, 2019
Codecov Report
@@ Coverage Diff @@
## master #8424 +/- ##
==========================================
+ Coverage 78.43% 78.63% +0.19%
==========================================
Files 380 380
Lines 115901 115903 +2
==========================================
+ Hits 90909 91140 +231
+ Misses 24992 24763 -229
Continue to review full report at Codecov.
|
Trying to mount a dataset from a readonly pool could inadvertently start the user accounting upgrade task, leading to the following failure: VERIFY3(tx->tx_threads == 2) failed (0 == 2) PANIC at txg.c:680:txg_wait_synced() Showing stack for process 2541 CPU: 2 PID: 2541 Comm: z_upgrade Tainted: P O 3.16.0-4-amd64 openzfs#1 Debian 3.16.51-3 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 Call Trace: [<0>] ? dump_stack+0x5d/0x78 [<0>] ? spl_panic+0xc9/0x110 [spl] [<0>] ? dnode_next_offset+0x1d4/0x2c0 [zfs] [<0>] ? dmu_object_next+0x77/0x130 [zfs] [<0>] ? dnode_rele_and_unlock+0x4d/0x120 [zfs] [<0>] ? txg_wait_synced+0x91/0x220 [zfs] [<0>] ? dmu_objset_id_quota_upgrade_cb+0x10f/0x140 [zfs] [<0>] ? dmu_objset_upgrade_task_cb+0xe3/0x170 [zfs] [<0>] ? taskq_thread+0x2cc/0x5d0 [spl] [<0>] ? wake_up_state+0x10/0x10 [<0>] ? taskq_thread_should_stop.part.3+0x70/0x70 [spl] [<0>] ? kthread+0xbd/0xe0 [<0>] ? kthread_create_on_node+0x180/0x180 [<0>] ? ret_from_fork+0x58/0x90 [<0>] ? kthread_create_on_node+0x180/0x180 This patch updates both functions responsible for checking if we can perform user accounting to verify the pool is not readonly. Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
loli10K
force-pushed
the
userspace_upgrade_readonly_pool
branch
from
38463e7 to
456291a
Compare
loli10K
added
Status: Code Review Needed
|
Test added to "upgrade" group. I wish we could include this fix, if/when it is merged, to the zfs-0.7-release branch, it could be useful to rescue legacy pools when imported readonly. |
|
@loli10K the fix looks same enough we might be able to include it in the 0.7 release, let's run it by @tonyhutter. |
alek-p
approved these changes
Feb 19, 2019
behlendorf
added
Status: Accepted
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Trying to rescue data from a dying pool which was imported readonly, used
mount -t zfs -o zfsutil $mntpntto override the mountpoint:This issue seems to affect both master and the zfs-0.7-release branch.
Description
Trying to mount a dataset from a readonly pool could inadvertently start the user accounting upgrade task, leading to the following failure:
This patch updates both functions responsible for checking if we can perform user accounting to verify the pool is not readonly.
How Has This Been Tested?
Tested manually with following snippet:
Will add test case to "upgrade" group later.. DoneTypes of changes
Checklist:
Signed-off-by.