Events for mount, remount and umount in devdAdd remount vfs event, publish mount, remount and unmount events.

[imp]

Summary

Some function had the blank lines, others didn't. Most of the ones that didn't were newer, so remove this now-optional blank line everywhere.

All the other printf() calls cast to (void) here, do the two newer ones for consistency.

Add VFS FS events for mount and unmount to devctl/devd

Report when a filesystem is mounted or unmounted via devd, along with details
about the mount point and mount options.

Sponsored by: Netflix

Add a remounted vfs event.

Hook up remounted vfs event to devd

[kib]

sys/kern/subr_bus.c

5935 ↗

(On Diff #75464)

Are spaces problematic for devd notifications consumers, typically shell scripts ?

5947 ↗

(On Diff #75464)

Does it make sense to decode 'reload' flag ?

5987 ↗

(On Diff #75464)

Both mount and unmount events are invoked while some vnodes are locked. sbuf_new_auto() seems to mean M_WAITOK allocations for the sbuf data by default. Waitable memory allocations while a vnode is locked can cause deadlock if the only way for pagedaemon to make the progress is to flush the locked vnode.

6000 ↗

(On Diff #75464)

!= 0

6011 ↗

(On Diff #75464)

I suggest to move all code to calculate sbuf strings from struct mount into kern/vfs_mount.c.
Even if functions like devctl_safe_quite_sb() would need to be exported, or better moved to kern/subr_sbuf.c under more generic name.

[imp]

sys/kern/subr_bus.c

5987 ↗

(On Diff #75464)

Gotcha. I went back and forth. I'll do a NOWAIT malloc for a buffer too big for the stack and just omit the message if that fails (which should be quite rare). That will avoid the deadlock.

6011 ↗

(On Diff #75464)

Like the whole thing? Or just the stuff that does flags and options?

[kib]

sys/kern/subr_bus.c

5987 ↗

(On Diff #75464)

/dev/devctl uses nowait malloc anyway, AFAIR ?

6011 ↗

(On Diff #75464)

I would say, move the whole thing. Then it is questionable why do you want to register eventhandlers instead of doing direct calls to the functions.

[imp]

sys/kern/subr_bus.c

5987 ↗

(On Diff #75464)

Yes. It does.

6011 ↗

(On Diff #75464)

OK. I've moved the whole thing, but need to tweak one or two more things before reposting the review. I'll do that tomorrow.

[imp]

Move everything to vfs_mount.c and simplify, per suggestions from kib@. Also do
a nowait alloc of the buffer, even though it might not strictly be needed after
the shuffle.

I didn't rename / make more general devctl_safe_quote_sb.
I didn't take the suggestion to export the bit after the MNT_ for the mount bits, but used mount's names instead.

[imp]

sbin/mount/mount.c

95–96

This needs a smidge more work... I missed it focusing on the kernel.

sys/kern/subr_bus.c

46 ↗

(On Diff #75587)

Hmm, stray now and can be removed.

5947 ↗

(On Diff #75464)

I was unsure. What do you think?

sys/kern/vfs_mount.c

105

Not sure this is the right name to use, so would be interested in feedback.

[imp]

Tweak mount to compile :)

[kib]

sbin/mount/mount.c

96–97

static const

sys/kern/vfs_mount.c

45

Perhaps devd events stuff could be split in dedicated header, as a followup.

1844

coveredvp is still locked there.

That said, I am not sure that it is reasonable to report mount options for unmount event.

2461

I suggest mount_devd_event()

[mckusick]

This change will provide the interface that we will need to notify daemon processes for our forcible unmount and forcible downgrade to read-only as well as re-establishment of read-write after a successful background fsck.

[trasz]

Update devd.conf.5 perhaps?

[imp]

sys/kern/vfs_mount.c

1844

Won't they communicate how the system was last mounted?

2461

elsewhere we use 'devctl' rather than 'devd' so I'll use mount_devctl_event

[imp]

Fold in feedback from reviewers

[imp]

All commits this time, eh?

[imp]

cddl/compat/opensolaris/misc/zmount.c

50

this is holding until after we land OpenZFS and won' tbe in this commit.

sys/amd64/conf/GENERIC

74

this won't be in the commit.

[kib]

sys/kern/vfs_mount.c

1844

Is it useful ? Also the operation of unmounting could mangle options in the process. From very quick view, for instance UFS clears MNT_LOCAL. Why ? I do not know.

2470

buf cannot be null.

[imp]

sys/kern/vfs_mount.c

1844

Ah, fair point. I'll create a followup.

2470

I'd intended to use M_NOWAIT...

[imp]

sys/kern/vfs_mount.c

1844

I added to umount, I just realized, to allow us to see if the umount was forced or not, which I know now isn't cleared... should we have a different mechanism to communicate that?

[mckusick]

What is the status with this change?

sys/kern/vfs_mount.c

1844

I think that this communication mechanism is fine.

[imp]

In D25969#582538, @mckusick wrote:

What is the status with this change?

I've committed this, but there were problems with it that I was too impatient to wait around to discover in the review :(. So it's there, mostly working but needs followup work which I have on my plate and mostly in review (well, the precursors for it).

[mckusick]

A new review? If so, I suggest that you close this one and give a pointer to the new one.

[imp]

In D25969#582988, @mckusick wrote:

A new review? If so, I suggest that you close this one and give a pointer to the new one.

I'll close this review and add you to the new one when I open it.