zfs: add kqfilter support for zvol's with volmode=dev

[rew]

Summary

Implement support for the EVFILT_VNODE filter to trigger the NOTE_ATTRIB
event.

This will allow bhyve to notify a guest when a zvol is resized from the
host.

Note: This patch is intended be upstreamed to OpenZFS. I want to get
feedback here, to make sure I'm on the right track before submitting a
patch upstream.

[kevans]

Tag a couple of ZFS folks

[imp]

The kqueue stuff looks good to my eye, but I'd like some ZFS folks to chime in.

[jhb]

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

638

My only thought is I don't know if you want to fail requests for any flags other than NOTE_ATTRIB?

[mav]

I am not closely familiar with kqueue, so only couple comments.

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

1340

Here you may access never initialized knlist in case make_dev_s() failed in the next chunk. You should move this down into the "if".

1556

Should NOTE_EXTEND also be reported if the size was increased?

Also searching through the kernel I see NOTE_ATTRIB reported only when file extended attributes or ACLs are changed. Am I missing something?

[mav]

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

638

I guess NOTE_DELETE could also easily be implemented and bhyve could use it. OPEN and CLOSE/CLOSE_WRITE also look trivial (not sure what for ;) ), unless they are already handled by devfs.

[rew]

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

638

I'll fail requests for any flag that's not supported - which makes sense.

I could see how NOTE_DELETE could be handy.

1340

good catch, thanks!

1556

Should NOTE_EXTEND also be reported if the size was increased?

It could, it would require a change in the OpenZFS bits that are not FreeBSD specific though. zv->zv_volsize is assigned to volsize before FreeBSD's zvol_update_volsize() is called - so that makes it difficult to figure out if we're extending, shrinking, or no change. I'm assuming it's not supposed to be like that.

Also searching through the kernel I see NOTE_ATTRIB reported only when file extended attributes or ACLs are changed. Am I missing something?

And regular file attributes...when the size of a regular file gets changed, NOTE_ATTRIB will be reported. I didn't see any other character devices catching the EVFILT_VNODE filter though.

[mav]

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

1556

Should NOTE_EXTEND also be reported if the size was increased?

It could, it would require a change in the OpenZFS bits that are not FreeBSD specific though. zv->zv_volsize is assigned to volsize before FreeBSD's zvol_update_volsize() is called - so that makes it difficult to figure out if we're extending, shrinking, or no change. I'm assuming it's not supposed to be like that.

It is weird that zv->zv_volsize is set both by shared code and FreeBSD-specific zvol_update_volsize(), but not Linux. It could probably benefit from some unification

[jhb]

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

1556

NOTE_EXTEND is a bit odd. Note that for directories it handles both shrinking and extending, but it does only handle extending via write(2) for regular files. (A NOTE_RESIZE would be nice if it existed) NOTE_ATTRIB is to catch things like truncate(2) which do VOP_SETATTR. It does seem that perhaps bhyve should listen for both events and just check the size if either event fires.

[afedorov]

@rew, do you have any plans to continue this work? If not, I'd like to take over.

[afedorov]

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

1448

@rew , there is no knlist_init_sx() in CURRENT. My understanding is that you are trying to introduce it in https://reviews.freebsd.org/D33401. Should sx(9) be here due to a ZFS zvol limitation, or are you trying to reuse zv->zv_state_lock?

[afedorov]

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

1448

Sorry, I mean https://reviews.freebsd.org/D33400

[rew]

In D33401#814574, @afedorov wrote:

@rew, do you have any plans to continue this work? If not, I'd like to take over.

Thanks for checking in..

I'll submit a pull request this coming week to OpenZFS and start getting this wrapped up.

As I'm thinking about it...I suppose https://reviews.freebsd.org/D33400 should be committed before the pull request goes up.

sys/contrib/openzfs/module/os/freebsd/zfs/zvol_os.c

1448

To reuse the zv->zv_state_lock...my line of thought is that it's easily accessible and is consistent with how other devices use their lock for the knlist.

There is a common knlist lock in kern_event.c meant for lockless objects, but that didn't feel like the correct solution.

[rew]

I've submitted a pull request upstream:

https://github.com/openzfs/zfs/pull/13773