Add events to zed and minor tweaks.

[lundman]

Collected all zed changes into one PR.

Motivation and Context

Description

This PR comes in 3 parts;

1. Add events for macOS, mounting snapshots, creating zvol symlinks
2. Work around different pthread setname. macOS can only set name from child thread.
3. Compile fixes for Werror, add includes etc.

How Has This Been Tested?

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)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[tonyhutter]

@lundman I'm back from vacation - I'll take a look at this.

[tonyhutter]

Would you mind squashing these commits? There seems to be some churn between them.

[lundman]

Would you mind squashing these commits? There seems to be some churn between them.

Not at all, one-commit next push.

[lundman]

I'm also wondering if for the pthread names, instead of #ifdefs everywhere, perhaps we could define 2 macros:
zfs_pthread_setname(1 arg)
zfs_pthread_setname_tid(2 args)

use both args in zed source files, but one macro is empty, depending on platform?

[tonyhutter]

I personally like the more verbose #ifdefs, because it's apparent that you're doing something special for OSX for the pthread names. If you use the macros, it could lead the casual reader getting confused:

        pthread_setname_tid(g_agents_tid, "agents");
	< spawn thread>
	        pthread_setname("agents");

"dude... why is it setting the thread name twice?"

[tonyhutter]

Once the last two outstanding comments get fixed, I think this will be good to go.

[lundman]

I believe I have updated for both comments, let me know if you want further tweaking.

[behlendorf]

I really don't have a good solution to the pthread_setname_np() issue since it's used so differently. It seems like we might need to live with the #ifdefs.

It seems to me it would be useful to post these new events on all platforms. That would allow users to create their own zedlets to do interesting things with them. Even if we still for example on Linux rely on udev to create the links and not the ZED. Plus it means we won't be merging any dead code with this PR, which would be good.

[tonyhutter]

Thanks for sticking with this @lundman

[lundman]

I'm just happy we are allowed to use zed - otherwise we'd have to write our own ;)

[behlendorf]

Just a matter of figuring out where in fbsd/linux to place the calls.

For Linux you'll want to place the call in the error == 0 block at the end of zvol_os_create_minor(). It looks like a pretty similar story for FreeBSD with zvol_create_minor_impl(). Though looking it this again two more questions come to mind.

1. Do we really want the kernel to provide a normal and raw symlink path in the event? I may be biased because of how udev works, but it seems like we'd want to leave this policy decision in user space and let the zedlet construct the right path from the zvol name.
2. There's also the zvol_rename_minor() functionality which has basically this same problem. Was the intent to generate this same event for renames? We also had a hook for deleting the symlink in another PR, right?

The zedlets would need something like #if linux // do nothing #else if macOS // "zfs mount"

Yes, that's pretty much what I had in mind. Although now I'm second guessing myself since I'm not sure we really have a solid use case for it yet on other platforms. For the moments let's leave it as you had it originally and not call zfs_ereport_snapshot_post() from anywhere yet.

[lundman]

Do we really want the kernel to provide a normal and raw symlink path in the event? I may be biased because of how udev works, but it seems like we'd want to leave this policy decision in user space and let the zedlet construct the right path from the zvol name.

I think your device names makes sense on linux, I don't actually know what udev produces? At least on illumos, you know the zvol is /dev/zvol/dsk/$pool/$volume right, is that what you mean? On macOS, you don't know what it will be called. So I have to pass the /dev/disk15 (or whatever it might be), so we can make the (user friendly) named-symlink /var/run/zfs/zvol/dsk/$pool/$volume to point to it. It's hard to stick /dev/disk15 into say /etc/fstab if the device gets called /dev/disk14 next boot.

The zedlet only get pool/volume and /dev/disk15. Then it generated the symlink name;
https://github.com/openzfsonosx/openzfs/blob/development/cmd/zed/zed.d/zvol.create.sh
However, there is the raw disk /dev/rdisk15. macOS doesn't need it, and can probably guess/generate it. I left it in just in-case some future OS has something unrelated. But maybe that is a bit premature.

Yes, that's pretty much what I had in mind. Although now I'm second guessing myself

Leaving it as it is.

[behlendorf]

Thanks for linking the zedlet that helped clear things up. I see what you're saying, that makes sense. Just thinking out loud, but would having the device major:minor numbers be helpful as well?

I don't actually know what udev produces?

That's actually sounds pretty similar to how it currently works on Linux. In the udev event we actually only get the /dev/disk15 (or whatever) path and not the pool/dataset name so we need to invoke a small helper utility called zvol_id which does an ioctl() on the new block device to look it up. At least with a zed event you'll get the device name and pool/dataset name which is nice, so that's an improvement.

[lundman]

Passing along major,minor could be useful for some platforms, sort of the future guessing I was trying to do with raw-device name.

Any changes desired for this PR?

[behlendorf]

@lundman I think I've derailed things enough for the time being. Since we've got the needed reviews let me just go ahead and get this merged so you can build on it. Thanks for your patience!