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

ztest creates partially initialized root dataset #8277

Merged
merged 1 commit into from
Jan 18, 2019

Conversation

loli10K
Copy link
Contributor

@loli10K loli10K commented Jan 13, 2019

Motivation and Context

Since d8fdfc2 was integrated dsl_pool_create() does not call dmu_objset_create_impl() for the root dataset when running in userland (ztest): this creates a pool with a partially initialized root dataset. Trying to import and use this pool results in both zpool and zfs executables dumping core.

root@linux:~# cat /proc/sys/kernel/spl/gitrev 
zfs-0.8.0-rc2-65-g6955b40
root@linux:~# ztest -p issue -T 10 -f /var/tmp/ -v 1 -s 128m -k 0 > /dev/null 2>&1
root@linux:~# echo $?
0
root@linux:~# zpool import -d /var/tmp/ issue
Aborted
root@linux:~# zfs list
Aborted
root@linux:~# ulimit -c unlimited 
root@linux:~# zfs list
Aborted (core dumped)
root@linux:~# gdb -q zfs core 
Reading symbols from zfs...done.
[New LWP 27733]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `zfs list'.
Program terminated with signal SIGABRT, Aborted.
#0  0x00007f01922c1067 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56	../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007f01922c1067 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007f01922c2448 in __GI_abort () at abort.c:89
#2  0x00007f01930a1ed6 in make_dataset_handle_common (zhp=0x253d480, zc=0x7ffc25c96ea0) at libzfs_dataset.c:450
#3  0x00007f01930a2075 in make_dataset_handle (hdl=0x2533060, path=0x2534d30 "issue") at libzfs_dataset.c:488
#4  0x00007f019309e024 in zfs_iter_root (hdl=0x2533060, func=0x405738 <zfs_callback>, data=0x7ffc25c9a4e0) at libzfs_config.c:449
#5  0x00000000004062db in zfs_for_each (argc=0, argv=0x2534cf0, flags=6, types=(ZFS_TYPE_FILESYSTEM | ZFS_TYPE_SNAPSHOT | ZFS_TYPE_VOLUME), sortcol=0x0, proplist=0x7ffc25c9a610, limit=0, callback=0x40c9ba <list_callback>, data=0x7ffc25c9a600) at zfs_iter.c:440
#6  0x000000000040cdbe in zfs_do_list (argc=0, argv=0x2534cf0) at zfs_main.c:3420
#7  0x00000000004163fa in main (argc=2, argv=0x7ffc25c9a788) at zfs_main.c:8042
(gdb) frame 2
#2  0x00007f01930a1ed6 in make_dataset_handle_common (zhp=0x253d480, zc=0x7ffc25c96ea0) at libzfs_dataset.c:450
450			abort();
(gdb) list
445		else if (zhp->zfs_dmustats.dds_type == DMU_OST_ZFS)
446			zhp->zfs_head_type = ZFS_TYPE_FILESYSTEM;
447		else if (zhp->zfs_dmustats.dds_type == DMU_OST_OTHER)
448			return (-1);
449		else
450			abort();
451	
452		if (zhp->zfs_dmustats.dds_is_snapshot)
453			zhp->zfs_type = ZFS_TYPE_SNAPSHOT;
454		else if (zhp->zfs_dmustats.dds_type == DMU_OST_ZVOL)
(gdb) p zhp->zfs_name 
$1 = "issue", '\000' <repeats 250 times>
(gdb) p zhp->zfs_dmustats.dds_type
$2 = DMU_OST_NONE
(gdb) 

This was discovered while testing #8181 (see buildbot ztest failures)

Description

Fix this by adopting an alternative change suggested in OpenZFS 8607 code review (https://www.illumos.org/rb/r/666/#comment2909).

How Has This Been Tested?

Run ztest, import "ztest" pool

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:

Since d8fdfc2 was integrated dsl_pool_create() does not call
dmu_objset_create_impl() for the root dataset when running in
userland (ztest): this creates a pool with a partially initialized
root dataset. Trying to import and use this pool results in both
zpool and zfs executables dumping core.

Fix this by adopting an alternative change suggested in OpenZFS 8607
code review.

Original-patch-by: Robert Mustacchi <[email protected]>
Signed-off-by: loli10K <[email protected]>
@loli10K loli10K added Status: Work in Progress Not yet ready for general review Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Jan 13, 2019
@codecov
Copy link

codecov bot commented Jan 14, 2019

Codecov Report

Merging #8277 into master will increase coverage by 8.71%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8277      +/-   ##
==========================================
+ Coverage   68.24%   76.96%   +8.71%     
==========================================
  Files         335      369      +34     
  Lines      109770   114842    +5072     
==========================================
+ Hits        74917    88387   +13470     
+ Misses      34853    26455    -8398
Flag Coverage Δ
#kernel 76.8% <100%> (+9.74%) ⬆️
#user 66.7% <100%> (+6.38%) ⬆️

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 6955b40...c65f730. Read the comment docs.

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.

Thanks! I noticed this myself a little while back and was meaning to investigate.

@behlendorf behlendorf requested a review from tonyhutter January 17, 2019 23:42
Copy link
Contributor

@tcaputi tcaputi left a comment

Choose a reason for hiding this comment

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

LGTM. good catch.

@behlendorf behlendorf merged commit 0a10863 into openzfs:master Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants