Differential D11920

zfsd(8): Close a race condition when onlining a disk paritition
ClosedPublic
Actions

Authored by asomers on Aug 7 2017, 11:49 PM.

Details

Reviewers

mav
gibbs
allanjude

Commits

rS326298: MFC r322854, r323995, r324568, r324991
rS322854: zfsd(8): Close a race condition when onlining a disk paritition

Summary

zfsd(8): Close a race condition when onlining a disk paritition

When inserting a partitioned disk, devfs and geom will announce the whole
disk before they announce the partition. If the partition containing ZFS
extends to one of the disk's extents, then zfsd will see a ZFS label on the
whole disk and attempt to online it. ZFS is smart enough to activate the
partition instead of the whole disk, but only if GEOM has already created
the partition's provider.

cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
cddl/contrib/opensolaris/lib/libzfs/common/libzfs_import.c
Add a zpool_read_all_labels method. It's similar to
zpool_read_label, but it will return the number of labels found.

cddl/usr.sbin/zfsd/zfsd_event.cc
When processing a DevFS CREATE event, only online a VDEV if we can
read all four ZFS labels.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 10959
Build 11346: arc lint + arc unit

Event Timeline

asomers created this revision.Aug 7 2017, 11:49 PM

Herald added a subscriber: imp. · View Herald TranscriptAug 7 2017, 11:49 PM

Harbormaster completed remote builds in B10937: Diff 31731.Aug 7 2017, 11:49 PM

You haven't included zpool_read_all_labels method into the patch.

I don't have objections other then ones I sounded when you implemented alike functionality in kernel.

Whoops! Here's zpool_read_all_labels

Harbormaster completed remote builds in B10959: Diff 31775.Aug 8 2017, 7:15 PM

Herald added a subscriber: delphij. · View Herald TranscriptAug 8 2017, 7:15 PM

In D11920#247428, @mav wrote:

I don't have objections other then ones I sounded when you implemented alike functionality in kernel.

Presumably you mean D10365. I agree that this seems like a hackish way to verify that we're reading the correct partition. But the label doesn't include the size of the device, so I don't see what other option there is. Adding mediasize to the label would be a change to the ZFS on-disk format, albeit a backwards-compatible one.

Closed by commit rS322854: zfsd(8): Close a race condition when onlining a disk paritition (authored by asomers). · Explain WhyAug 24 2017, 7:48 PM

This revision was automatically updated to reflect the committed changes.