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

Invariant Disks #258

Merged
merged 33 commits into from
Dec 25, 2014
Merged

Invariant Disks #258

merged 33 commits into from
Dec 25, 2014

Conversation

ilovezfs
Copy link
Contributor

@ilovezfs ilovezfs commented Dec 5, 2014

No description provided.

InvariantDisks is a small program maintaining a mapping from invariant
labels to the potentially varying /dev/disk* entries. Only the CLI
skeleton and other infrastructure exists.
The DiskArbitrationDispatcher dispatches appearance and disappearance
events from disks to the registered handlers. The log handler just logs
the disks with the aid of the disk arbitration utilities.
This program is licensed under a "3-Clause BSD" license.
Renamed from DiskArbitrationLogger, which is a very unwieldy name.
Create symlinks from a media path derived link to the associated
dev files. This is only possible for disks that have an IODeviceTree
media path.
Also extract the volume path and the media content keys from the
DADiskRef. An IOKit dictionary is also created as preparation for
additional information demand.
More details, and using the correct project name.
Use 755 instead of 744 when creating new directories, which gives
list rights to group and others as well. Simplified directory
creation code. Dead code is not needed.
Only create a DiskInformation object once per DADiskRef in the
dispatcher. That allows DiskArbitrationHandler to reuse the information
which improves efficiency.
Make the logging much more readable, with just one data element per line.
Links disks/volumes by media/volume/device ID. I am unclear what
the various IDs imply, but disk images have volume UUIDs, while
normal volumes and core-storage volumes have both media and volume
UUIDs, and some volumes only have media UUIDs.
Add the device serial number and some flags to the DiskInformation
object. The serial number is available in all devices, volumes and
even core storage devices on a particular device. The device model
is also available.
Links whole disks by their device serial number. The model and serial
number are both used in the symlinks for more human readability. There
is no straight-forward way to identify physical whole disks, but the
implemented heuristic seems to work well.
The new features are now properly described in the readme file.
Instead of checking for potential issues with symlink creation/
removal, just try it and handle the errors. The errors are now also
made more human readable.
The verbosity of the logging can now be changed by adding -v. By default,
the Program is significantly less talkative.
Command line parsing is now more scalable and more reliable. A map of
Handler for the command line flags replaces counting and searching in
the flags. Not quite boost::program_options but ...
@evansus
Copy link
Contributor

evansus commented Dec 6, 2014

Cool - one question, can the launchd plist specify a dependency to load prior zpool import? Or if it already does so, is it simply loading the service/plist by lexicographical / alphabetical order? e.g.

org.openzfsonosx.zed.service.plist
org.openzfsonosx.zpool-autoimport.plist
org.openzfsonosx.zpool-import-all.plist
org.openzfsonosx.InvariantDisks.plist <-- launchd loads this one earlier?

I haven't had the time to complete it, but I have on multiple occasions begun porting find_by_guid into vdev_disk - using minimally IOKit c++ to locate disks, and only in the case of a mismatch.

On one hand the end result is almost the same, except the output of zpool list -v, zpool status, and zdb is better with invariant disks.
On the other hand, invariant disks relies on an external launchd service to find the disks, which seems to me to leave room for error. As you already know, I'm partial to changes that assist with bootable pools ;)

@ilovezfs
Copy link
Contributor Author

ilovezfs commented Dec 6, 2014

@evansus I don't think launchd supports dependencies in that way, but perhaps we could have import all wait for a cookie file that invariant disks could place in /tmp after its first run as we do with zfs_util:
https://github.com/openzfsonosx/zfs/search?utf8=✓&q=ZPOOL_IMPORT_ALL_COOKIE

Other ideas?

@evansus have you tried invariant + vdev_iokit yet?

@evansus
Copy link
Contributor

evansus commented Dec 6, 2014

@ilovezfs Sounds good, I just checked out the Mac developer reference on launchd and I see that this is the recommended behavior.

And to answer the other question, no, but I already determined that additional work is needed for it to resolve the symlink to the disk device. vdev-iokit currently does not handle symlinks properly. I haven't needed it, with the find_by_guid logic, but it does mean that zpool import -d or zpool create using symlinks fails.

Partitions are now also linked in by-serial, with a media-path style
colon + partitionId suffix. This allows using stable IDs without a
fixed partitioning scheme.
A tiny bit more consistency.
In addition to the 3-Clause BSD License, also license under the CDDL.
@ilovezfs ilovezfs force-pushed the invariant branch 3 times, most recently from e811605 to e9b95eb Compare December 13, 2014 05:09
Apparently there's no clear Standard on how to label Serial Numbers,
so try different strings, and just take the first.
cbreak-black and others added 12 commits December 13, 2014 15:36
Don't try to trim if there's nothing found to keep. This would lead
to an out-of-bounds exception.
The new idle handler can do things when DiskArbitration has not
given any new disks in a user-defined time span. This uses a timer
dispatch source, so some of the dispatch related code was moved into
its own files.
Create a file to allow scripts and other API limited entities to
detect if InvariantDisk has been idle. The file is deleted when
ever disk changes happen, and created after the idle timeout has passed.
Only use non-empty model names for generating a serial number. Do
not format and link a serial number if the serial number is not
sufficiently non-empty.
Correctly pass through empty serials as empty, so they can later be
ignored.
…9efa1e6e9d'

git-subtree-dir: cmd/InvariantDisks
git-subtree-mainline: 4b4e80d
git-subtree-split: bb56ecc
ilovezfs added a commit that referenced this pull request Dec 25, 2014
@ilovezfs ilovezfs merged commit 7731d2d into master Dec 25, 2014
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.

3 participants